Browse Source

Tolerate identity compression as no compression (#1016)

Motivation:

The GRPC specification acknowledges that "identity" is a way to say "no
compression", but when the user has disabled compression support we
refuse it as a compression format. That's not right.

Modifications:

- Add code to validate that the compression format isn't identity.
- Add some tests for newly added code.

Results:

Resolves #1005.
Cory Benfield 5 years ago
parent
commit
7346c615b7

+ 15 - 4
Sources/GRPC/HTTP1ToGRPCServerCodec.swift

@@ -563,11 +563,22 @@ struct MessageEncodingHeaderValidator {
         )
         )
       }
       }
 
 
-    // Compression is disabled and the client sent a message encoding header. We clearly don't
-    // support this. Note this is different to the supported but not advertised case since we have
-    // explicitly not enabled compression.
+    // Compression is disabled and the client sent a message encoding header. We don't support this
+    // unless the header is "identity", which is no compression. Note this is different to the
+    // supported but not advertised case since we have explicitly not enabled compression.
     case let (.disabled, .some(header)):
     case let (.disabled, .some(header)):
-      return .unsupported(requestEncoding: header, acceptEncoding: [])
+      guard let algorithm = CompressionAlgorithm(rawValue: header) else {
+        return .unsupported(
+          requestEncoding: header,
+          acceptEncoding: []
+        )
+      }
+
+      if algorithm == .identity {
+        return .noCompression
+      } else {
+        return .unsupported(requestEncoding: header, acceptEncoding: [])
+      }
 
 
     // The client didn't send a message encoding header.
     // The client didn't send a message encoding header.
     case (_, .none):
     case (_, .none):

+ 60 - 0
Tests/GRPCTests/CompressionTests.swift

@@ -293,4 +293,64 @@ class MessageCompressionTests: GRPCTestCase {
     self.wait(for: [status], timeout: self.defaultTimeout)
     self.wait(for: [status], timeout: self.defaultTimeout)
     XCTAssertEqual(responses.count, 1)
     XCTAssertEqual(responses.count, 1)
   }
   }
+
+  func testIdentityCompressionIsntCompression() throws {
+    // The client offers "identity" compression, the server doesn't support compression. We should
+    // tolerate this, as "identity" is no compression at all.
+    try self
+      .setupServer(encoding: .disabled)
+    // We can't specify a compression we don't support, like identity, so we'll specify no compression and then
+    // send a 'grpc-encoding' with our initial metadata.
+    self.setupClient(encoding: .disabled)
+
+    let headers: HPACKHeaders = ["grpc-encoding": "identity"]
+    let get = self.echo.get(
+      .with { $0.text = "foo" },
+      callOptions: CallOptions(customMetadata: headers)
+    )
+
+    let initialMetadata = self.expectation(description: "received initial metadata")
+    get.initialMetadata.map {
+      $0.contains(name: "grpc-encoding")
+    }.assertEqual(false, fulfill: initialMetadata)
+
+    let status = self.expectation(description: "received status")
+    get.status.map {
+      $0.code
+    }.assertEqual(.ok, fulfill: status)
+
+    self.wait(for: [initialMetadata, status], timeout: self.defaultTimeout)
+  }
+
+  func testCompressedRequestWithDisabledServerCompressionAndUnknownCompressionAlgorithm() throws {
+    try self.setupServer(encoding: .disabled)
+    // We can't specify a compression we don't support, so we'll specify no compression and then
+    // send a 'grpc-encoding' with our initial metadata.
+    self.setupClient(encoding: .enabled(.init(
+      forRequests: .none,
+      acceptableForResponses: [.deflate, .gzip],
+      decompressionLimit: .ratio(10)
+    )))
+
+    let headers: HPACKHeaders = ["grpc-encoding": "you-don't-support-this"]
+    let get = self.echo.get(
+      .with { $0.text = "foo" },
+      callOptions: CallOptions(customMetadata: headers)
+    )
+
+    let response = self.expectation(description: "received response")
+    get.response.assertError(fulfill: response)
+
+    let trailers = self.expectation(description: "received trailing metadata")
+    get.trailingMetadata.map {
+      $0.contains(name: "grpc-accept-encoding")
+    }.assertEqual(false, fulfill: trailers)
+
+    let status = self.expectation(description: "received status")
+    get.status.map {
+      $0.code
+    }.assertEqual(.unimplemented, fulfill: status)
+
+    self.wait(for: [response, trailers, status], timeout: self.defaultTimeout)
+  }
 }
 }

+ 2 - 0
Tests/GRPCTests/XCTestManifests.swift

@@ -756,10 +756,12 @@ extension MessageCompressionTests {
     static let __allTests__MessageCompressionTests = [
     static let __allTests__MessageCompressionTests = [
         ("testCompressedRequestsUncompressedResponses", testCompressedRequestsUncompressedResponses),
         ("testCompressedRequestsUncompressedResponses", testCompressedRequestsUncompressedResponses),
         ("testCompressedRequestWithCompressionNotSupportedOnServer", testCompressedRequestWithCompressionNotSupportedOnServer),
         ("testCompressedRequestWithCompressionNotSupportedOnServer", testCompressedRequestWithCompressionNotSupportedOnServer),
+        ("testCompressedRequestWithDisabledServerCompressionAndUnknownCompressionAlgorithm", testCompressedRequestWithDisabledServerCompressionAndUnknownCompressionAlgorithm),
         ("testDecompressionLimitIsRespectedByClientForStreamingCall", testDecompressionLimitIsRespectedByClientForStreamingCall),
         ("testDecompressionLimitIsRespectedByClientForStreamingCall", testDecompressionLimitIsRespectedByClientForStreamingCall),
         ("testDecompressionLimitIsRespectedByClientForUnaryCall", testDecompressionLimitIsRespectedByClientForUnaryCall),
         ("testDecompressionLimitIsRespectedByClientForUnaryCall", testDecompressionLimitIsRespectedByClientForUnaryCall),
         ("testDecompressionLimitIsRespectedByServerForStreamingCall", testDecompressionLimitIsRespectedByServerForStreamingCall),
         ("testDecompressionLimitIsRespectedByServerForStreamingCall", testDecompressionLimitIsRespectedByServerForStreamingCall),
         ("testDecompressionLimitIsRespectedByServerForUnaryCall", testDecompressionLimitIsRespectedByServerForUnaryCall),
         ("testDecompressionLimitIsRespectedByServerForUnaryCall", testDecompressionLimitIsRespectedByServerForUnaryCall),
+        ("testIdentityCompressionIsntCompression", testIdentityCompressionIsntCompression),
         ("testServerCanDecompressNonAdvertisedButSupportedCompression", testServerCanDecompressNonAdvertisedButSupportedCompression),
         ("testServerCanDecompressNonAdvertisedButSupportedCompression", testServerCanDecompressNonAdvertisedButSupportedCompression),
         ("testServerCompressesResponseWithDifferentAlgorithmToRequest", testServerCompressesResponseWithDifferentAlgorithmToRequest),
         ("testServerCompressesResponseWithDifferentAlgorithmToRequest", testServerCompressesResponseWithDifferentAlgorithmToRequest),
         ("testUncompressedRequestsCompressedResponses", testUncompressedRequestsCompressedResponses),
         ("testUncompressedRequestsCompressedResponses", testUncompressedRequestsCompressedResponses),