Merge pull request #9539 from markdroth/canonicalize_server_uri

Canonify server URI when setting the channel arg.

src/core/ext/client_channel/client_channel.c

@@ -546,10 +546,18 @@ static grpc_error *cc_init_channel_elem(grpc_exec_ctx *exec_ctx,
   arg = grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVER_URI);
   GPR_ASSERT(arg != NULL);
   GPR_ASSERT(arg->type == GRPC_ARG_STRING);
-  chand->server_name = gpr_strdup(arg->value.string);
+  grpc_uri *uri = grpc_uri_parse(arg->value.string, true);
+  if (uri == NULL) return GRPC_ERROR_CREATE("cannot parse server URI");
+  if (uri->path[0] == '\0') {
+    grpc_uri_destroy(uri);
+    return GRPC_ERROR_CREATE("server URI is missing path");
+  }
+  chand->server_name =
+      gpr_strdup(uri->path[0] == '/' ? uri->path + 1 : uri->path);
+  grpc_uri_destroy(uri);
   chand->proxy_name = grpc_get_http_proxy_server();
   char *name_to_resolve =
-      chand->proxy_name == NULL ? chand->server_name : chand->proxy_name;
+      chand->proxy_name == NULL ? arg->value.string : chand->proxy_name;
   chand->resolver = grpc_resolver_create(
       exec_ctx, name_to_resolve, args->channel_args, chand->interested_parties);
   if (chand->resolver == NULL) {

src/core/ext/transport/chttp2/client/insecure/channel_create.c

@@ -39,6 +39,7 @@
 #include <grpc/support/string_util.h>
 
 #include "src/core/ext/client_channel/client_channel.h"
+#include "src/core/ext/client_channel/resolver_registry.h"
 #include "src/core/ext/transport/chttp2/client/chttp2_connector.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/surface/api_trace.h"
@@ -63,12 +64,17 @@ static grpc_channel *client_channel_factory_create_channel(
     grpc_exec_ctx *exec_ctx, grpc_client_channel_factory *cc_factory,
     const char *target, grpc_client_channel_type type,
     const grpc_channel_args *args) {
+  if (target == NULL) {
+    gpr_log(GPR_ERROR, "cannot create channel with NULL target name");
+    return NULL;
+  }
   // Add channel arg containing the server URI.
   grpc_arg arg;
   arg.type = GRPC_ARG_STRING;
   arg.key = GRPC_ARG_SERVER_URI;
-  arg.value.string = (char *)target;
+  arg.value.string = grpc_resolver_factory_add_default_prefix_if_needed(target);
   grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args, &arg, 1);
+  gpr_free(arg.value.string);
   grpc_channel *channel = grpc_channel_create(exec_ctx, target, new_args,
                                               GRPC_CLIENT_CHANNEL, NULL);
   grpc_channel_args_destroy(exec_ctx, new_args);

src/core/ext/transport/chttp2/client/secure/secure_channel_create.c

@@ -39,6 +39,7 @@
 #include <grpc/support/string_util.h>
 
 #include "src/core/ext/client_channel/client_channel.h"
+#include "src/core/ext/client_channel/resolver_registry.h"
 #include "src/core/ext/transport/chttp2/client/chttp2_connector.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/security/credentials/credentials.h"
@@ -65,12 +66,17 @@ static grpc_channel *client_channel_factory_create_channel(
     grpc_exec_ctx *exec_ctx, grpc_client_channel_factory *cc_factory,
     const char *target, grpc_client_channel_type type,
     const grpc_channel_args *args) {
+  if (target == NULL) {
+    gpr_log(GPR_ERROR, "cannot create channel with NULL target name");
+    return NULL;
+  }
   // Add channel arg containing the server URI.
   grpc_arg arg;
   arg.type = GRPC_ARG_STRING;
   arg.key = GRPC_ARG_SERVER_URI;
-  arg.value.string = (char *)target;
+  arg.value.string = grpc_resolver_factory_add_default_prefix_if_needed(target);
   grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args, &arg, 1);
+  gpr_free(arg.value.string);
   grpc_channel *channel = grpc_channel_create(exec_ctx, target, new_args,
                                               GRPC_CLIENT_CHANNEL, NULL);
   grpc_channel_args_destroy(exec_ctx, new_args);
@@ -138,5 +144,8 @@ grpc_channel *grpc_secure_channel_create(grpc_channel_credentials *creds,
                                 "secure_client_channel_factory_create_channel");
   grpc_channel_args_destroy(&exec_ctx, new_args);
   grpc_exec_ctx_finish(&exec_ctx);
-  return channel; /* may be NULL */
+  return channel != NULL ? channel
+                         : grpc_lame_client_channel_create(
+                               target, GRPC_STATUS_INTERNAL,
+                               "Failed to create secure client channel");
 }

test/core/surface/secure_channel_create_test.c

@@ -43,45 +43,44 @@
 #include "test/core/util/test_config.h"
 
 void test_unknown_scheme_target(void) {
-  grpc_channel *chan;
-  grpc_channel_credentials *creds;
   grpc_resolver_registry_shutdown();
   grpc_resolver_registry_init();
-
-  creds = grpc_fake_transport_security_credentials_create();
-  chan = grpc_secure_channel_create(creds, "blah://blah", NULL, NULL);
-  GPR_ASSERT(chan == NULL);
+  grpc_channel_credentials *creds =
+      grpc_fake_transport_security_credentials_create();
+  grpc_channel *chan =
+      grpc_secure_channel_create(creds, "blah://blah", NULL, NULL);
+  grpc_channel_element *elem =
+      grpc_channel_stack_element(grpc_channel_get_channel_stack(chan), 0);
+  GPR_ASSERT(0 == strcmp(elem->filter->name, "lame-client"));
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+  GRPC_CHANNEL_INTERNAL_UNREF(&exec_ctx, chan, "test");
   grpc_channel_credentials_unref(&exec_ctx, creds);
   grpc_exec_ctx_finish(&exec_ctx);
 }
 
 void test_security_connector_already_in_arg(void) {
-  grpc_channel *chan;
-  grpc_channel_element *elem;
-  grpc_channel_args args;
   grpc_arg arg;
-  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-
   arg.type = GRPC_ARG_POINTER;
   arg.value.pointer.p = NULL;
   arg.key = GRPC_SECURITY_CONNECTOR_ARG;
+  grpc_channel_args args;
   args.num_args = 1;
   args.args = &arg;
-  chan = grpc_secure_channel_create(NULL, NULL, &args, NULL);
-  elem = grpc_channel_stack_element(grpc_channel_get_channel_stack(chan), 0);
+  grpc_channel *chan = grpc_secure_channel_create(NULL, NULL, &args, NULL);
+  grpc_channel_element *elem =
+      grpc_channel_stack_element(grpc_channel_get_channel_stack(chan), 0);
   GPR_ASSERT(0 == strcmp(elem->filter->name, "lame-client"));
+  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   GRPC_CHANNEL_INTERNAL_UNREF(&exec_ctx, chan, "test");
   grpc_exec_ctx_finish(&exec_ctx);
 }
 
 void test_null_creds(void) {
-  grpc_channel *chan;
-  grpc_channel_element *elem;
-  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-  chan = grpc_secure_channel_create(NULL, NULL, NULL, NULL);
-  elem = grpc_channel_stack_element(grpc_channel_get_channel_stack(chan), 0);
+  grpc_channel *chan = grpc_secure_channel_create(NULL, NULL, NULL, NULL);
+  grpc_channel_element *elem =
+      grpc_channel_stack_element(grpc_channel_get_channel_stack(chan), 0);
   GPR_ASSERT(0 == strcmp(elem->filter->name, "lame-client"));
+  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   GRPC_CHANNEL_INTERNAL_UNREF(&exec_ctx, chan, "test");
   grpc_exec_ctx_finish(&exec_ctx);
 }