Use http_proxy environment variable instead of URI query param.

6 files changed, 35 insertions, 13 deletions

diff --git a/src/core/ext/client_config/http_connect_handshaker.c b/src/core/ext/client_config/http_connect_handshaker.c
index 55b01bd46e..097465469e 100644
--- a/src/core/ext/client_config/http_connect_handshaker.c
+++ b/src/core/ext/client_config/http_connect_handshaker.c
@@ -40,9 +40,11 @@
 #include <grpc/impl/codegen/slice_buffer.h>
 #include <grpc/support/string_util.h>
 
+#include "src/core/ext/client_config/uri_parser.h"
 #include "src/core/lib/http/format_request.h"
 #include "src/core/lib/http/parser.h"
 #include "src/core/lib/iomgr/timer.h"
+#include "src/core/lib/support/env.h"
 
 typedef struct http_connect_handshaker {
   // Base class.  Must be first.
@@ -247,3 +249,26 @@ grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server,
   gpr_ref_init(&handshaker->refcount, 1);
   return &handshaker->base;
 }
+
+char* grpc_get_http_proxy_server() {
+  char* uri_str = gpr_getenv("http_proxy");
+  if (uri_str == NULL) return NULL;
+  grpc_uri* uri = grpc_uri_parse(uri_str, false /* suppress_errors */);
+  char* proxy_name = NULL;
+  if (uri == NULL || uri->authority == NULL) {
+    gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var");
+    goto done;
+  }
+  if (strcmp(uri->scheme, "http") != 0) {
+    gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI", uri->scheme);
+    goto done;
+  }
+  if (strchr(uri->authority, '@') != NULL) {
+    gpr_log(GPR_ERROR, "userinfo not supported in proxy URI");
+    goto done;
+  }
+  proxy_name = gpr_strdup(uri->authority);
+done:
+  grpc_uri_destroy(uri);
+  return proxy_name;
+}
diff --git a/src/core/ext/client_config/http_connect_handshaker.h b/src/core/ext/client_config/http_connect_handshaker.h
index 146ef9369a..1fc3948267 100644
--- a/src/core/ext/client_config/http_connect_handshaker.h
+++ b/src/core/ext/client_config/http_connect_handshaker.h
@@ -40,4 +40,8 @@
 grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server,
                                                      const char* server_name);
 
+/// Returns the name of the proxy to use, or NULL if no proxy is configured.
+/// Caller takes ownership of result.
+char* grpc_get_http_proxy_server();
+
 #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_HTTP_CONNECT_HANDSHAKER_H */
diff --git a/src/core/ext/client_config/lb_policy_factory.h b/src/core/ext/client_config/lb_policy_factory.h
index 5806deef9b..a9d3588767 100644
--- a/src/core/ext/client_config/lb_policy_factory.h
+++ b/src/core/ext/client_config/lb_policy_factory.h
@@ -48,7 +48,7 @@ struct grpc_lb_policy_factory {
 };
 
 typedef struct grpc_lb_policy_args {
-  char *server_name;  // Does not own.
+  char *server_name;
   grpc_resolved_addresses *addresses;
   grpc_client_channel_factory *client_channel_factory;
 } grpc_lb_policy_args;
diff --git a/src/core/ext/client_config/resolver_registry.c b/src/core/ext/client_config/resolver_registry.c
index 5a8f137103..e7a4abd568 100644
--- a/src/core/ext/client_config/resolver_registry.c
+++ b/src/core/ext/client_config/resolver_registry.c
@@ -129,8 +129,7 @@ static grpc_resolver_factory *resolve_factory(const char *target,
 }
 
 grpc_resolver *grpc_resolver_create(
-    const char *target, grpc_client_channel_factory *client_channel_factory,
-    char **http_proxy) {
+    const char *target, grpc_client_channel_factory *client_channel_factory) {
   grpc_uri *uri = NULL;
   grpc_resolver_factory *factory = resolve_factory(target, &uri);
   grpc_resolver *resolver;
@@ -139,8 +138,6 @@ 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 28843001ea..5ef1383cd3 100644
--- a/src/core/ext/client_config/resolver_registry.h
+++ b/src/core/ext/client_config/resolver_registry.h
@@ -54,12 +54,9 @@ 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 target specifies an http_proxy as a query arg, sets \a http_proxy
-    to the value (which the caller takes ownership of). */
+    If a resolver factory was not found, return NULL. */
 grpc_resolver *grpc_resolver_create(
-    const char *target, grpc_client_channel_factory *client_channel_factory,
-    char **http_proxy);
+    const char *target, grpc_client_channel_factory *client_channel_factory);
 
 /** 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 5e8432c6c8..3ca1a58e69 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 / or : */
-        if (uri_text[*i] == '?' || uri_text[*i] == '/' || uri_text[*i] == ':') {
+        /* maybe it's ? or / */
+        if (uri_text[*i] == '?' || uri_text[*i] == '/') {
           (*i)++;
           break;
         } else {
@@ -282,7 +282,6 @@ grpc_uri *grpc_uri_parse(const char *uri_text, int suppress_errors) {
 }
 
 const char *grpc_uri_get_query_arg(const grpc_uri *uri, const char *key) {
-  if (uri == NULL) return NULL;
   GPR_ASSERT(key != NULL);
   if (key[0] == '\0') return NULL;