From d9a50886041fbd57e228b0b41d259029a576d589 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 29 Jun 2015 15:57:36 -0700 Subject: [PATCH] SSL channel args work with client config again --- src/core/channel/channel_args.c | 18 +++++++++++++----- src/core/channel/channel_args.h | 4 +++- src/core/security/client_auth_filter.c | 1 - src/core/security/credentials.c | 2 +- src/core/security/server_secure_chttp2.c | 2 +- src/core/surface/channel_create.c | 6 ++++++ src/core/surface/secure_channel_create.c | 18 ++++++++++++++---- .../fixtures/chttp2_simple_ssl_fullstack.c | 2 +- .../chttp2_simple_ssl_fullstack_with_poll.c | 2 +- .../chttp2_simple_ssl_with_oauth2_fullstack.c | 2 +- 10 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 166d559a456..371da4210ee 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -62,7 +62,8 @@ static grpc_arg copy_arg(const grpc_arg *src) { } grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, - const grpc_arg *to_add) { + const grpc_arg *to_add, + size_t num_to_add) { grpc_channel_args *dst = gpr_malloc(sizeof(grpc_channel_args)); size_t i; size_t src_num_args = (src == NULL) ? 0 : src->num_args; @@ -71,17 +72,24 @@ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, dst->args = NULL; return dst; } - dst->num_args = src_num_args + ((to_add == NULL) ? 0 : 1); + dst->num_args = src_num_args + num_to_add; dst->args = gpr_malloc(sizeof(grpc_arg) * dst->num_args); for (i = 0; i < src_num_args; i++) { dst->args[i] = copy_arg(&src->args[i]); } - if (to_add != NULL) dst->args[src_num_args] = copy_arg(to_add); + for (i = 0; i < num_to_add; i++) { + dst->args[i + src_num_args] = copy_arg(&to_add[i]); + } return dst; } grpc_channel_args *grpc_channel_args_copy(const grpc_channel_args *src) { - return grpc_channel_args_copy_and_add(src, NULL); + return grpc_channel_args_copy_and_add(src, NULL, 0); +} + +grpc_channel_args *grpc_channel_args_merge(const grpc_channel_args *a, + const grpc_channel_args *b) { + return grpc_channel_args_copy_and_add(a, b->args, b->num_args); } void grpc_channel_args_destroy(grpc_channel_args *a) { @@ -137,5 +145,5 @@ void grpc_channel_args_set_compression_level( tmp.type = GRPC_ARG_INTEGER; tmp.key = GRPC_COMPRESSION_LEVEL_ARG; tmp.value.integer = level; - *a = grpc_channel_args_copy_and_add(*a, &tmp); + *a = grpc_channel_args_copy_and_add(*a, &tmp, 1); } diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index 41f3a554d6c..27ad57b3e8f 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -43,8 +43,10 @@ grpc_channel_args *grpc_channel_args_copy(const grpc_channel_args *src); /** Copy some arguments and add the to_add parameter in the end. If to_add is NULL, it is equivalent to call grpc_channel_args_copy. */ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, - const grpc_arg *to_add); + const grpc_arg *to_add, + size_t num_to_add); +/** Copy args from a then args from b into a new channel args */ grpc_channel_args *grpc_channel_args_merge(const grpc_channel_args *a, const grpc_channel_args *b); /** Destroy arguments created by grpc_channel_args_copy */ diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c index 0bd370e457d..93bf846978c 100644 --- a/src/core/security/client_auth_filter.c +++ b/src/core/security/client_auth_filter.c @@ -291,7 +291,6 @@ static void init_channel_elem(grpc_channel_element *elem,grpc_channel *master, /* The first and the last filters tend to be implemented differently to handle the case that there's no 'next' filter to call on the up or down path */ - GPR_ASSERT(!is_first); GPR_ASSERT(!is_last); GPR_ASSERT(sc != NULL); diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index cf663faf2d0..e79e9ce3516 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -225,7 +225,7 @@ static grpc_security_status ssl_create_security_connector( arg.type = GRPC_ARG_STRING; arg.key = GRPC_ARG_HTTP2_SCHEME; arg.value.string = "https"; - *new_args = grpc_channel_args_copy_and_add(args, &arg); + *new_args = grpc_channel_args_copy_and_add(args, &arg, 1); return status; } diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index d76bd86337d..018ec3d1d7f 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -83,7 +83,7 @@ static void setup_transport(void *statep, grpc_server_secure_state *state = statep; grpc_arg connector_arg = grpc_security_connector_to_arg(state->sc); grpc_channel_args *args_copy = grpc_channel_args_copy_and_add( - grpc_server_get_channel_args(state->server), &connector_arg); + grpc_server_get_channel_args(state->server), &connector_arg, 1); grpc_server_setup_transport(state->server, transport, extra_filters, GPR_ARRAY_SIZE(extra_filters), mdctx, args_copy); diff --git a/src/core/surface/channel_create.c b/src/core/surface/channel_create.c index 0d756a131e4..6b559e1fb42 100644 --- a/src/core/surface/channel_create.c +++ b/src/core/surface/channel_create.c @@ -102,6 +102,7 @@ typedef struct { grpc_subchannel_factory base; gpr_refcount refs; grpc_mdctx *mdctx; + grpc_channel_args *merge_args; } subchannel_factory; static void subchannel_factory_ref(grpc_subchannel_factory *scf) { @@ -120,13 +121,17 @@ static void subchannel_factory_unref(grpc_subchannel_factory *scf) { static grpc_subchannel *subchannel_factory_create_subchannel(grpc_subchannel_factory *scf, grpc_subchannel_args *args) { subchannel_factory *f = (subchannel_factory *)scf; connector *c = gpr_malloc(sizeof(*c)); + grpc_channel_args *final_args = + grpc_channel_args_merge(args->args, f->merge_args); grpc_subchannel *s; memset(c, 0, sizeof(*c)); c->base.vtable = &connector_vtable; gpr_ref_init(&c->refs, 1); args->mdctx = f->mdctx; + args->args = final_args; s = grpc_subchannel_create(&c->base, args); grpc_connector_unref(&c->base); + grpc_channel_args_destroy(final_args); return s; } @@ -157,6 +162,7 @@ grpc_channel *grpc_channel_create(const char *target, gpr_ref_init(&f->refs, 1); grpc_mdctx_ref(mdctx); f->mdctx = mdctx; + f->merge_args = grpc_channel_args_copy(args); resolver = grpc_resolver_create(target, &f->base); if (!resolver) { return NULL; diff --git a/src/core/surface/secure_channel_create.c b/src/core/surface/secure_channel_create.c index ad11c3954be..b60f390370b 100644 --- a/src/core/surface/secure_channel_create.c +++ b/src/core/surface/secure_channel_create.c @@ -81,14 +81,18 @@ static void on_secure_transport_setup_done(void *arg, if (status != GRPC_SECURITY_OK) { gpr_log(GPR_ERROR, "Secure transport setup failed with error %d.", status); memset(c->result, 0, sizeof(*c->result)); - notify = c->notify; - c->notify = NULL; - grpc_iomgr_add_callback(notify); } else { c->result->transport = grpc_create_chttp2_transport( c->args.channel_args, secure_endpoint, NULL, 0, c->args.metadata_context, 1); + c->result->filters = gpr_malloc(sizeof(grpc_channel_filter *) * 2); + c->result->filters[0] = &grpc_client_auth_filter; + c->result->filters[1] = &grpc_http_client_filter; + c->result->num_filters = 2; } + notify = c->notify; + c->notify = NULL; + grpc_iomgr_add_callback(notify); } static void connected(void *arg, grpc_endpoint *tcp) { @@ -123,6 +127,7 @@ typedef struct { grpc_subchannel_factory base; gpr_refcount refs; grpc_mdctx *mdctx; + grpc_channel_args *merge_args; grpc_channel_security_connector *security_connector; } subchannel_factory; @@ -142,14 +147,18 @@ static void subchannel_factory_unref(grpc_subchannel_factory *scf) { static grpc_subchannel *subchannel_factory_create_subchannel(grpc_subchannel_factory *scf, grpc_subchannel_args *args) { subchannel_factory *f = (subchannel_factory *)scf; connector *c = gpr_malloc(sizeof(*c)); + grpc_channel_args *final_args = + grpc_channel_args_merge(args->args, f->merge_args); grpc_subchannel *s; memset(c, 0, sizeof(*c)); c->base.vtable = &connector_vtable; c->security_connector = f->security_connector; gpr_ref_init(&c->refs, 1); args->mdctx = f->mdctx; + args->args = final_args; s = grpc_subchannel_create(&c->base, args); grpc_connector_unref(&c->base); + grpc_channel_args_destroy(final_args); return s; } @@ -334,7 +343,7 @@ grpc_channel *grpc_secure_channel_create(grpc_credentials *creds, connector_arg = grpc_security_connector_to_arg(&connector->base); args_copy = grpc_channel_args_copy_and_add( new_args_from_connector != NULL ? new_args_from_connector : args, - &connector_arg); + &connector_arg, 1); /* TODO(census) if (grpc_channel_args_is_census_enabled(args)) { filters[n++] = &grpc_client_census_filter; @@ -347,6 +356,7 @@ grpc_channel *grpc_secure_channel_create(grpc_credentials *creds, gpr_ref_init(&f->refs, 1); f->mdctx = mdctx; f->security_connector = connector; + f->merge_args = grpc_channel_args_copy(args_copy); resolver = grpc_resolver_create(target, &f->base); if (!resolver) { return NULL; diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c index 237d0727021..73a36116fb1 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c @@ -105,7 +105,7 @@ static void chttp2_init_client_simple_ssl_secure_fullstack( GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, {"foo.test.google.fr"}}; grpc_channel_args *new_client_args = - grpc_channel_args_copy_and_add(client_args, &ssl_name_override); + grpc_channel_args_copy_and_add(client_args, &ssl_name_override, 1); chttp2_init_client_secure_fullstack(f, new_client_args, ssl_creds); grpc_channel_args_destroy(new_client_args); } diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c index ff5642642de..b1ac3e535f1 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c @@ -105,7 +105,7 @@ static void chttp2_init_client_simple_ssl_secure_fullstack( GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, {"foo.test.google.fr"}}; grpc_channel_args *new_client_args = - grpc_channel_args_copy_and_add(client_args, &ssl_name_override); + grpc_channel_args_copy_and_add(client_args, &ssl_name_override, 1); chttp2_init_client_secure_fullstack(f, new_client_args, ssl_creds); grpc_channel_args_destroy(new_client_args); } diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c b/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c index d4bb5d3ef56..de418bf7ee0 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c @@ -108,7 +108,7 @@ static void chttp2_init_client_simple_ssl_with_oauth2_secure_fullstack( GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, {"foo.test.google.fr"}}; grpc_channel_args *new_client_args = - grpc_channel_args_copy_and_add(client_args, &ssl_name_override); + grpc_channel_args_copy_and_add(client_args, &ssl_name_override, 1); chttp2_init_client_secure_fullstack(f, new_client_args, ssl_oauth2_creds); grpc_channel_args_destroy(new_client_args); grpc_credentials_release(ssl_creds);