Browse Source

Use string comparison instead of regex in the HTTPProtocolSwitcher (#618)

Motivation:

Regex is overkill to check which HTTP version is being used in the
HTTPProtocolSwitcher.

Modifications:

Determine the HTTP version by checking for a substring instead of using
a regex.

Result:

Fewer allocations!
George Barnett 6 years ago
parent
commit
2cd9ea7008
2 changed files with 25 additions and 35 deletions
  1. 8 8
      Package.resolved
  2. 17 27
      Sources/GRPC/HTTPProtocolSwitcher.swift

+ 8 - 8
Package.resolved

@@ -15,8 +15,8 @@
         "repositoryURL": "https://github.com/apple/swift-nio.git",
         "state": {
           "branch": null,
-          "revision": "9201908b54578aa33f1d1826a5a680aca8991843",
-          "version": "2.8.0"
+          "revision": "8066b0f581604e3711979307a4377457e2b0f007",
+          "version": "2.9.0"
         }
       },
       {
@@ -33,8 +33,8 @@
         "repositoryURL": "https://github.com/apple/swift-nio-ssl.git",
         "state": {
           "branch": null,
-          "revision": "f5dd7a60ff56f501ff7bf9be753e4b1875bfaf20",
-          "version": "2.4.0"
+          "revision": "e5c1af45ac934ac0a6117b2927a51d845cf4f705",
+          "version": "2.4.3"
         }
       },
       {
@@ -42,8 +42,8 @@
         "repositoryURL": "https://github.com/apple/swift-nio-transport-services.git",
         "state": {
           "branch": null,
-          "revision": "6cba688855180e0ebf21f40d80987556986cfe03",
-          "version": "1.1.1"
+          "revision": "e30bf63ea1b47132de05771216ccd1e2a5123bd0",
+          "version": "1.2.0"
         }
       },
       {
@@ -51,8 +51,8 @@
         "repositoryURL": "https://github.com/apple/swift-protobuf.git",
         "state": {
           "branch": null,
-          "revision": "3a3594f84b746793c84c2ab2f1e855aaa9d3a593",
-          "version": "1.6.0"
+          "revision": "da75a93ac017534e0028e83c0e4fc4610d2acf48",
+          "version": "1.7.0"
         }
       }
     ]

+ 17 - 27
Sources/GRPC/HTTPProtocolSwitcher.swift

@@ -84,15 +84,28 @@ extension HTTPProtocolSwitcher: ChannelInboundHandler, RemovableChannelHandler {
       // couldn't be detected.
       var inBuffer = self.unwrapInboundIn(data)
       guard let initialData = inBuffer.readString(length: inBuffer.readableBytes),
-        let preamble = initialData.split(separator: "\r\n",
-                                         maxSplits: 1,
-                                         omittingEmptySubsequences: true).first,
-        let version = protocolVersion(String(preamble)) else {
+        let firstLine = initialData.split(
+          separator: "\r\n",
+          maxSplits: 1,
+          omittingEmptySubsequences: true
+        ).first else {
           self.logger.error("unable to determine http version")
           context.fireErrorCaught(HTTPProtocolVersionError.invalidHTTPProtocolVersion)
           return
       }
 
+      let version: HTTPProtocolVersion
+
+      if firstLine.contains("HTTP/2") {
+        version = .http2
+      } else if firstLine.contains("HTTP/1") {
+        version = .http1
+      } else {
+        self.logger.error("unable to determine http version")
+        context.fireErrorCaught(HTTPProtocolVersionError.invalidHTTPProtocolVersion)
+        return
+      }
+
       self.logger.info("determined http version", metadata: ["http_version": "\(version)"])
 
       // Once configured remove ourself from the pipeline, or handle the error.
@@ -161,27 +174,4 @@ extension HTTPProtocolSwitcher: ChannelInboundHandler, RemovableChannelHandler {
       context.fireErrorCaught(error)
     }
   }
-
-  /// Peek into the first line of the packet to check which HTTP version is being used.
-  private func protocolVersion(_ preamble: String) -> HTTPProtocolVersion? {
-    let range = NSRange(location: 0, length: preamble.utf16.count)
-    let regex = try! NSRegularExpression(pattern: "^.*HTTP/(\\d)\\.\\d$")
-    guard let result = regex.firstMatch(in: preamble, options: [], range: range) else {
-      return nil
-    }
-
-    let versionRange = result.range(at: 1)
-
-    let start = String.Index(utf16Offset: versionRange.location, in: preamble)
-    let end = String.Index(utf16Offset: versionRange.location + versionRange.length, in: preamble)
-
-    switch String(preamble.utf16[start..<end])! {
-    case "1":
-      return .http1
-    case "2":
-      return .http2
-    default:
-      return nil
-    }
-  }
 }