Improve metadata handling.

Namely:

- Add support for metadata arrays with more than 10 elements.
- Fix a memory leak in `cgrpc_metadata_array_destroy`.
- Use `gpr_malloc` to allocate the metadata array, as that is also used to free it. (What's the difference anyway? Should we use gpr_malloc everywhere?)
- Remove the unused `MetadataPair` struct.
- Remove support for NSCopying (would only be relevant for Objective-C interop, which we neither have nor need) and make `copy()` return an object of type `Metadata` (instead of `Any`).
- Slightly improve the `verify_metadata` test method.

Sources/CgRPC/shim/metadata.c

@@ -13,6 +13,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+#include <grpc/support/alloc.h>
+
 #include "internal.h"
 #include "cgrpc.h"
 
@@ -20,13 +23,14 @@
 #include "string.h"
 
 cgrpc_metadata_array *cgrpc_metadata_array_create() {
-  cgrpc_metadata_array *metadata = (cgrpc_metadata_array *) malloc(sizeof(cgrpc_metadata_array));
-  memset(metadata, 0, sizeof(cgrpc_metadata_array));
+  cgrpc_metadata_array *metadata = (cgrpc_metadata_array *) gpr_malloc(sizeof(cgrpc_metadata_array));
+  grpc_metadata_array_init(metadata);
   return metadata;
 }
 
 void cgrpc_metadata_array_destroy(cgrpc_metadata_array *array) {
   grpc_metadata_array_destroy(array);
+  gpr_free(array);
 }
 
 size_t cgrpc_metadata_array_get_count(cgrpc_metadata_array *array) {
@@ -49,19 +53,8 @@ char *cgrpc_metadata_array_copy_value_at_index(cgrpc_metadata_array *array, size
   return str;
 }
 
-size_t 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,
-                                       cgrpc_metadata_array *source) {
+                                        cgrpc_metadata_array *source) {
   destination->count = source->count;
   destination->capacity = source->capacity;
   destination->metadata = source->metadata;
@@ -72,15 +65,22 @@ void cgrpc_metadata_array_move_metadata(cgrpc_metadata_array *destination,
 }
 
 void cgrpc_metadata_array_append_metadata(cgrpc_metadata_array *metadata, const char *key, const char *value) {
-  if (!metadata->count) {
-    metadata->metadata = (grpc_metadata *) malloc(10 * sizeof(grpc_metadata));
-    metadata->count = 0;
-    metadata->capacity = 10;
-  }
-  if (metadata->count < metadata->capacity) {
-    size_t i = metadata->count;
-    metadata->metadata[i].key = grpc_slice_from_copied_string(key);
-    metadata->metadata[i].value = grpc_slice_from_copied_string(value);
-    metadata->count++;
+  if (metadata->count >= metadata->capacity) {
+    size_t new_capacity = 2 * metadata->capacity;
+    if (new_capacity < 10) {
+      new_capacity = 10;
+    }
+    
+    if (metadata->metadata != NULL) {
+      metadata->metadata = gpr_realloc(metadata->metadata, new_capacity * sizeof(grpc_metadata));
+    } else {
+      metadata->metadata = gpr_malloc(new_capacity * sizeof(grpc_metadata));
+    }
+    metadata->capacity = new_capacity;
   }
+
+  size_t i = metadata->count;
+  metadata->metadata[i].key = grpc_slice_from_copied_string(key);
+  metadata->metadata[i].value = grpc_slice_from_copied_string(value);
+  metadata->count++;
 }

Sources/gRPC/Call.swift

@@ -218,7 +218,7 @@ public class Call {
         throw CallError.invalidMessage
       }
       operations = [
-        .sendInitialMetadata(metadata.copy() as! Metadata),
+        .sendInitialMetadata(metadata.copy()),
         .receiveInitialMetadata,
         .receiveStatusOnClient,
         .sendMessage(ByteBuffer(data: message)),
@@ -230,14 +230,14 @@ public class Call {
         throw CallError.invalidMessage
       }
       operations = [
-        .sendInitialMetadata(metadata.copy() as! Metadata),
+        .sendInitialMetadata(metadata.copy()),
         .receiveInitialMetadata,
         .sendMessage(ByteBuffer(data: message)),
         .sendCloseFromClient
       ]
     case .clientStreaming, .bidiStreaming:
       operations = [
-        .sendInitialMetadata(metadata.copy() as! Metadata),
+        .sendInitialMetadata(metadata.copy()),
         .receiveInitialMetadata
       ]
     }

Sources/gRPC/Metadata.swift

@@ -18,18 +18,8 @@
 #endif
 import Foundation // for String.Encoding
 
-/// An item of metadata
-private struct MetadataPair {
-  var key: String
-  var value: String
-  init(key: String, value: String) {
-    self.key = key
-    self.value = value
-  }
-}
-
 /// Metadata sent with gRPC messages
-public class Metadata: CustomStringConvertible, NSCopying {
+public class Metadata: CustomStringConvertible {
   /// Pointer to underlying C representation
   var underlyingArray: UnsafeMutableRawPointer
 
@@ -41,23 +31,10 @@ public class Metadata: CustomStringConvertible, NSCopying {
     underlyingArray = cgrpc_metadata_array_create()
   }
 
-  public init(_ pairs: [[String: String]]) {
-    underlyingArray = cgrpc_metadata_array_create()
-    for pair in pairs {
-      for key in pair.keys {
-        if let value = pair[key] {
-          add(key: key, value: value)
-        }
-      }
-    }
-  }
-
   public init(_ pairs: [String: String]) {
     underlyingArray = cgrpc_metadata_array_create()
-    for key in pairs.keys {
-      if let value = pairs[key] {
-        add(key: key, value: value)
-      }
+    for (key, value) in pairs {
+      add(key: key, value: value)
     }
   }
 
@@ -68,49 +45,47 @@ public class Metadata: CustomStringConvertible, NSCopying {
   public func count() -> Int {
     return cgrpc_metadata_array_get_count(underlyingArray)
   }
-
-  public func key(_ index: Int) -> String {
-    if let string = cgrpc_metadata_array_copy_key_at_index(underlyingArray, index) {
-      defer {
-        cgrpc_free_copied_string(string)
-      }
-      if let key = String(cString: string, encoding: String.Encoding.utf8) {
-        return key
-      }
-    }
-    return "<binary-metadata-key>"
+  
+  // Returns `nil` for non-UTF8 metadata key strings.
+  public func key(_ index: Int) -> String? {
+    // We actually know that this method will never return nil,
+    // so we can forcibly unwrap the result. (Also below.)
+    let keyData = cgrpc_metadata_array_copy_key_at_index(underlyingArray, index)!
+    defer { cgrpc_free_copied_string(keyData) }
+    return String(cString: keyData, encoding: String.Encoding.utf8)
   }
-
-  public func value(_ index: Int) -> String {
-    if let string = cgrpc_metadata_array_copy_value_at_index(underlyingArray, index) {
-      defer {
-        cgrpc_free_copied_string(string)
-      }
-      if let value = String(cString: string, encoding: String.Encoding.utf8) {
-        return value
-      }
-    }
-    return "<binary-metadata-value>"
+  
+  // Returns `nil` for non-UTF8 metadata value strings.
+  public func value(_ index: Int) -> String? {
+    // We actually know that this method will never return nil,
+    // so we can forcibly unwrap the result. (Also below.)
+    let valueData = cgrpc_metadata_array_copy_value_at_index(underlyingArray, index)!
+    defer { cgrpc_free_copied_string(valueData) }
+    return String(cString: valueData, encoding: String.Encoding.utf8)
   }
-
+  
   public func add(key: String, value: String) {
     cgrpc_metadata_array_append_metadata(underlyingArray, key, value)
   }
-
+  
   public var description: String {
     var result = ""
     for i in 0..<count() {
       let key = self.key(i)
       let value = self.value(i)
-      result += key + ":" + value + "\n"
+      result += (key ?? "(nil)") + ":" + (value ?? "(nil)") + "\n"
     }
     return result
   }
-
-  public func copy(with _: NSZone? = nil) -> Any {
+  
+  public func copy() -> Metadata {
     let copy = Metadata()
-    for i in 0..<count() {
-      copy.add(key: key(i), value: value(i))
+    for index in 0..<count() {
+      let keyData = cgrpc_metadata_array_copy_key_at_index(underlyingArray, index)!
+      defer { cgrpc_free_copied_string(keyData) }
+      let valueData = cgrpc_metadata_array_copy_value_at_index(underlyingArray, index)!
+      defer { cgrpc_free_copied_string(valueData) }
+      cgrpc_metadata_array_append_metadata(copy.underlyingArray, keyData, valueData)
     }
     return copy
   }

Tests/gRPCTests/GRPCTests.swift

@@ -53,9 +53,21 @@ let initialServerMetadata =
   ]
 let trailingServerMetadata =
   [
+    // We have more than ten entries here to ensure that even large metadata entries work
+    // and aren't limited by e.g. a fixed-size entry buffer.
     "0": "zero",
     "1": "one",
-    "2": "two"
+    "2": "two",
+    "3": "three",
+    "4": "four",
+    "5": "five",
+    "6": "six",
+    "7": "seven",
+    "8": "eight",
+    "9": "nine",
+    "10": "ten",
+    "11": "eleven",
+    "12": "twelve"
   ]
 let steps = 10
 let hello = "/hello"
@@ -109,13 +121,16 @@ func runTest(useSSL: Bool) {
   _ = serverRunningSemaphore.wait(timeout: DispatchTime.distantFuture)
 }
 
-func verify_metadata(_ metadata: Metadata, expected: [String: String]) {
+func verify_metadata(_ metadata: Metadata, expected: [String: String], file: StaticString = #file, line: UInt = #line) {
   XCTAssertGreaterThanOrEqual(metadata.count(), expected.count)
+  var allPresentKeys = Set<String>()
   for i in 0..<metadata.count() {
-    if expected[metadata.key(i)] != nil {
-      XCTAssertEqual(metadata.value(i), expected[metadata.key(i)])
-    }
+    guard let expectedValue = expected[metadata.key(i)!]
+      else { continue }
+    allPresentKeys.insert(metadata.key(i)!)
+    XCTAssertEqual(metadata.value(i), expectedValue, file: file, line: line)
   }
+  XCTAssertEqual(allPresentKeys.sorted(), expected.keys.sorted(), file: file, line: line)
 }
 
 func runClient(useSSL: Bool) throws {
@@ -136,7 +151,7 @@ func runClient(useSSL: Bool) throws {
   }
 
   channel.host = host
-  for i in 0..<steps {
+  for _ in 0..<steps {
     let sem = DispatchSemaphore(value: 0)
     let method = hello
     let call = channel.makeCall(method)