Merge pull request #256 from MrMage/metadata-tests

Fix trailing metadata corruption if accessed after the original has been deallocated

Sources/SwiftGRPC/Core/OperationGroup.swift

@@ -144,7 +144,7 @@ class OperationGroup {
       case .receiveInitialMetadata:
         cachedInitialMetadata = Metadata(
           underlyingArray: cgrpc_observer_recv_initial_metadata_get_metadata(underlyingObservers[i]),
-          ownsFields: false)
+          ownsFields: false).copy()
         return cachedInitialMetadata!
       default:
         continue
@@ -198,7 +198,7 @@ class OperationGroup {
       case .receiveStatusOnClient:
         cachedTrailingMetadata = Metadata(
           underlyingArray: cgrpc_observer_recv_status_on_client_get_metadata(underlyingObservers[i]),
-          ownsFields: false)
+          ownsFields: false).copy()
         return cachedTrailingMetadata!
       default:
         continue

Tests/LinuxMain.swift

@@ -27,6 +27,7 @@ XCTMain([
   testCase(EchoTests.allTests),
   testCase(EchoTestsSecure.allTests),
   testCase(EchoTestsMutualAuth.allTests),
+  testCase(MetadataTests.allTests),
   testCase(ServerCancellingTests.allTests),
   testCase(ServerTestExample.allTests),
   testCase(ServerThrowingTests.allTests),

Tests/SwiftGRPCTests/MetadataTests.swift

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2018, 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
+@testable import SwiftGRPC
+import XCTest
+
+class MetadataTests: XCTestCase {
+  static var allTests: [(String, (MetadataTests) -> () throws -> Void)] {
+    return [
+      ("testCopyDoesNotCorruptData", testCopyDoesNotCorruptData)
+    ]
+  }
+  
+  func testCopyDoesNotCorruptData() {
+    let metadata = try! Metadata(["foo": "bar"])
+    XCTAssertEqual(["foo": "bar"], metadata.copy().dictionaryRepresentation)
+  }
+}

Tests/SwiftGRPCTests/ServerThrowingTests.swift

@@ -19,10 +19,12 @@ import Foundation
 import XCTest
 
 fileprivate let testStatus = ServerStatus(code: .permissionDenied, message: "custom status message")
+fileprivate let testStatusWithTrailingMetadata = ServerStatus(code: .permissionDenied, message: "custom status message",
+                                                              trailingMetadata: try! Metadata(["some_long_key": "bar"]))
 
 fileprivate class StatusThrowingProvider: Echo_EchoProvider {
   func get(request: Echo_EchoRequest, session _: Echo_EchoGetSession) throws -> Echo_EchoResponse {
-    throw testStatus
+    throw testStatusWithTrailingMetadata
   }
   
   func expand(request: Echo_EchoRequest, session: Echo_EchoExpandSession) throws -> ServerStatus? {
@@ -42,6 +44,7 @@ class ServerThrowingTests: BasicEchoTestCase {
   static var allTests: [(String, (ServerThrowingTests) -> () throws -> Void)] {
     return [
       ("testServerThrowsUnary", testServerThrowsUnary),
+      ("testServerThrowsUnary_noTrailingMetadataCorruption", testServerThrowsUnary_noTrailingMetadataCorruptionAfterOriginalTrailingMetadataGetsReleased),
       ("testServerThrowsClientStreaming", testServerThrowsClientStreaming),
       ("testServerThrowsServerStreaming", testServerThrowsServerStreaming),
       ("testServerThrowsBidirectionalStreaming", testServerThrowsBidirectionalStreaming)
@@ -52,15 +55,37 @@ class ServerThrowingTests: BasicEchoTestCase {
 }
 
 extension ServerThrowingTests {
-  func testServerThrowsUnary() {
+  func testServerThrowsUnary() throws {
     do {
       let result = try client.get(Echo_EchoRequest(text: "foo")).text
       XCTFail("should have thrown, received \(result) instead")
-    } catch {
-      guard case let .callError(callResult) = error as! RPCError
+      return
+    } catch let error as RPCError {
+      guard case let .callError(callResult) = error
         else { XCTFail("unexpected error \(error)"); return }
+      
       XCTAssertEqual(.permissionDenied, callResult.statusCode)
       XCTAssertEqual("custom status message", callResult.statusMessage)
+      XCTAssertEqual(["some_long_key": "bar"], callResult.trailingMetadata?.dictionaryRepresentation)
+    }
+  }
+  
+  func testServerThrowsUnary_noTrailingMetadataCorruptionAfterOriginalTrailingMetadataGetsReleased() throws {
+    do {
+      let result = try client.get(Echo_EchoRequest(text: "foo")).text
+      XCTFail("should have thrown, received \(result) instead")
+      return
+    } catch let error as RPCError {
+      guard case let .callError(callResult) = error
+        else { XCTFail("unexpected error \(error)"); return }
+      
+      // Send another RPC to cause the original metadata array to be deallocated.
+      // If we were not copying the metadata array correctly, this would cause the metadata of the call result to become
+      // corrupted.
+      _ = try? client.get(Echo_EchoRequest(text: "foo")).text
+      // It seems like we need this sleep for the memory corruption to occur.
+      Thread.sleep(forTimeInterval: 0.01)
+      XCTAssertEqual(["some_long_key": "bar"], callResult.trailingMetadata?.dictionaryRepresentation)
     }
   }