Improve robustness of metadata string handling, fix top-level tests.

Author: timburks

Sources/CgRPC/include/CgRPC.h

@@ -165,8 +165,9 @@ void cgrpc_operations_add_operation(cgrpc_operations *call, cgrpc_observer *obse
 cgrpc_metadata_array *cgrpc_metadata_array_create();
 void cgrpc_metadata_array_destroy(cgrpc_metadata_array *array);
 size_t cgrpc_metadata_array_get_count(cgrpc_metadata_array *array);
-const char *cgrpc_metadata_array_get_key_at_index(cgrpc_metadata_array *array, size_t index);
-const char *cgrpc_metadata_array_get_value_at_index(cgrpc_metadata_array *array, size_t index);
+const char *cgrpc_metadata_array_copy_key_at_index(cgrpc_metadata_array *array, size_t index);
+const char *cgrpc_metadata_array_copy_value_at_index(cgrpc_metadata_array *array, size_t index);
+void cgrpc_metadata_free_copied_string(const char *string);
 void cgrpc_metadata_array_move_metadata(cgrpc_metadata_array *dest, cgrpc_metadata_array *src);
 void cgrpc_metadata_array_append_metadata(cgrpc_metadata_array *metadata, const char *key, const char *value);
 

Sources/CgRPC/shim/cgrpc.h

@@ -165,8 +165,9 @@ void cgrpc_operations_add_operation(cgrpc_operations *call, cgrpc_observer *obse
 cgrpc_metadata_array *cgrpc_metadata_array_create();
 void cgrpc_metadata_array_destroy(cgrpc_metadata_array *array);
 size_t cgrpc_metadata_array_get_count(cgrpc_metadata_array *array);
-const char *cgrpc_metadata_array_get_key_at_index(cgrpc_metadata_array *array, size_t index);
-const char *cgrpc_metadata_array_get_value_at_index(cgrpc_metadata_array *array, size_t index);
+const char *cgrpc_metadata_array_copy_key_at_index(cgrpc_metadata_array *array, size_t index);
+const char *cgrpc_metadata_array_copy_value_at_index(cgrpc_metadata_array *array, size_t index);
+void cgrpc_metadata_free_copied_string(const char *string);
 void cgrpc_metadata_array_move_metadata(cgrpc_metadata_array *dest, cgrpc_metadata_array *src);
 void cgrpc_metadata_array_append_metadata(cgrpc_metadata_array *metadata, const char *key, const char *value);
 

Sources/CgRPC/shim/metadata.c

@@ -33,12 +33,35 @@ size_t cgrpc_metadata_array_get_count(cgrpc_metadata_array *array) {
   return array->count;
 }
 
-const char *cgrpc_metadata_array_get_key_at_index(cgrpc_metadata_array *array, size_t index) {
-  return (const char *) GRPC_SLICE_START_PTR(array->metadata[index].key);
+const char *cgrpc_metadata_array_copy_key_at_index(cgrpc_metadata_array *array, size_t index) {
+  int length = GRPC_SLICE_LENGTH(array->metadata[index].key);
+  char *str = (char *) malloc(length + 1);
+  memcpy(str, GRPC_SLICE_START_PTR(array->metadata[index].key), length);
+  str[length] = 0;
+  return str;
 }
 
-const char *cgrpc_metadata_array_get_value_at_index(cgrpc_metadata_array *array, size_t index) {
-  return (const char *) GRPC_SLICE_START_PTR(array->metadata[index].value);
+const char *cgrpc_metadata_array_copy_value_at_index(cgrpc_metadata_array *array, size_t index) {
+  int length = GRPC_SLICE_LENGTH(array->metadata[index].value);
+  char *str = (char *) malloc(length + 1);
+  memcpy(str, GRPC_SLICE_START_PTR(array->metadata[index].value), length);
+  str[length] = 0;
+  return str;
+}
+
+void cgrpc_metadata_free_copied_string(const char *string) {
+	free(string);
+}
+
+int cgrpc_metadata_array_get_value_length_at_index(cgrpc_metadata_array *array, size_t index) {
+  return GRPC_SLICE_LENGTH(array->metadata[index].value);
+  /*
+  int length = GRPC_SLICE_LENGTH(array->metadata[index].value);
+  char *str = (char *) malloc(length + 1);
+  memcpy(str, GRPC_SLICE_START_PTR(array->metadata[index].value), length);
+  str[length] = 0;
+  return str;
+*/
 }
 
 void cgrpc_metadata_array_move_metadata(cgrpc_metadata_array *destination,

Sources/gRPC/Metadata.swift

@@ -71,21 +71,27 @@ public class Metadata : CustomStringConvertible, NSCopying {
   }
 
   public func key(_ index: Int) -> (String) {
-    if let key = String(cString:cgrpc_metadata_array_get_key_at_index(underlyingArray, index),
-                        encoding:String.Encoding.utf8) {
-      return key
-    } else {
-      return "<binary-metadata-key>"
+    if let string = cgrpc_metadata_array_copy_key_at_index(underlyingArray, index) {
+      defer {
+        cgrpc_metadata_free_copied_string(string)
+      }
+      if let key = String(cString:string, encoding:String.Encoding.utf8) {
+        return key
+      }
     }
+    return "<binary-metadata-key>"
   }
 
   public func value(_ index: Int) -> (String) {
-    if let value = String(cString:cgrpc_metadata_array_get_value_at_index(underlyingArray, index),
-                          encoding:String.Encoding.utf8) {
-      return value
-    } else {
-      return "<binary-metadata-value>"
+    if let string = cgrpc_metadata_array_copy_value_at_index(underlyingArray, index) {
+      defer {
+        cgrpc_metadata_free_copied_string(string)
+      }
+      if let value = String(cString:string, encoding:String.Encoding.utf8)  {
+        return value
+      }
     }
+    return "<binary-metadata-value>"
   }
 
   public func add(key:String, value:String) {

Tests/gRPCTests/GRPCTests.swift

@@ -1,98 +1,106 @@
-/*
- * Copyright 2017, 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 XCTest
-import Foundation
-import Dispatch
-@testable import gRPC
+ /*
+  * Copyright 2017, 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 XCTest
+ import Foundation
+ import Dispatch
+ @testable import gRPC
 
-func Log(_ message : String) {
+ func Log(_ message : String) {
   FileHandle.standardError.write((message + "\n").data(using:.utf8)!)
-}
+ }
 
-class gRPCTests: XCTestCase {
+ class gRPCTests: XCTestCase {
 
   func testBasicSanity() {
     gRPC.initialize()
-    let latch = CountDownLatch(2)
+
+    let sem = DispatchSemaphore(value: 0)
+
+    // start the server
     DispatchQueue.global().async() {
       do {
-        try server()
+        try runServer()
       } catch (let error) {
         XCTFail("server error \(error)")
       }
-      latch.signal()
+      sem.signal() // when the server exits, the test is finished
     }
-    DispatchQueue.global().async() {
-      do {
-        try client()
-      } catch (let error) {
-        XCTFail("client error \(error)")
-      }
-      latch.signal()
+
+    // run the client
+    do {
+      try runClient()
+    } catch (let error) {
+      XCTFail("client error \(error)")
     }
-    latch.wait()
+	
+    // stop the server
+    server.stop()
+	
+    // wait until the server has shut down
+    _ = sem.wait(timeout: DispatchTime.distantFuture)
   }
-}
+ }
 
-extension gRPCTests {
+ extension gRPCTests {
   static var allTests : [(String, (gRPCTests) -> () throws -> Void)] {
     return [
       ("testBasicSanity", testBasicSanity),
     ]
   }
-}
+ }
 
-let address = "localhost:8999"
-let host = "foo.test.google.fr"
-let clientText = "hello, server!"
-let serverText = "hello, client!"
-let initialClientMetadata =
+ let address = "localhost:8998"
+ let host = "foo.test.google.fr"
+ let clientText = "hello, server!"
+ let serverText = "hello, client!"
+ let initialClientMetadata =
   ["x": "xylophone",
    "y": "yu",
    "z": "zither"]
-let initialServerMetadata =
+ let initialServerMetadata =
   ["a": "Apple",
    "b": "Banana",
    "c": "Cherry"]
-let trailingServerMetadata =
+ let trailingServerMetadata =
   ["0": "zero",
    "1": "one",
    "2": "two"]
-let steps = 30
-let hello = "/hello"
-let goodbye = "/goodbye"
-let statusCode = 0
-let statusMessage = "OK"
+ let steps = 1
+ let hello = "/hello"
+ let statusCode = 0
+ let statusMessage = "OK"
 
-func verify_metadata(_ metadata: Metadata, expected: [String:String]) {
+ let server = gRPC.Server(address:address)
+
+ func verify_metadata(_ metadata: Metadata, expected: [String:String]) {
   XCTAssertGreaterThanOrEqual(metadata.count(), expected.count)
   for i in 0..<metadata.count() {
     if expected[metadata.key(i)] != nil {
       XCTAssertEqual(metadata.value(i), expected[metadata.key(i)])
     }
   }
-}
+ }
 
-func client() throws {
+ func runClient() throws {
   let message = clientText.data(using: .utf8)
   let channel = gRPC.Channel(address:address)
   channel.host = host
   for i in 0..<steps {
-    let latch = CountDownLatch(1)
-    let method = (i < steps-1) ? hello : goodbye
+      let sem = DispatchSemaphore(value: 0)
+    let method = hello
     let call = channel.makeCall(method)
     let metadata = Metadata(initialClientMetadata)
     try call.start(.unary, metadata:metadata, message:message) {
@@ -111,27 +119,23 @@ func client() throws {
       let trailingMetadata = response.trailingMetadata!
       verify_metadata(trailingMetadata, expected: trailingServerMetadata)
       // report completion
-      latch.signal()
+      sem.signal()
     }
     // wait for the call to complete
-    latch.wait()
+    _ = sem.wait(timeout: DispatchTime.distantFuture)
+    print("finished client step \(i)")
   }
-  usleep(500) // temporarily delay calls to the channel destructor
-}
+ }
 
-func server() throws {
-  let server = gRPC.Server(address:address)
+ func runServer() throws {
   var requestCount = 0
-  let latch = CountDownLatch(1)
+  let sem = DispatchSemaphore(value: 0)
   server.run() {(requestHandler) in
     do {
+      print("handling request \(requestHandler.method)")
       requestCount += 1
       XCTAssertEqual(requestHandler.host, host)
-      if (requestCount < steps) {
         XCTAssertEqual(requestHandler.method, hello)
-      } else {
-        XCTAssertEqual(requestHandler.method, goodbye)
-      }
       let initialMetadata = requestHandler.requestMetadata
       verify_metadata(initialMetadata, expected: initialClientMetadata)
       let initialMetadataToSend = Metadata(initialServerMetadata)
@@ -140,9 +144,6 @@ func server() throws {
         let messageString = String(data: messageData!, encoding: .utf8)
         XCTAssertEqual(messageString, clientText)
       }
-      if requestHandler.method == goodbye {
-        server.stop()
-      }
       let replyMessage = serverText
       let trailingMetadataToSend = Metadata(trailingServerMetadata)
       try requestHandler.sendResponse(message:replyMessage.data(using: .utf8)!,
@@ -154,9 +155,9 @@ func server() throws {
     }
   }
   server.onCompletion() {
-    // exit the server thread
-    latch.signal()
+    // return from runServer()
+    sem.signal()
   }
   // wait for the server to exit
-  latch.wait()
-}
+  _ = sem.wait(timeout: DispatchTime.distantFuture)
+ }