Added string value validation, modified key normalization and validation

3 files changed, 39 insertions, 10 deletions

diff --git a/src/node/src/metadata.js b/src/node/src/metadata.js
index 39514b2547..8e0884acea 100644
--- a/src/node/src/metadata.js
+++ b/src/node/src/metadata.js
@@ -49,10 +49,14 @@ function Metadata() {
 }
 
 function normalizeKey(key) {
-  return _.deburr(key).toLowerCase();
+  return key.toLowerCase();
 }
 
 function validate(key, value) {
+  if (!(/^[a-z\d-]+$/.test(key))) {
+    throw new Error('Metadata keys must be nonempty strings containing only ' +
+        'alphanumeric characters and hyphens');
+  }
   if (_.endsWith(key, '-bin')) {
     if (!(value instanceof Buffer)) {
       throw new Error('keys that end with \'-bin\' must have Buffer values');
@@ -62,6 +66,10 @@ function validate(key, value) {
       throw new Error(
           'keys that don\'t end with \'-bin\' must have String values');
     }
+    if (!(/^[\x20-\x7E]*$/.test(value))) {
+      throw new Error('Metadata string values can only contain printable ' +
+          'ASCII characters and space');
+    }
   }
 }
 
diff --git a/src/node/test/metadata_test.js b/src/node/test/metadata_test.js
index f1859b674d..227fa9c994 100644
--- a/src/node/test/metadata_test.js
+++ b/src/node/test/metadata_test.js
@@ -59,6 +59,19 @@ describe('Metadata', function() {
         metadata.set('key-bin', new Buffer('value'));
       });
     });
+    it('Rejects invalid keys', function() {
+      assert.throws(function() {
+        metadata.set('key$', 'value');
+      });
+      assert.throws(function() {
+        metadata.set('', 'value');
+      });
+    });
+    it('Rejects values with non-ASCII characters', function() {
+      assert.throws(function() {
+        metadata.set('key', 'résumé');
+      });
+    });
     it('Saves values that can be retrieved', function() {
       metadata.set('key', 'value');
       assert.deepEqual(metadata.get('key'), ['value']);
@@ -92,6 +105,14 @@ describe('Metadata', function() {
         metadata.add('key-bin', new Buffer('value'));
       });
     });
+    it('Rejects invalid keys', function() {
+      assert.throws(function() {
+        metadata.add('key$', 'value');
+      });
+      assert.throws(function() {
+        metadata.add('', 'value');
+      });
+    });
     it('Saves values that can be retrieved', function() {
       metadata.add('key', 'value');
       assert.deepEqual(metadata.get('key'), ['value']);
diff --git a/src/node/test/surface_test.js b/src/node/test/surface_test.js
index c7e63e9814..7c2a8d7258 100644
--- a/src/node/test/surface_test.js
+++ b/src/node/test/surface_test.js
@@ -359,7 +359,7 @@ describe('Other conditions', function() {
     test_service = test_proto.lookup('TestService');
     server = new grpc.Server();
     var trailer_metadata = new grpc.Metadata();
-    trailer_metadata.add('trailer_present', 'yes');
+    trailer_metadata.add('trailer-present', 'yes');
     server.addProtoService(test_service, {
       unary: function(call, cb) {
         var req = call.request;
@@ -514,7 +514,7 @@ describe('Other conditions', function() {
         assert.ifError(err);
       });
       call.on('status', function(status) {
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -523,7 +523,7 @@ describe('Other conditions', function() {
         assert(err);
       });
       call.on('status', function(status) {
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -535,7 +535,7 @@ describe('Other conditions', function() {
       call.write({error: false});
       call.end();
       call.on('status', function(status) {
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -547,7 +547,7 @@ describe('Other conditions', function() {
       call.write({error: true});
       call.end();
       call.on('status', function(status) {
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -556,7 +556,7 @@ describe('Other conditions', function() {
       call.on('data', function(){});
       call.on('status', function(status) {
         assert.strictEqual(status.code, grpc.status.OK);
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -564,7 +564,7 @@ describe('Other conditions', function() {
       var call = client.serverStream({error: true});
       call.on('data', function(){});
       call.on('error', function(error) {
-        assert.deepEqual(error.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(error.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -576,7 +576,7 @@ describe('Other conditions', function() {
       call.on('data', function(){});
       call.on('status', function(status) {
         assert.strictEqual(status.code, grpc.status.OK);
-        assert.deepEqual(status.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(status.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });
@@ -587,7 +587,7 @@ describe('Other conditions', function() {
       call.end();
       call.on('data', function(){});
       call.on('error', function(error) {
-        assert.deepEqual(error.metadata.get('trailer_present'), ['yes']);
+        assert.deepEqual(error.metadata.get('trailer-present'), ['yes']);
         done();
       });
     });