From 28ea7e26309b277f9ba9a7578388b7996f5acbec Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 25 Jul 2016 11:06:22 -0700 Subject: Use URI query string instead of channel arg to indicate use of proxy. --- .../ext/client_config/http_connect_handshaker.c | 47 ++++++++++++---------- .../ext/client_config/http_connect_handshaker.h | 11 ++--- src/core/ext/client_config/resolver_registry.c | 6 ++- src/core/ext/client_config/resolver_registry.h | 7 +++- src/core/ext/client_config/uri_parser.c | 4 +- src/core/ext/resolver/dns/native/dns_resolver.c | 27 +++++++------ .../chttp2/client/insecure/channel_create.c | 11 +++-- .../chttp2/client/secure/secure_channel_create.c | 11 +++-- 8 files changed, 70 insertions(+), 54 deletions(-) (limited to 'src') diff --git a/src/core/ext/client_config/http_connect_handshaker.c b/src/core/ext/client_config/http_connect_handshaker.c index f7fa931788..a5cb6d1047 100644 --- a/src/core/ext/client_config/http_connect_handshaker.c +++ b/src/core/ext/client_config/http_connect_handshaker.c @@ -46,7 +46,6 @@ typedef struct http_connect_handshaker { // Base class. Must be first. grpc_handshaker base; - // These pointers are borrowed, we don't own them. char* proxy_server; char* server_name; @@ -69,8 +68,14 @@ typedef struct http_connect_handshaker { static void on_write_done(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { http_connect_handshaker* h = arg; - grpc_endpoint_read(exec_ctx, h->endpoint, &h->response_buffer, - &h->response_read_closure); + if (error != GRPC_ERROR_NONE) { + // If the write failed, invoke the callback immediately with the error. + h->cb(exec_ctx, h->endpoint, h->args, h->user_data, GRPC_ERROR_REF(error)); + } else { + // Otherwise, read the response. + grpc_endpoint_read(exec_ctx, h->endpoint, &h->response_buffer, + &h->response_read_closure); + } } // Callback invoked for reading HTTP CONNECT response. @@ -102,6 +107,14 @@ static void on_read_done(grpc_exec_ctx* exec_ctx, void* arg, &h->response_read_closure); return; } + // Make sure we got a 2xx response. + if (h->http_response.status < 200 || h->http_response.status >= 300) { + char* msg; + gpr_asprintf(&msg, "HTTP proxy returned response code %d", + h->http_response.status); + error = GRPC_ERROR_CREATE(msg); + gpr_free(msg); + } } done: // Invoke handshake-done callback. @@ -115,6 +128,8 @@ static void on_read_done(grpc_exec_ctx* exec_ctx, void* arg, static void http_connect_handshaker_destroy(grpc_exec_ctx* exec_ctx, grpc_handshaker* handshaker) { http_connect_handshaker* h = (http_connect_handshaker*)handshaker; + gpr_free(h->proxy_server); + gpr_free(h->server_name); gpr_slice_buffer_destroy(&h->request_buffer); gpr_slice_buffer_destroy(&h->response_buffer); grpc_http_parser_destroy(&h->http_parser); @@ -145,6 +160,8 @@ static void http_connect_handshaker_do_handshake( grpc_http_parser_init(&h->http_parser, GRPC_HTTP_RESPONSE, &h->http_response); // Send HTTP CONNECT request. + gpr_log(GPR_INFO, "Connecting to server %s via HTTP proxy %s", + h->server_name, h->proxy_server); grpc_httpcli_request request; memset(&request, 0, sizeof(request)); request.host = gpr_strdup(h->proxy_server); @@ -161,27 +178,15 @@ static const struct grpc_handshaker_vtable http_connect_handshaker_vtable = { http_connect_handshaker_destroy, http_connect_handshaker_shutdown, http_connect_handshaker_do_handshake}; -char* grpc_get_http_connect_proxy_server_from_args(grpc_channel_args* args) { - for (size_t i = 0; i < args->num_args; ++i) { - if (strcmp(args->args[i].key, GRPC_ARG_HTTP_CONNECT_PROXY_SERVER) == 0) { - if (args->args[i].type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "%s: must be a string", - GRPC_ARG_HTTP_CONNECT_PROXY_SERVER); - break; - } - return args->args[i].value.string; - } - } - return NULL; -} - -grpc_handshaker* grpc_http_connect_handshaker_create(char* proxy_server, - char* server_name) { +grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server, + const char* server_name) { + GPR_ASSERT(proxy_server != NULL); + GPR_ASSERT(server_name != NULL); http_connect_handshaker* handshaker = gpr_malloc(sizeof(http_connect_handshaker)); memset(handshaker, 0, sizeof(*handshaker)); grpc_handshaker_init(&http_connect_handshaker_vtable, &handshaker->base); - handshaker->proxy_server = proxy_server; - handshaker->server_name = server_name; + handshaker->proxy_server = gpr_strdup(proxy_server); + handshaker->server_name = gpr_strdup(server_name); return (grpc_handshaker*)handshaker; } diff --git a/src/core/ext/client_config/http_connect_handshaker.h b/src/core/ext/client_config/http_connect_handshaker.h index feb2ec13f0..146ef9369a 100644 --- a/src/core/ext/client_config/http_connect_handshaker.h +++ b/src/core/ext/client_config/http_connect_handshaker.h @@ -36,13 +36,8 @@ #include "src/core/lib/channel/handshaker.h" -/// Caller does NOT take ownership of returned string. -char* grpc_get_http_connect_proxy_server_from_args(grpc_channel_args *args); - -/// Borrows references to \a proxy_server or \a server_name. -/// The caller must ensure that they remain alive until handshaking is -/// complete. -grpc_handshaker* grpc_http_connect_handshaker_create(char* proxy_server, - char* server_name); +/// Does NOT take ownership of \a proxy_server or \a server_name. +grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server, + const char* server_name); #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_HTTP_CONNECT_HANDSHAKER_H */ diff --git a/src/core/ext/client_config/resolver_registry.c b/src/core/ext/client_config/resolver_registry.c index e7a4abd568..13f08e9fe6 100644 --- a/src/core/ext/client_config/resolver_registry.c +++ b/src/core/ext/client_config/resolver_registry.c @@ -129,7 +129,8 @@ static grpc_resolver_factory *resolve_factory(const char *target, } grpc_resolver *grpc_resolver_create( - const char *target, grpc_client_channel_factory *client_channel_factory) { + const char *target, grpc_client_channel_factory *client_channel_factory, + char **http_proxy) { grpc_uri *uri = NULL; grpc_resolver_factory *factory = resolve_factory(target, &uri); grpc_resolver *resolver; @@ -138,6 +139,9 @@ grpc_resolver *grpc_resolver_create( args.uri = uri; args.client_channel_factory = client_channel_factory; resolver = grpc_resolver_factory_create_resolver(factory, &args); + const char *proxy = grpc_uri_get_query_arg(uri, "http_proxy"); + if (proxy != NULL) + *http_proxy = gpr_strdup(proxy); grpc_uri_destroy(uri); return resolver; } diff --git a/src/core/ext/client_config/resolver_registry.h b/src/core/ext/client_config/resolver_registry.h index 5ef1383cd3..28843001ea 100644 --- a/src/core/ext/client_config/resolver_registry.h +++ b/src/core/ext/client_config/resolver_registry.h @@ -54,9 +54,12 @@ void grpc_register_resolver_type(grpc_resolver_factory *factory); was not NULL). If a resolver factory was found, use it to instantiate a resolver and return it. - If a resolver factory was not found, return NULL. */ + If a resolver factory was not found, return NULL. + If \a target specifies an http_proxy as a query arg, sets \a http_proxy + to the value (which the caller takes ownership of). */ grpc_resolver *grpc_resolver_create( - const char *target, grpc_client_channel_factory *client_channel_factory); + const char *target, grpc_client_channel_factory *client_channel_factory, + char **http_proxy); /** Find a resolver factory given a name and return an (owned-by-the-caller) * reference to it */ diff --git a/src/core/ext/client_config/uri_parser.c b/src/core/ext/client_config/uri_parser.c index 3ca1a58e69..bc80432336 100644 --- a/src/core/ext/client_config/uri_parser.c +++ b/src/core/ext/client_config/uri_parser.c @@ -118,8 +118,8 @@ static int parse_fragment_or_query(const char *uri_text, size_t *i) { const size_t advance = parse_pchar(uri_text, *i); /* pchar */ switch (advance) { case 0: /* uri_text[i] isn't in pchar */ - /* maybe it's ? or / */ - if (uri_text[*i] == '?' || uri_text[*i] == '/') { + /* maybe it's ? or / or : */ + if (uri_text[*i] == '?' || uri_text[*i] == '/' || uri_text[*i] == ':') { (*i)++; break; } else { diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index 255448f427..c8402da0fd 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -54,8 +54,10 @@ typedef struct { grpc_resolver base; /** refcount */ gpr_refcount refs; - /** name to resolve */ - char *name; + /** target name */ + char *target_name; + /** name to resolve (usually the same as target_name) */ + char *name_to_resolve; /** default port to use */ char *default_port; /** subchannel factory */ @@ -174,7 +176,7 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, if (addresses != NULL) { grpc_lb_policy_args lb_policy_args; memset(&lb_policy_args, 0, sizeof(lb_policy_args)); - lb_policy_args.server_name = r->name; + lb_policy_args.server_name = r->target_name; lb_policy_args.addresses = addresses; lb_policy_args.client_channel_factory = r->client_channel_factory; lb_policy = @@ -221,7 +223,7 @@ static void dns_start_resolving_locked(grpc_exec_ctx *exec_ctx, GPR_ASSERT(!r->resolving); r->resolving = true; r->addresses = NULL; - grpc_resolve_address(exec_ctx, r->name, r->default_port, + grpc_resolve_address(exec_ctx, r->name_to_resolve, r->default_port, grpc_closure_create(dns_on_resolved, r), &r->addresses); } @@ -246,7 +248,8 @@ static void dns_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) { grpc_client_config_unref(exec_ctx, r->resolved_config); } grpc_client_channel_factory_unref(exec_ctx, r->client_channel_factory); - gpr_free(r->name); + gpr_free(r->target_name); + gpr_free(r->name_to_resolve); gpr_free(r->default_port); gpr_free(r->lb_policy_name); gpr_free(r); @@ -255,22 +258,22 @@ static void dns_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) { static grpc_resolver *dns_create(grpc_resolver_args *args, const char *default_port, const char *lb_policy_name) { - dns_resolver *r; - const char *path = args->uri->path; - if (0 != strcmp(args->uri->authority, "")) { gpr_log(GPR_ERROR, "authority based dns uri's not supported"); return NULL; } - + // Get name and (optionally) proxy address from args. + const char *path = args->uri->path; if (path[0] == '/') ++path; - - r = gpr_malloc(sizeof(dns_resolver)); + const char *proxy_name = grpc_uri_get_query_arg(args->uri, "http_proxy"); + // Create resolver. + dns_resolver *r = gpr_malloc(sizeof(dns_resolver)); memset(r, 0, sizeof(*r)); gpr_ref_init(&r->refs, 1); gpr_mu_init(&r->mu); grpc_resolver_init(&r->base, &dns_resolver_vtable); - r->name = gpr_strdup(path); + r->target_name = gpr_strdup(path); + r->name_to_resolve = gpr_strdup(proxy_name == NULL ? path : proxy_name); r->default_port = gpr_strdup(default_port); r->client_channel_factory = args->client_channel_factory; gpr_backoff_init(&r->backoff_state, BACKOFF_MULTIPLIER, BACKOFF_JITTER, diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.c b/src/core/ext/transport/chttp2/client/insecure/channel_create.c index 2df4ce1dda..8f8da7f5ea 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.c +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.c @@ -155,6 +155,7 @@ typedef struct { gpr_refcount refs; grpc_channel_args *merge_args; grpc_channel *master; + char *http_proxy; } client_channel_factory; static void client_channel_factory_ref( @@ -172,6 +173,8 @@ static void client_channel_factory_unref( "client_channel_factory"); } grpc_channel_args_destroy(f->merge_args); + if (f->http_proxy != NULL) + gpr_free(f->http_proxy); gpr_free(f); } } @@ -188,10 +191,9 @@ static grpc_subchannel *client_channel_factory_create_subchannel( c->base.vtable = &connector_vtable; gpr_ref_init(&c->refs, 1); c->handshake_mgr = grpc_handshake_manager_create(); - char *proxy_server = grpc_get_http_connect_proxy_server_from_args(final_args); - if (proxy_server != NULL) { + if (f->http_proxy != NULL) { grpc_handshake_manager_add( - grpc_http_connect_handshaker_create(proxy_server, args->server_name), + grpc_http_connect_handshaker_create(f->http_proxy, args->server_name), c->handshake_mgr); } args->args = final_args; @@ -210,7 +212,8 @@ static grpc_channel *client_channel_factory_create_channel( grpc_channel *channel = grpc_channel_create(exec_ctx, target, final_args, GRPC_CLIENT_CHANNEL, NULL); grpc_channel_args_destroy(final_args); - grpc_resolver *resolver = grpc_resolver_create(target, &f->base); + grpc_resolver *resolver = grpc_resolver_create(target, &f->base, + &f->http_proxy); if (!resolver) { GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, channel, "client_channel_factory_create_channel"); diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c index ee8f39434b..863051fea4 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c @@ -222,6 +222,7 @@ typedef struct { grpc_channel_args *merge_args; grpc_channel_security_connector *security_connector; grpc_channel *master; + char *http_proxy; } client_channel_factory; static void client_channel_factory_ref( @@ -241,6 +242,8 @@ static void client_channel_factory_unref( "client_channel_factory"); } grpc_channel_args_destroy(f->merge_args); + if (f->http_proxy != NULL) + gpr_free(f->http_proxy); gpr_free(f); } } @@ -257,10 +260,9 @@ static grpc_subchannel *client_channel_factory_create_subchannel( c->base.vtable = &connector_vtable; c->security_connector = f->security_connector; c->handshake_mgr = grpc_handshake_manager_create(); - char *proxy_server = grpc_get_http_connect_proxy_server_from_args(final_args); - if (proxy_server != NULL) { + if (f->http_proxy != NULL) { grpc_handshake_manager_add( - grpc_http_connect_handshaker_create(proxy_server, args->server_name), + grpc_http_connect_handshaker_create(f->http_proxy, args->server_name), c->handshake_mgr); } gpr_mu_init(&c->mu); @@ -283,7 +285,8 @@ static grpc_channel *client_channel_factory_create_channel( GRPC_CLIENT_CHANNEL, NULL); grpc_channel_args_destroy(final_args); - grpc_resolver *resolver = grpc_resolver_create(target, &f->base); + grpc_resolver *resolver = grpc_resolver_create(target, &f->base, + &f->http_proxy); if (resolver != NULL) { grpc_client_channel_set_resolver( exec_ctx, grpc_channel_get_channel_stack(channel), resolver); -- cgit v1.2.3