Fix hard crash when describing "zero" datetime (#64)

gwynne · web-flow · commit e453cf5537ea · 2021-09-08T09:11:58.000-05:00
* 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. * Use the thread-safe gmtime_r() instead of gmtime() in MySQLTime.init(date:). * Pass a more sensible capacity value to ByteBufferAllocator for SHA digests. * Fix deprecation warnings for TLSConfiguration and declare the explicit dependency on the update NIOSSL version. * Another round of long-overdue CI updates * Don't crash if server capabilities are not available during connection close.
diff --git 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:
@@ -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
@@ -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
@@ -117,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
@@ -126,16 +129,19 @@ jobs:
           - mysql@8.0
           - mysql@5.7
           - mariadb
+        xcode:
+          - latest
+          - latest-stable
         include:
           - username: root
           - formula: mariadb
             username: runner
     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
@@ -157,4 +163,20 @@ jobs:
         env: 
           MYSQL_HOSTNAME: '127.0.0.1'
           MYSQL_DATABASE: vapor_database
-          LOG_LEVEL: error
+          LOG_LEVEL: trace
+#  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 }}
diff --git 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
@@ -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/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<Void>?) 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 {
diff --git 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/Sources/MySQLNIO/MySQLTime.swift b/Sources/MySQLNIO/MySQLTime.swift
@@ -58,18 +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)
-        let tm = gmtime(&rawtime)!.pointee
+        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)
         )
     }
diff --git 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
 }
diff --git 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()
diff --git 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
         )
     }

Original file line number	Diff line number	Diff line change
`@@ -12,12 +12,14 @@ extension MySQLConnection {`
`12`	`12`	`} catch {`
`13`	`13`	`return eventLoop.makeFailedFuture(error)`
`14`	`14`	`}`
	`15`	`+ var tls = TLSConfiguration.makeClientConfiguration()`
	`16`	`+ tls.certificateVerification = .none`
`15`	`17`	`return self.connect(`
`16`	`18`	`to: addr,`
`17`	`19`	`username: env("MYSQL_USERNAME") ?? "vapor_username",`
`18`	`20`	`database: env("MYSQL_DATABASE") ?? "vapor_database",`
`19`	`21`	`password: env("MYSQL_PASSWORD") ?? "vapor_password",`
`20`		`- tlsConfiguration: .forClient(certificateVerification: .none),`
	`22`	`+ tlsConfiguration: tls,`
`21`	`23`	`on: eventLoop`
`22`	`24`	`)`
`23`	`25`	`}`