Various fixes found during interop testing (#442)

* Handle trailers-only responses

* Handle non-ASCII status messages correctly

* Remove incorrect space from timeout

* Pass trailing metadata down the pipeline

* Move gRPC header names into static constants

Sources/SwiftGRPCNIO/ClientCalls/BaseClientCall.swift

@@ -243,10 +243,10 @@ extension BaseClientCall {
     //! FIXME: Add a more specific user-agent, see: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#user-agents
     requestHead.headers.add(name: "user-agent", value: "grpc-swift-nio")
 
-    requestHead.headers.add(name: "grpc-accept-encoding", value: CompressionMechanism.acceptEncodingHeader)
+    requestHead.headers.add(name: GRPCHeaderName.acceptEncoding, value: CompressionMechanism.acceptEncodingHeader)
 
     if callOptions.timeout != .infinite {
-      requestHead.headers.add(name: "grpc-timeout", value: String(describing: callOptions.timeout))
+      requestHead.headers.add(name: GRPCHeaderName.timeout, value: String(describing: callOptions.timeout))
     }
 
     return requestHead

Sources/SwiftGRPCNIO/GRPCHeaderName.swift

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2019, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import Foundation
+
+public struct GRPCHeaderName {
+  private init() { }
+
+  public static let timeout = "grpc-timeout"
+  public static let encoding = "grpc-encoding"
+  public static let acceptEncoding = "grpc-accept-encoding"
+  public static let statusCode = "grpc-status"
+  public static let statusMessage = "grpc-message"
+}

Sources/SwiftGRPCNIO/GRPCStatusMessageMarshaller.swift

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2019, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import Foundation
+
+public struct GRPCStatusMessageMarshaller {
+  // See: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses
+  // 0x20-0x24 and 0x26-0x7e inclusive.
+  public static let allowedCharacters: CharacterSet = {
+    var allowed = CharacterSet(charactersIn: Unicode.Scalar(0x20)...Unicode.Scalar(0x7e))
+    // Remove '%' (0x25)
+    allowed.remove(Unicode.Scalar(0x25))
+    return allowed
+  }()
+
+  /// Adds percent encoding to the given message.
+  ///
+  /// - Parameter message: Message to percent encode.
+  /// - Returns: Percent encoded string, or `nil` if it could not be encoded.
+  public static func marshall(_ message: String) -> String? {
+    return message.addingPercentEncoding(withAllowedCharacters: GRPCStatusMessageMarshaller.allowedCharacters)
+  }
+
+  /// Removes percent encoding from the given message.
+  ///
+  /// - Parameter message: Message to remove encoding from.
+  /// - Returns: The string with percent encoding removed, or the input string if the encoding
+  ///   could not be removed.
+  public static func unmarshall(_ message: String) -> String {
+    return message.removingPercentEncoding ?? message
+  }
+}

Sources/SwiftGRPCNIO/GRPCTimeout.swift

@@ -30,7 +30,7 @@ public struct GRPCTimeout: CustomStringConvertible, Equatable {
     if amount >= 100_000_000  { throw GRPCTimeoutError.tooManyDigits }
 
     // See "Timeout" in https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#requests
-    let description = "\(amount) \(unit.rawValue)"
+    let description = "\(amount)\(unit.rawValue)"
     let nanoseconds = Int64(amount) * Int64(unit.asNanoseconds)
 
     return GRPCTimeout(nanoseconds: nanoseconds, description: description)

Sources/SwiftGRPCNIO/HTTP1ToRawGRPCClientCodec.swift

@@ -93,7 +93,13 @@ extension HTTP1ToRawGRPCClientCodec: ChannelInboundHandler {
       throw GRPCError.client(.HTTPStatusNotOk(head.status))
     }
 
-    let inboundCompression: CompressionMechanism = head.headers["grpc-encoding"]
+    // Trailers-Only response.
+    if head.headers.contains(name: GRPCHeaderName.statusCode) {
+      self.state = .expectingBodyOrTrailers
+      return try self.processTrailers(context: context, trailers: head.headers)
+    }
+
+    let inboundCompression: CompressionMechanism = head.headers[GRPCHeaderName.encoding]
       .first
       .map { CompressionMechanism(rawValue: $0) ?? .unknown } ?? .none
 
@@ -131,12 +137,23 @@ extension HTTP1ToRawGRPCClientCodec: ChannelInboundHandler {
       throw GRPCError.client(.invalidState("received trailers while in state \(state)"))
     }
 
-    let statusCode = trailers?["grpc-status"].first
-      .flatMap { Int($0) }
-      .flatMap { StatusCode(rawValue: $0) }
-    let statusMessage = trailers?["grpc-message"].first
+    let statusCode = trailers?[GRPCHeaderName.statusCode].first.flatMap {
+      Int($0)
+    }.flatMap {
+      StatusCode(rawValue: $0)
+    } ?? .unknown
+
+    let statusMessage = trailers?[GRPCHeaderName.statusMessage].first.map {
+      GRPCStatusMessageMarshaller.unmarshall($0)
+    }
+
+    var trailingCustomMetadata = trailers ?? HTTPHeaders()
+    trailingCustomMetadata.remove(name: GRPCHeaderName.statusCode)
+    trailingCustomMetadata.remove(name: GRPCHeaderName.statusMessage)
+
+    let status = GRPCStatus(code: statusCode, message: statusMessage, trailingMetadata: trailingCustomMetadata)
 
-    context.fireChannelRead(wrapInboundOut(.status(GRPCStatus(code: statusCode ?? .unknown, message: statusMessage))))
+    context.fireChannelRead(wrapInboundOut(.status(status)))
     return .ignore
   }
 }

Sources/SwiftGRPCNIO/HTTP1ToRawGRPCServerCodec.swift

@@ -224,9 +224,9 @@ extension HTTP1ToRawGRPCServerCodec: ChannelOutboundHandler {
       }
 
       var trailers = status.trailingMetadata
-      trailers.add(name: "grpc-status", value: String(describing: status.code.rawValue))
-      if let message = status.message {
-        trailers.add(name: "grpc-message", value: message)
+      trailers.add(name: GRPCHeaderName.statusCode, value: String(describing: status.code.rawValue))
+      if let message = status.message.flatMap(GRPCStatusMessageMarshaller.marshall) {
+        trailers.add(name: GRPCHeaderName.statusMessage, value: message)
       }
 
       if contentType == .text {

Tests/SwiftGRPCNIOTests/GRPCStatusMessageMarshallerTests.swift

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2019, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import Foundation
+import SwiftGRPCNIO
+import XCTest
+
+class GRPCStatusMessageMarshallerTests: XCTestCase {
+  func testASCIIMarshallingAndUnmarshalling() {
+    XCTAssertEqual(GRPCStatusMessageMarshaller.marshall("Hello, World!"), "Hello, World!")
+    XCTAssertEqual(GRPCStatusMessageMarshaller.unmarshall("Hello, World!"), "Hello, World!")
+  }
+
+  func testPercentMarshallingAndUnmarshalling() {
+    XCTAssertEqual(GRPCStatusMessageMarshaller.marshall("%"), "%25")
+    XCTAssertEqual(GRPCStatusMessageMarshaller.unmarshall("%25"), "%")
+
+    XCTAssertEqual(GRPCStatusMessageMarshaller.marshall("25%"), "25%25")
+    XCTAssertEqual(GRPCStatusMessageMarshaller.unmarshall("25%25"), "25%")
+  }
+
+  func testUnicodeMarshalling() {
+    XCTAssertEqual(GRPCStatusMessageMarshaller.marshall("🚀"), "%F0%9F%9A%80")
+    XCTAssertEqual(GRPCStatusMessageMarshaller.unmarshall("%F0%9F%9A%80"), "🚀")
+
+    let message = "\t\ntest with whitespace\r\nand Unicode BMP ☺ and non-BMP 😈\t\n"
+    let marshalled = "%09%0Atest with whitespace%0D%0Aand Unicode BMP %E2%98%BA and non-BMP %F0%9F%98%88%09%0A"
+    XCTAssertEqual(GRPCStatusMessageMarshaller.marshall(message), marshalled)
+    XCTAssertEqual(GRPCStatusMessageMarshaller.unmarshall(marshalled), message)
+  }
+}

Tests/SwiftGRPCNIOTests/XCTestManifests.swift

@@ -70,6 +70,17 @@ extension GRPCSecureInteroperabilityTests {
     ]
 }
 
+extension GRPCStatusMessageMarshallerTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__GRPCStatusMessageMarshallerTests = [
+        ("testASCIIMarshallingAndUnmarshalling", testASCIIMarshallingAndUnmarshalling),
+        ("testPercentMarshallingAndUnmarshalling", testPercentMarshallingAndUnmarshalling),
+        ("testUnicodeMarshalling", testUnicodeMarshalling),
+    ]
+}
+
 extension HTTP1ToRawGRPCServerCodecTests {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -255,6 +266,7 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(GRPCChannelHandlerTests.__allTests__GRPCChannelHandlerTests),
         testCase(GRPCInsecureInteroperabilityTests.__allTests__GRPCInsecureInteroperabilityTests),
         testCase(GRPCSecureInteroperabilityTests.__allTests__GRPCSecureInteroperabilityTests),
+        testCase(GRPCStatusMessageMarshallerTests.__allTests__GRPCStatusMessageMarshallerTests),
         testCase(HTTP1ToRawGRPCServerCodecTests.__allTests__HTTP1ToRawGRPCServerCodecTests),
         testCase(LengthPrefixedMessageReaderTests.__allTests__LengthPrefixedMessageReaderTests),
         testCase(NIOClientCancellingTests.__allTests__NIOClientCancellingTests),