feat(protocol-http): add uri type and implementation (#4771)

Adds the URI type to types package, and uses it in HttpRequest. The URI type adds additional properties that didn't exist on Endpoint. Http handlers were updated to use these new properties when constructing request URLs. Tests were also added to make sure handlers construct URLs properly with the new properties.
aws · Jun 6, 2023 · 5c7b40e · 5c7b40e
1 parent d29cb58
commit 5c7b40e
Show file tree

Hide file tree

Showing 11 changed files with 132 additions and 17 deletions.
diff --git a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts
@@ -95,7 +95,11 @@ describe(FetchHttpHandler.name, () => {
       headers: {},
       hostname: "foo.amazonaws.com",
       method: "GET",
-      path: "/test/?bar=baz",
+      path: "/test",
+      query: { bar: "baz" },
+      username: "username",
+      password: "password",
+      fragment: "fragment",
       protocol: "https:",
       port: 443,
     });
@@ -105,7 +109,7 @@ describe(FetchHttpHandler.name, () => {
 
     expect(mockFetch.mock.calls.length).toBe(1);
     const requestCall = mockRequest.mock.calls[0];
-    expect(requestCall[0]).toBe("https://foo.amazonaws.com:443/test/?bar=baz");
+    expect(requestCall[0]).toBe("https://username:password@foo.amazonaws.com:443/test?bar=baz#fragment");
   });
 
   it("will omit body if method is GET", async () => {

diff --git a/packages/fetch-http-handler/src/fetch-http-handler.ts b/packages/fetch-http-handler/src/fetch-http-handler.ts
@@ -50,15 +50,23 @@ export class FetchHttpHandler implements HttpHandler {
     }
 
     let path = request.path;
-    if (request.query) {
-      const queryString = buildQueryString(request.query);
-      if (queryString) {
-        path += `?${queryString}`;
-      }
+    const queryString = buildQueryString(request.query || {});
+    if (queryString) {
+      path += `?${queryString}`;
+    }
+    if (request.fragment) {
+      path += `#${request.fragment}`;
+    }
+
+    let auth = "";
+    if (request.username != null || request.password != null) {
+      const username = request.username ?? "";
+      const password = request.password ?? "";
+      auth = `${username}:${password}@`;
     }
 
     const { port, method } = request;
-    const url = `${request.protocol}//${request.hostname}${port ? `:${port}` : ""}${path}`;
+    const url = `${request.protocol}//${auth}${request.hostname}${port ? `:${port}` : ""}${path}`;
     // Request constructor doesn't allow GET/HEAD request with body
     // ref: https://github.com/whatwg/fetch/issues/551
     const body = method === "GET" || method === "HEAD" ? undefined : request.body;

diff --git a/packages/node-http-handler/src/node-http-handler.spec.ts b/packages/node-http-handler/src/node-http-handler.spec.ts
@@ -115,6 +115,27 @@ describe("NodeHttpHandler", () => {
         expect(providerInvokedCount).toBe(1);
         expect(providerResolvedCount).toBe(1);
       });
+
+      it("sends requests to the right url", async () => {
+        const nodeHttpHandler = new NodeHttpHandler({});
+        const httpRequest = {
+          protocol: "http:",
+          username: "username",
+          password: "password",
+          hostname: "host",
+          port: 1234,
+          path: "/some/path",
+          query: {
+            some: "query",
+          },
+          fragment: "fragment",
+        };
+        await nodeHttpHandler.handle(httpRequest as any);
+        expect(hRequestSpy.mock.calls[0][0]?.auth).toEqual("username:password");
+        expect(hRequestSpy.mock.calls[0][0]?.host).toEqual("host");
+        expect(hRequestSpy.mock.calls[0][0]?.port).toEqual(1234);
+        expect(hRequestSpy.mock.calls[0][0]?.path).toEqual("/some/path?some=query#fragment");
+      });
     });
   });
 

diff --git a/packages/node-http-handler/src/node-http-handler.ts b/packages/node-http-handler/src/node-http-handler.ts
@@ -119,13 +119,27 @@ export class NodeHttpHandler implements HttpHandler {
       // determine which http(s) client to use
       const isSSL = request.protocol === "https:";
       const queryString = buildQueryString(request.query || {});
+      let auth = undefined;
+      if (request.username != null || request.password != null) {
+        const username = request.username ?? "";
+        const password = request.password ?? "";
+        auth = `${username}:${password}`;
+      }
+      let path = request.path;
+      if (queryString) {
+        path += `?${queryString}`;
+      }
+      if (request.fragment) {
+        path += `#${request.fragment}`;
+      }
       const nodeHttpsOptions: RequestOptions = {
         headers: request.headers,
         host: request.hostname,
         method: request.method,
-        path: queryString ? `${request.path}?${queryString}` : request.path,
+        path,
         port: request.port,
         agent: isSSL ? this.config.httpsAgent : this.config.httpAgent,
+        auth,
       };
 
       // create the http request

diff --git a/packages/node-http-handler/src/node-http2-handler.spec.ts b/packages/node-http-handler/src/node-http2-handler.spec.ts
@@ -625,4 +625,22 @@ describe(NodeHttp2Handler.name, () => {
       });
     });
   });
+
+  it("sends the request to the correct url", async () => {
+    const server = createMockHttp2Server();
+    server.on("request", (request, response) => {
+      expect(request.url).toBe("http://foo:bar@localhost/foo/bar?foo=bar#foo");
+      response.statusCode = 200;
+    });
+    const handler = new NodeHttp2Handler({});
+    await handler.handle({
+      ...getMockReqOptions(),
+      username: "foo",
+      password: "bar",
+      path: "/foo/bar",
+      query: { foo: "bar" },
+      fragment: "foo",
+    } as any);
+    handler.destroy();
+  });
 });
diff --git a/packages/node-http-handler/src/node-http2-handler.ts b/packages/node-http-handler/src/node-http2-handler.ts
@@ -100,8 +100,14 @@ export class NodeHttp2Handler implements HttpHandler {
         return;
       }
 
-      const { hostname, method, port, protocol, path, query } = request;
-      const authority = `${protocol}//${hostname}${port ? `:${port}` : ""}`;
+      const { hostname, method, port, protocol, query } = request;
+      let auth = "";
+      if (request.username != null || request.password != null) {
+        const username = request.username ?? "";
+        const password = request.password ?? "";
+        auth = `${username}:${password}@`;
+      }
+      const authority = `${protocol}//${auth}${hostname}${port ? `:${port}` : ""}`;
       const requestContext = { destination: new URL(authority) } as RequestContext;
       const session = this.connectionManager.lease(requestContext, {
         requestTimeout: this.config?.sessionTimeout,
@@ -117,10 +123,17 @@ export class NodeHttp2Handler implements HttpHandler {
       };
 
       const queryString = buildQueryString(query || {});
+      let path = request.path;
+      if (queryString) {
+        path += `?${queryString}`;
+      }
+      if (request.fragment) {
+        path += `#${request.fragment}`;
+      }
       // create the http2 request
       const req = session.request({
         ...request.headers,
-        [constants.HTTP2_HEADER_PATH]: queryString ? `${path}?${queryString}` : path,
+        [constants.HTTP2_HEADER_PATH]: path,
         [constants.HTTP2_HEADER_METHOD]: method,
       });
 

diff --git a/packages/protocol-http/src/httpRequest.ts b/packages/protocol-http/src/httpRequest.ts
@@ -1,17 +1,20 @@
-import { Endpoint, HeaderBag, HttpMessage, HttpRequest as IHttpRequest, QueryParameterBag } from "@aws-sdk/types";
+import { HeaderBag, HttpMessage, HttpRequest as IHttpRequest, QueryParameterBag, URI } from "@aws-sdk/types";
 
-type HttpRequestOptions = Partial<HttpMessage> & Partial<Endpoint> & { method?: string };
+type HttpRequestOptions = Partial<HttpMessage> & Partial<URI> & { method?: string };
 
 export interface HttpRequest extends IHttpRequest {}
 
-export class HttpRequest implements HttpMessage, Endpoint {
+export class HttpRequest implements HttpMessage, URI {
   public method: string;
   public protocol: string;
   public hostname: string;
   public port?: number;
   public path: string;
   public query: QueryParameterBag;
   public headers: HeaderBag;
+  public username?: string;
+  public password?: string;
+  public fragment?: string;
   public body?: any;
 
   constructor(options: HttpRequestOptions) {
@@ -27,6 +30,9 @@ export class HttpRequest implements HttpMessage, Endpoint {
         : options.protocol
       : "https:";
     this.path = options.path ? (options.path.charAt(0) !== "/" ? `/${options.path}` : options.path) : "/";
+    this.username = options.username;
+    this.password = options.password;
+    this.fragment = options.fragment;
   }
 
   static isInstance(request: unknown): request is HttpRequest {

diff --git a/packages/types/src/http.ts b/packages/types/src/http.ts
@@ -1,4 +1,6 @@
 import { AbortSignal } from "./abort";
+import { URI } from "./uri";
+
 /**
  * @public
  *
@@ -86,7 +88,7 @@ export interface Endpoint {
  * Interface an HTTP request class. Contains
  * addressing information in addition to standard message properties.
  */
-export interface HttpRequest extends HttpMessage, Endpoint {
+export interface HttpRequest extends HttpMessage, URI {
   method: string;
 }
 

diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -25,5 +25,6 @@ export * from "./signature";
 export * from "./stream";
 export * from "./token";
 export * from "./transfer";
+export * from "./uri";
 export * from "./util";
 export * from "./waiter";
diff --git a/packages/types/src/uri.ts b/packages/types/src/uri.ts
@@ -0,0 +1,18 @@
+import { QueryParameterBag } from "./http";
+
+/**
+ * @internal
+ *
+ * Represents the components parts of a Uniform Resource Identifier used to
+ * construct the target location of a Request.
+ */
+export type URI = {
+  protocol: string;
+  hostname: string;
+  port?: number;
+  path: string;
+  query?: QueryParameterBag;
+  username?: string;
+  password?: string;
+  fragment?: string;
+};
diff --git a/packages/util-format-url/src/index.ts b/packages/util-format-url/src/index.ts
@@ -17,5 +17,15 @@ export function formatUrl(request: Omit<HttpRequest, "headers" | "method">): str
   if (queryString && queryString[0] !== "?") {
     queryString = `?${queryString}`;
   }
-  return `${protocol}//${hostname}${path}${queryString}`;
+  let auth = "";
+  if (request.username != null || request.password != null) {
+    const username = request.username ?? "";
+    const password = request.password ?? "";
+    auth = `${username}:${password}@`;
+  }
+  let fragment = "";
+  if (request.fragment) {
+    fragment = `#${request.fragment}`;
+  }
+  return `${protocol}//${auth}${hostname}${path}${queryString}${fragment}`;
 }