grpc-js: Fix proxy + URI parsing bugs

packages/grpc-js/src/channel.ts

@@ -33,7 +33,7 @@ import { FilterStackFactory } from './filter-stack';
 import { CallCredentialsFilterFactory } from './call-credentials-filter';
 import { DeadlineFilterFactory } from './deadline-filter';
 import { CompressionFilterFactory } from './compression-filter';
-import { getDefaultAuthority } from './resolver';
+import { getDefaultAuthority, mapUriDefaultScheme } from './resolver';
 import { ServiceConfig, validateServiceConfig } from './service-config';
 import { trace, log } from './logging';
 import { SubchannelAddress } from './subchannel';
@@ -170,20 +170,18 @@ export class ChannelImplementation implements Channel {
     if (originalTargetUri === null) {
       throw new Error(`Could not parse target name "${target}"`);
     }
+    /* This ensures that the target has a scheme that is registered with the
+     * resolver */
+    const defaultSchemeMapResult = mapUriDefaultScheme(originalTargetUri);
     if (this.options['grpc.default_authority']) {
       this.defaultAuthority = this.options['grpc.default_authority'] as string;
     } else {
-      this.defaultAuthority = getDefaultAuthority(originalTargetUri);
+      this.defaultAuthority = getDefaultAuthority(defaultSchemeMapResult);
     }
-    const proxyMapResult = mapProxyName(originalTargetUri, options);
+    const proxyMapResult = mapProxyName(defaultSchemeMapResult, options);
     this.target = proxyMapResult.target;
     this.options = Object.assign({}, this.options, proxyMapResult.extraOptions);
 
-    const targetUri = parseUri(target);
-    if (targetUri === null) {
-      throw new Error(`Could not parse target name "${target}"`);
-    }
-    this.target = targetUri;
     /* The global boolean parameter to getSubchannelPool has the inverse meaning to what
      * the grpc.use_local_subchannel_pool channel option means. */
     this.subchannelPool = getSubchannelPool(

packages/grpc-js/src/http_proxy.ts

@@ -29,6 +29,7 @@ import {
 } from './subchannel';
 import { ChannelOptions } from './channel-options';
 import { GrpcUri, parseUri, splitHostPort, uriToString } from './uri-parser';
+import { URL } from 'url';
 
 const TRACER_NAME = 'proxy';
 
@@ -60,30 +61,31 @@ function getProxyInfo(): ProxyInfo {
   } else {
     return {};
   }
-  const proxyUrl = parseUri(proxyEnv);
-  if (proxyUrl === null) {
+  let proxyUrl: URL;
+  try {
+    proxyUrl = new URL(proxyEnv);
+  } catch (e) {
     log(LogVerbosity.ERROR, `cannot parse value of "${envVar}" env var`);
     return {};
   }
-  if (proxyUrl.scheme !== 'http') {
+  if (proxyUrl.protocol !== 'http:') {
     log(
       LogVerbosity.ERROR,
-      `"${proxyUrl.scheme}" scheme not supported in proxy URI`
+      `"${proxyUrl.protocol}" scheme not supported in proxy URI`
     );
     return {};
   }
-  const splitPath = proxyUrl.path.split('@');
-  let host: string;
   let userCred: string | null = null;
-  if (splitPath.length === 2) {
-    log(LogVerbosity.INFO, 'userinfo found in proxy URI');
-    userCred = splitPath[0];
-    host = splitPath[1];
-  } else {
-    host = proxyUrl.path;
+  if (proxyUrl.username) {
+    if (proxyUrl.password) {
+      log(LogVerbosity.INFO, 'userinfo found in proxy URI');
+      userCred = `${proxyUrl.username}:${proxyUrl.password}`;
+    } else {
+      userCred = proxyUrl.username;
+    }
   }
   const result: ProxyInfo = {
-    address: host,
+    address: proxyUrl.host,
   };
   if (userCred) {
     result.creds = userCred;
@@ -145,7 +147,10 @@ export function mapProxyName(
     extraOptions['grpc.http_connect_creds'] = proxyInfo.creds;
   }
   return {
-    target: { path: proxyInfo.address },
+    target: { 
+      scheme: 'dns',
+      path: proxyInfo.address
+    },
     extraOptions: extraOptions,
   };
 }

packages/grpc-js/src/resolver-dns.ts

@@ -18,7 +18,6 @@ import {
   Resolver,
   ResolverListener,
   registerResolver,
-  registerDefaultResolver,
 } from './resolver';
 import * as dns from 'dns';
 import * as util from 'util';
@@ -281,7 +280,6 @@ class DnsResolver implements Resolver {
  */
 export function setup(): void {
   registerResolver('dns', DnsResolver);
-  registerDefaultResolver(DnsResolver);
 }
 
 export interface DnsUrl {

packages/grpc-js/src/resolver.ts

@@ -74,7 +74,7 @@ export interface ResolverConstructor {
 }
 
 const registeredResolvers: { [scheme: string]: ResolverConstructor } = {};
-let defaultResolver: ResolverConstructor | null = null;
+let defaultScheme: string | null = null;
 
 /**
  * Register a resolver class to handle target names prefixed with the `prefix`
@@ -95,8 +95,8 @@ export function registerResolver(
  * any registered prefix.
  * @param resolverClass
  */
-export function registerDefaultResolver(resolverClass: ResolverConstructor) {
-  defaultResolver = resolverClass;
+export function registerDefaultScheme(scheme: string) {
+  defaultScheme = scheme;
 }
 
 /**
@@ -112,18 +112,10 @@ export function createResolver(
   if (target.scheme !== undefined && target.scheme in registeredResolvers) {
     return new registeredResolvers[target.scheme](target, listener);
   } else {
-    if (defaultResolver !== null) {
-      /* If the scheme does not correspond to a registered scheme, we assume
-       * that the whole thing is the path, and the scheme was pulled out
-       * incorrectly. For example, it is valid to parse "localhost:80" as
-       * having a scheme of "localhost" and a path of 80, but that is not
-       * how the resolver should see it */
-      return new defaultResolver({ path: uriToString(target) }, listener);
-    }
+    throw new Error(
+      `No resolver could be created for target ${uriToString(target)}`
+    );
   }
-  throw new Error(
-    `No resolver could be created for target ${uriToString(target)}`
-  );
 }
 
 /**
@@ -135,12 +127,22 @@ export function getDefaultAuthority(target: GrpcUri): string {
   if (target.scheme !== undefined && target.scheme in registeredResolvers) {
     return registeredResolvers[target.scheme].getDefaultAuthority(target);
   } else {
-    if (defaultResolver !== null) {
-      // See comment in createResolver for why we handle the target like this
-      return defaultResolver.getDefaultAuthority({ path: uriToString(target) });
+    throw new Error(`Invalid target ${uriToString(target)}`);
+  }
+}
+
+export function mapUriDefaultScheme(target: GrpcUri): GrpcUri {
+  if (target.scheme === undefined || !(target.scheme in registeredResolvers)) {
+    if (defaultScheme !== null) {
+      return {
+        scheme: defaultScheme,
+        path: uriToString(target)
+      };
+    } else {
+      throw new Error(`Invalid target ${uriToString(target)}`);
     }
   }
-  throw new Error(`Invalid target ${uriToString(target)}`);
+  return target;
 }
 
 export function registerAll() {