From 5c7b40e16e40fe55ba8428bd9cab5b7fd181d1e7 Mon Sep 17 00:00:00 2001 From: Miles Ziemer <45497130+milesziemer@users.noreply.github.com> Date: Tue, 6 Jun 2023 06:33:06 -0400 Subject: [PATCH] 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. --- .../src/fetch-http-handler.spec.ts | 8 +++++-- .../src/fetch-http-handler.ts | 20 ++++++++++++------ .../src/node-http-handler.spec.ts | 21 +++++++++++++++++++ .../src/node-http-handler.ts | 16 +++++++++++++- .../src/node-http2-handler.spec.ts | 18 ++++++++++++++++ .../src/node-http2-handler.ts | 19 ++++++++++++++--- packages/protocol-http/src/httpRequest.ts | 12 ++++++++--- packages/types/src/http.ts | 4 +++- packages/types/src/index.ts | 1 + packages/types/src/uri.ts | 18 ++++++++++++++++ packages/util-format-url/src/index.ts | 12 ++++++++++- 11 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 packages/types/src/uri.ts diff --git a/packages/fetch-http-handler/src/fetch-http-handler.spec.ts b/packages/fetch-http-handler/src/fetch-http-handler.spec.ts index 7f6d6c75633f..7e5fb2d21ef8 100644 --- 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 index 47b5ccacf1b8..e8802a24f7e9 100644 --- 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 index 5d87a8bd924e..817008bbb213 100644 --- 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 index 948efb29efe5..5bd31d47d51c 100644 --- 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 index f10b2ce94f04..574ffaf3dd17 100644 --- 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 index d5df770ec485..5f256f84c090 100644 --- 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 index b0ab5efa3633..f8bcde17b828 100644 --- a/packages/protocol-http/src/httpRequest.ts +++ b/packages/protocol-http/src/httpRequest.ts @@ -1,10 +1,10 @@ -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 & Partial & { method?: string }; +type HttpRequestOptions = Partial & Partial & { 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; @@ -12,6 +12,9 @@ export class HttpRequest implements HttpMessage, Endpoint { 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 index 49e20cb55c69..4cb9f938d860 100644 --- 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 index ce1e6cf85815..f9879c17f018 100644 --- 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 new file mode 100644 index 000000000000..3aff9296a3f5 --- /dev/null +++ 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 index fe254054e201..bc892b217a0a 100644 --- 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): 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}`; }