From 56a63b8d312af4a69dcbb3f15e9ba7c9cd6c6bd4 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 04:47:28 -0500 Subject: [PATCH 01/10] Fix #63: Hard crash in MySQLData.description when a zero-value datetime is received from older MySQL configurations which still do this. Returns a description of the epoch instead. --- Sources/MySQLNIO/MySQLData.swift | 2 +- Tests/MySQLNIOTests/MySQLNIOTests.swift | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Sources/MySQLNIO/MySQLData.swift b/Sources/MySQLNIO/MySQLData.swift index bdaf7e9..d7383a4 100644 --- a/Sources/MySQLNIO/MySQLData.swift +++ b/Sources/MySQLNIO/MySQLData.swift @@ -455,7 +455,7 @@ public struct MySQLData: CustomStringConvertible, ExpressibleByStringLiteral, Ex case .bit: return self.bool!.description case .datetime, .timestamp: - return self.date!.description + return (self.time!.date ?? Date(timeIntervalSince1970: 0)).description case .varchar, .varString, .string: return self.string!.debugDescription case .double: diff --git a/Tests/MySQLNIOTests/MySQLNIOTests.swift b/Tests/MySQLNIOTests/MySQLNIOTests.swift index 5701d8a..a50ac8d 100644 --- a/Tests/MySQLNIOTests/MySQLNIOTests.swift +++ b/Tests/MySQLNIOTests/MySQLNIOTests.swift @@ -264,6 +264,13 @@ final class MySQLNIOTests: XCTestCase { XCTAssert(time.microsecond == UInt32(100000)) XCTAssert(time2.microsecond == UInt32(100000)) } + + func testDate_zeroIsInvalidButMySQLReturnsIt() throws { + let zeroTime = MySQLTime() + let data = MySQLData(time: zeroTime) + + XCTAssertEqual(data.description, "1970-01-01 00:00:00 +0000") + } func testString_lengthEncoded_uint8() throws { let conn = try MySQLConnection.test(on: self.eventLoop).wait() From 8acf034f9b1b1d0f8e788af949ea6f0f024aee4a Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 04:48:41 -0500 Subject: [PATCH 02/10] Use the thread-safe gmtime_r() instead of gmtime() in MySQLTime.init(date:). --- Sources/MySQLNIO/MySQLTime.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/MySQLNIO/MySQLTime.swift b/Sources/MySQLNIO/MySQLTime.swift index 40a9c21..5ff2e56 100644 --- a/Sources/MySQLNIO/MySQLTime.swift +++ b/Sources/MySQLNIO/MySQLTime.swift @@ -58,7 +58,8 @@ public struct MySQLTime: Equatable, MySQLDataConvertible { public init(date: Date) { // let comps = Calendar.current.dateComponents(in: .gmt, from: date) var rawtime = Int(date.timeIntervalSince1970) - let tm = gmtime(&rawtime)!.pointee + var tm = tm() + gmtime_r(&rawtime, &tm) var microseconds = date.timeIntervalSince1970.microseconds if microseconds < 0.0 { microseconds = 1_000_000 - microseconds From dc474208e294d0e90124d7aadc43d2182e041878 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 04:49:07 -0500 Subject: [PATCH 03/10] Pass a more sensible capacity value to ByteBufferAllocator for SHA digests. --- Sources/MySQLNIO/Utilities/CryptoUtils.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/MySQLNIO/Utilities/CryptoUtils.swift b/Sources/MySQLNIO/Utilities/CryptoUtils.swift index cdb345e..4c12a27 100644 --- a/Sources/MySQLNIO/Utilities/CryptoUtils.swift +++ b/Sources/MySQLNIO/Utilities/CryptoUtils.swift @@ -2,14 +2,14 @@ import Crypto func sha256(_ messages: ByteBuffer...) -> ByteBuffer { let digest = SHA256.hash(data: [UInt8](messages.combine().readableBytesView)) - var buffer = ByteBufferAllocator().buffer(capacity: 0) + var buffer = ByteBufferAllocator().buffer(capacity: SHA256.Digest.byteCount) buffer.writeBytes(digest) return buffer } func sha1(_ messages: ByteBuffer...) -> ByteBuffer { let digest = Insecure.SHA1.hash(data: [UInt8](messages.combine().readableBytesView)) - var buffer = ByteBufferAllocator().buffer(capacity: 0) + var buffer = ByteBufferAllocator().buffer(capacity: Insecure.SHA1.Digest.byteCount) buffer.writeBytes(digest) return buffer } From 28cc9aa43d7702f9434300403672a933391d1915 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 04:49:33 -0500 Subject: [PATCH 04/10] Fix deprecation warnings for TLSConfiguration and declare the explicit dependency on the update NIOSSL version. --- Package.swift | 2 +- Sources/MySQLNIO/MySQLConnection.swift | 2 +- Tests/MySQLNIOTests/Utilities.swift | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Package.swift b/Package.swift index 1bc5738..8674b08 100644 --- a/Package.swift +++ b/Package.swift @@ -13,7 +13,7 @@ let package = Package( .package(url: "https://github.com/apple/swift-crypto.git", from: "1.0.0"), .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"), .package(url: "https://github.com/apple/swift-nio.git", from: "2.0.0"), - .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.0.0"), + .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.14.0"), ], targets: [ .target(name: "MySQLNIO", dependencies: [ diff --git a/Sources/MySQLNIO/MySQLConnection.swift b/Sources/MySQLNIO/MySQLConnection.swift index 1627e6a..c25f74a 100644 --- a/Sources/MySQLNIO/MySQLConnection.swift +++ b/Sources/MySQLNIO/MySQLConnection.swift @@ -6,7 +6,7 @@ public final class MySQLConnection: MySQLDatabase { username: String, database: String, password: String? = nil, - tlsConfiguration: TLSConfiguration? = .forClient(), + tlsConfiguration: TLSConfiguration? = .makeClientConfiguration(), serverHostname: String? = nil, logger: Logger = .init(label: "codes.vapor.mysql"), on eventLoop: EventLoop diff --git a/Tests/MySQLNIOTests/Utilities.swift b/Tests/MySQLNIOTests/Utilities.swift index 3ee90b3..ac27c95 100644 --- a/Tests/MySQLNIOTests/Utilities.swift +++ b/Tests/MySQLNIOTests/Utilities.swift @@ -12,12 +12,14 @@ extension MySQLConnection { } catch { return eventLoop.makeFailedFuture(error) } + var tls = TLSConfiguration.makeClientConfiguration() + tls.certificateVerification = .none return self.connect( to: addr, username: env("MYSQL_USERNAME") ?? "vapor_username", database: env("MYSQL_DATABASE") ?? "vapor_database", password: env("MYSQL_PASSWORD") ?? "vapor_password", - tlsConfiguration: .forClient(certificateVerification: .none), + tlsConfiguration: tls, on: eventLoop ) } From 7ff7d10474ed7b0403f05770d45a9dad0ad58747 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 04:55:03 -0500 Subject: [PATCH 05/10] Another round of long-overdue CI updates --- .github/workflows/test.yml | 52 +++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3a5b5a5..4e9daa8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,7 +22,7 @@ jobs: MYSQL_USER: vapor_username MYSQL_PASSWORD: vapor_password MYSQL_DATABASE: vapor_database - container: swift:5.2-focal + container: swift:5.4-focal strategy: fail-fast: false matrix: @@ -63,36 +63,39 @@ jobs: - mysql:8.0 - mariadb:latest runner: - # 5.2 Stable - swift:5.2-xenial - swift:5.2-bionic - swift:5.2-focal - swift:5.2-centos7 - swift:5.2-centos8 - swift:5.2-amazonlinux2 - # 5.2 Unstable - - swiftlang/swift:nightly-5.2-xenial - - swiftlang/swift:nightly-5.2-bionic - # 5.3 Unstable - - swiftlang/swift:nightly-5.3-xenial - - swiftlang/swift:nightly-5.3-bionic - - swiftlang/swift:nightly-5.3-focal - - swiftlang/swift:nightly-5.3-centos7 - - swiftlang/swift:nightly-5.3-centos8 - - swiftlang/swift:nightly-5.3-amazonlinux2 - # Main Unsable - - swiftlang/swift:nightly-master-xenial - - swiftlang/swift:nightly-master-bionic - - swiftlang/swift:nightly-master-focal - - swiftlang/swift:nightly-master-centos7 - - swiftlang/swift:nightly-master-centos8 - - swiftlang/swift:nightly-master-amazonlinux2 + - swift:5.3-xenial + - swift:5.3-bionic + - swift:5.3-focal + - swift:5.3-centos7 + - swift:5.3-centos8 + - swift:5.3-amazonlinux2 + - swift:5.4-bionic + - swift:5.4-focal + - swift:5.4-centos7 + - swift:5.4-centos8 + - swift:5.4-amazonlinux2 + - swiftlang/swift:nightly-5.5-focal + - swiftlang/swift:nightly-5.5-centos8 + - swiftlang/swift:nightly-5.5-amazonlinux2 + - swiftlang/swift:nightly-main-focal + - swiftlang/swift:nightly-main-centos8 + - swiftlang/swift:nightly-main-amazonlinux2 exclude: - runner: swift:5.2-amazonlinux2 dbimage: mysql:8.0 - - runner: swiftlang/swift:nightly-5.3-amazonlinux2 + - runner: swift:5.3-amazonlinux2 dbimage: mysql:8.0 - - runner: swiftlang/swift:nightly-master-amazonlinux2 + - runner: swift:5.4-amazonlinux2 + dbimage: mysql:8.0 + - runner: swiftlang/swift:nightly-5.5-amazonlinux2 + dbimage: mysql:8.0 + - runner: swiftlang/swift:nightly-main-amazonlinux2 dbimage: mysql:8.0 container: ${{ matrix.runner }} runs-on: ubuntu-latest @@ -126,6 +129,9 @@ jobs: - mysql@8.0 - mysql@5.7 - mariadb + xcode: + - latest + - latest-stable include: - username: root - formula: mariadb @@ -133,9 +139,9 @@ jobs: runs-on: macos-latest steps: - name: Select latest available Xcode - uses: maxim-lobanov/setup-xcode@1.0 + uses: maxim-lobanov/setup-xcode@v1 with: - xcode-version: latest + xcode-version: ${{ matrix.xcode }} - name: Install MySQL server from Homebrew run: brew install ${{ matrix.formula }} && brew link --force ${{ matrix.formula }} - name: Start MySQL server From 5b97bbe510e6a1e0e5d563c1f47a3d0f1a5816d9 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 05:16:42 -0500 Subject: [PATCH 06/10] Vars named identically to structs cause trouble before Swift 5.4. --- Sources/MySQLNIO/MySQLTime.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/MySQLNIO/MySQLTime.swift b/Sources/MySQLNIO/MySQLTime.swift index 5ff2e56..8ba2a54 100644 --- a/Sources/MySQLNIO/MySQLTime.swift +++ b/Sources/MySQLNIO/MySQLTime.swift @@ -58,19 +58,19 @@ public struct MySQLTime: Equatable, MySQLDataConvertible { public init(date: Date) { // let comps = Calendar.current.dateComponents(in: .gmt, from: date) var rawtime = Int(date.timeIntervalSince1970) - var tm = tm() - gmtime_r(&rawtime, &tm) + var tms = tm() + gmtime_r(&rawtime, &tms) var microseconds = date.timeIntervalSince1970.microseconds if microseconds < 0.0 { microseconds = 1_000_000 - microseconds } self.init( - year: numericCast(1900 + tm.tm_year), - month: numericCast(1 + tm.tm_mon), - day: numericCast(tm.tm_mday), - hour: numericCast(tm.tm_hour), - minute: numericCast(tm.tm_min), - second: numericCast(tm.tm_sec), + year: numericCast(1900 + tms.tm_year), + month: numericCast(1 + tms.tm_mon), + day: numericCast(tms.tm_mday), + hour: numericCast(tms.tm_hour), + minute: numericCast(tms.tm_min), + second: numericCast(tms.tm_sec), microsecond: UInt32(microseconds) ) } From 8c96e444e059ef644c9901e1f6db41da784ac2ac Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 06:23:19 -0500 Subject: [PATCH 07/10] Add Windows to CI, but allow it to fail since we don't expect this to actually work yet. --- .github/workflows/test.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4e9daa8..79ff817 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -164,3 +164,19 @@ jobs: MYSQL_HOSTNAME: '127.0.0.1' MYSQL_DATABASE: vapor_database LOG_LEVEL: error + windows: + strategy: + fail-fast: false + matrix: + swiftver: + - '5.4.2' + runs-on: windows-latest + continue-on-error: true + steps: + - name: Check out code + uses: actions/checkout@v2 + - name: Run tests + uses: MaxDesiatov/swift-windows-action@v1 + with: + shell-action: swift test + swift-version: ${{ matrix.swiftver }} From 98b8230132a91504696a23e763b24122ed71fa58 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 06:40:53 -0500 Subject: [PATCH 08/10] Comment Windows test workflow out; neat to have, but SPM on Windows appears to be broken again and the workflow never completes. --- .github/workflows/test.yml | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 79ff817..22e7b1b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -164,19 +164,19 @@ jobs: MYSQL_HOSTNAME: '127.0.0.1' MYSQL_DATABASE: vapor_database LOG_LEVEL: error - windows: - strategy: - fail-fast: false - matrix: - swiftver: - - '5.4.2' - runs-on: windows-latest - continue-on-error: true - steps: - - name: Check out code - uses: actions/checkout@v2 - - name: Run tests - uses: MaxDesiatov/swift-windows-action@v1 - with: - shell-action: swift test - swift-version: ${{ matrix.swiftver }} +# windows: +# strategy: +# fail-fast: false +# matrix: +# swiftver: +# - '5.4.2' +# runs-on: windows-latest +# continue-on-error: true +# steps: +# - name: Check out code +# uses: actions/checkout@v2 +# - name: Run tests +# uses: MaxDesiatov/swift-windows-action@v1 +# with: +# shell-action: swift test +# swift-version: ${{ matrix.swiftver }} From cb75234bc6de1d542090c41bf3dbd06eb4116e25 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 06:41:09 -0500 Subject: [PATCH 09/10] Don't crash if server capabilities are not available during connection close. --- Sources/MySQLNIO/MySQLConnectionHandler.swift | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/Sources/MySQLNIO/MySQLConnectionHandler.swift b/Sources/MySQLNIO/MySQLConnectionHandler.swift index 15c6f9f..3441903 100644 --- a/Sources/MySQLNIO/MySQLConnectionHandler.swift +++ b/Sources/MySQLNIO/MySQLConnectionHandler.swift @@ -74,7 +74,10 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { case .commandPhase: if let current = self.queue.first { do { - let commandState = try current.handler.handle(packet: &packet, capabilities: self.serverCapabilities!) + guard let capabilities = self.serverCapabilities else { + throw MySQLError.protocolError + } + let commandState = try current.handler.handle(packet: &packet, capabilities: capabilities) self.handleCommandState(context: context, commandState) } catch { self.queue.removeFirst() @@ -226,7 +229,10 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { database: state.database, authPluginName: authPluginName ) - try context.write(self.wrapOutboundOut(.encode(res, capabilities: self.serverCapabilities!)), promise: nil) + guard let capabilities = self.serverCapabilities else { + throw MySQLError.protocolError + } + try context.write(self.wrapOutboundOut(.encode(res, capabilities: capabilities)), promise: nil) context.flush() } @@ -264,7 +270,10 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { } guard !packet.isError else { self.logger.trace("caching_sha2_password replied ERR, decoding") - let err = try packet.decode(MySQLProtocol.ERR_Packet.self, capabilities: self.serverCapabilities!) + guard let capabilities = self.serverCapabilities else { + throw MySQLError.protocolError + } + let err = try packet.decode(MySQLProtocol.ERR_Packet.self, capabilities: capabilities) throw MySQLError.server(err) } guard let status = packet.payload.readInteger(endianness: .little, as: UInt8.self) else { @@ -327,7 +336,10 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { case "mysql_native_password": guard !packet.isError else { self.logger.trace("mysql_native_password sent ERR, decoding") - let error = try packet.decode(MySQLProtocol.ERR_Packet.self, capabilities: self.serverCapabilities!) + guard let capabilities = self.serverCapabilities else { + throw MySQLError.protocolError + } + let error = try packet.decode(MySQLProtocol.ERR_Packet.self, capabilities: capabilities) throw MySQLError.server(error) } guard !packet.isOK else { @@ -365,12 +377,16 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { guard let command = self.queue.first else { return } + guard let capabilities = self.serverCapabilities else { + command.promise.fail(MySQLError.protocolError) + return + } self.commandState = .busy // send initial do { self.sequence.current = nil - let commandState = try command.handler.activate(capabilities: self.serverCapabilities!) + let commandState = try command.handler.activate(capabilities: capabilities) self.handleCommandState(context: context, commandState) } catch { self.queue.removeFirst() @@ -413,7 +429,10 @@ final class MySQLConnectionHandler: ChannelDuplexHandler { private func _close(context: ChannelHandlerContext, mode: CloseMode, promise: EventLoopPromise?) throws { self.sequence.reset() let quit = MySQLProtocol.COM_QUIT() - try context.write(self.wrapOutboundOut(.encode(quit, capabilities: self.serverCapabilities!)), promise: nil) + // N.B.: It is possible to get here without having processed a handshake packet yet, in which case there will + // not be any serverCapabilities. Since COM_QUIT doesn't care about any of those anyway, don't crash if they're + // not there! + try context.write(self.wrapOutboundOut(.encode(quit, capabilities: self.serverCapabilities ?? .init())), promise: nil) context.flush() if let promise = promise { From 44412829a00d456ed43bfd0c52c74b3c2e864fcb Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 8 Sep 2021 07:44:19 -0500 Subject: [PATCH 10/10] Max out logging level in CI tests, hopefully this won't mask the weird intermittent failures --- .github/workflows/test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 22e7b1b..e1e5e37 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -53,7 +53,7 @@ jobs: MYSQL_HOSTNAME: mysql-a MYSQL_HOSTNAME_A: mysql-a MYSQL_HOSTNAME_B: mysql-b - LOG_LEVEL: error + LOG_LEVEL: trace linux: strategy: fail-fast: false @@ -120,7 +120,7 @@ jobs: run: swift test --enable-test-discovery --sanitize=thread env: MYSQL_HOSTNAME: mysql - LOG_LEVEL: error + LOG_LEVEL: trace macOS: strategy: fail-fast: false @@ -163,7 +163,7 @@ jobs: env: MYSQL_HOSTNAME: '127.0.0.1' MYSQL_DATABASE: vapor_database - LOG_LEVEL: error + LOG_LEVEL: trace # windows: # strategy: # fail-fast: false