Merge pull request #5328 from murgatroid99/node_jwt_interop_fix
Node: fix metadata validation bug, improve error reporting
diff --git a/src/node/ext/call_credentials.cc b/src/node/ext/call_credentials.cc
index 91acb86..98696db 100644
--- a/src/node/ext/call_credentials.cc
+++ b/src/node/ext/call_credentials.cc
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -187,7 +187,8 @@
shared_ptr<Resources> resources(new Resources);
grpc_status_code code = static_cast<grpc_status_code>(
Nan::To<uint32_t>(info[0]).FromJust());
- char *details = *Utf8String(info[1]);
+ Utf8String details_utf8_str(info[1]);
+ char *details = *details_utf8_str;
grpc_metadata_array array;
if (!CreateMetadataArray(Nan::To<Object>(info[2]).ToLocalChecked(),
&array, resources)){
diff --git a/src/node/ext/node_grpc.cc b/src/node/ext/node_grpc.cc
index 654c5ae..0c71b2d 100644
--- a/src/node/ext/node_grpc.cc
+++ b/src/node/ext/node_grpc.cc
@@ -237,7 +237,8 @@
"headerKeyIsLegal's argument must be a string");
}
Local<String> key = Nan::To<String>(info[0]).ToLocalChecked();
- char *key_str = *Nan::Utf8String(key);
+ Nan::Utf8String key_utf8_str(key);
+ char *key_str = *key_utf8_str;
info.GetReturnValue().Set(static_cast<bool>(
grpc_header_key_is_legal(key_str, static_cast<size_t>(key->Length()))));
}
@@ -248,7 +249,8 @@
"metadataNonbinValueIsLegal's argument must be a string");
}
Local<String> value = Nan::To<String>(info[0]).ToLocalChecked();
- char *value_str = *Nan::Utf8String(value);
+ Nan::Utf8String value_utf8_str(value);
+ char *value_str = *value_utf8_str;
info.GetReturnValue().Set(static_cast<bool>(
grpc_header_nonbin_value_is_legal(
value_str, static_cast<size_t>(value->Length()))));
@@ -260,7 +262,8 @@
"metadataKeyIsLegal's argument must be a string");
}
Local<String> key = Nan::To<String>(info[0]).ToLocalChecked();
- char *key_str = *Nan::Utf8String(key);
+ Nan::Utf8String key_utf8_str(key);
+ char *key_str = *key_utf8_str;
info.GetReturnValue().Set(static_cast<bool>(
grpc_is_binary_header(key_str, static_cast<size_t>(key->Length()))));
}
diff --git a/src/node/src/credentials.js b/src/node/src/credentials.js
index 710ab6d..1d73723 100644
--- a/src/node/src/credentials.js
+++ b/src/node/src/credentials.js
@@ -98,6 +98,8 @@
message = error.message;
if (error.hasOwnProperty('code')) {
code = error.code;
+ } else {
+ code = grpc.status.UNAUTHENTICATED;
}
if (!metadata) {
metadata = new Metadata();
@@ -116,13 +118,16 @@
exports.createFromGoogleCredential = function(google_credential) {
return exports.createFromMetadataGenerator(function(auth_context, callback) {
var service_url = auth_context.service_url;
+ console.log('Service URL:', service_url);
google_credential.getRequestMetadata(service_url, function(err, header) {
if (err) {
+ console.log('Auth error:', err);
callback(err);
return;
}
var metadata = new Metadata();
metadata.add('authorization', header.Authorization);
+ console.log(header.Authorization);
callback(null, metadata);
});
});
diff --git a/src/node/src/metadata.js b/src/node/src/metadata.js
index 51a9f8a..33d7ea1 100644
--- a/src/node/src/metadata.js
+++ b/src/node/src/metadata.js
@@ -64,7 +64,7 @@
if (grpc.metadataKeyIsLegal(key)) {
return key;
} else {
- throw new Error('Metadata key contains illegal characters');
+ throw new Error('Metadata key"' + key + '" contains illegal characters');
}
}
@@ -79,7 +79,8 @@
'keys that don\'t end with \'-bin\' must have String values');
}
if (!grpc.metadataNonbinValueIsLegal(value)) {
- throw new Error('Metadata string value contains illegal characters');
+ throw new Error('Metadata string value "' + value +
+ '" contains illegal characters');
}
}
}
diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py
index 76be932..df3ab90 100755
--- a/tools/run_tests/run_interop_tests.py
+++ b/tools/run_tests/run_interop_tests.py
@@ -422,7 +422,7 @@
time.sleep(2)
-def cloud_to_prod_jobspec(language, test_case, server_host_name,
+def cloud_to_prod_jobspec(language, test_case, server_host_name,
server_host_detail, docker_image=None, auth=False):
"""Creates jobspec for cloud-to-prod interop test"""
container_name = None
@@ -441,7 +441,7 @@
cwd = language.client_cwd
if docker_image:
- container_name = dockerjob.random_name('interop_client_%s' %
+ container_name = dockerjob.random_name('interop_client_%s' %
language.safename)
cmdline = docker_run_cmdline(cmdline,
image=docker_image,
@@ -457,7 +457,7 @@
cmdline=cmdline,
cwd=cwd,
environ=environ,
- shortname='%s:%s:%s:%s' % (suite_name, server_host_name, language,
+ shortname='%s:%s:%s:%s' % (suite_name, server_host_name, language,
test_case),
timeout_seconds=90,
flake_retries=5 if args.allow_flakes else 0,
@@ -575,18 +575,18 @@
'percent': 1.0 * passed / (passed + failed)
}
-# A dictionary of prod servers to test.
+# A dictionary of prod servers to test.
# Format: server_name: (server_host, server_host_override, errors_allowed)
# TODO(adelez): implement logic for errors_allowed where if the indicated tests
# fail, they don't impact the overall test result.
prod_servers = {
- 'default': ('grpc-test.sandbox.googleapis.com',
+ 'default': ('grpc-test.sandbox.googleapis.com',
'grpc-test.sandbox.googleapis.com', False),
- 'gateway_v2': ('grpc-test2.sandbox.googleapis.com',
+ 'gateway_v2': ('grpc-test2.sandbox.googleapis.com',
'grpc-test2.sandbox.googleapis.com', True),
- 'cloud_gateway': ('216.239.32.255', 'grpc-test.sandbox.googleapis.com',
+ 'cloud_gateway': ('216.239.32.255', 'grpc-test.sandbox.googleapis.com',
False),
- 'cloud_gateway_v2': ('216.239.32.255', 'grpc-test2.sandbox.googleapis.com',
+ 'cloud_gateway_v2': ('216.239.32.255', 'grpc-test2.sandbox.googleapis.com',
True)
}
@@ -720,7 +720,7 @@
if not test_case in language.unimplemented_test_cases():
if not test_case in _SKIP_ADVANCED + _SKIP_COMPRESSION:
test_job = cloud_to_prod_jobspec(
- language, test_case, server_host_name,
+ language, test_case, server_host_name,
prod_servers[server_host_name],
docker_image=docker_images.get(str(language)))
jobs.append(test_job)
@@ -728,7 +728,7 @@
if args.http2_interop:
for test_case in _HTTP2_TEST_CASES:
test_job = cloud_to_prod_jobspec(
- http2Interop, test_case, server_host_name,
+ http2Interop, test_case, server_host_name,
prod_servers[server_host_name],
docker_image=docker_images.get(str(http2Interop)))
jobs.append(test_job)
@@ -739,7 +739,7 @@
for test_case in _AUTH_TEST_CASES:
if not test_case in language.unimplemented_test_cases():
test_job = cloud_to_prod_jobspec(
- language, test_case, server_host_name,
+ language, test_case, server_host_name,
prod_servers[server_host_name],
docker_image=docker_images.get(str(language)), auth=True)
jobs.append(test_job)
@@ -802,7 +802,7 @@
report_utils.render_interop_html_report(
set([str(l) for l in languages]), servers, _TEST_CASES, _AUTH_TEST_CASES,
_HTTP2_TEST_CASES, resultset, num_failures,
- args.cloud_to_prod_auth or args.cloud_to_prod, args.prod_servers,
+ args.cloud_to_prod_auth or args.cloud_to_prod, args.prod_servers,
args.http2_interop)
finally: