chore(protocol-http): switch to HttpRequest static clone method (#1345)

* chore(protocol-http): switch to HttpRequest static clone method * update unit test
smithy-lang · Jul 18, 2024 · 86862ea · 86862ea
1 parent 9624938
commit 86862ea
Show file tree

Hide file tree

Showing 13 changed files with 85 additions and 97 deletions.
diff --git a/.changeset/poor-dolls-cheer.md b/.changeset/poor-dolls-cheer.md
@@ -0,0 +1,8 @@
+---
+"@smithy/protocol-http": minor
+"@smithy/signature-v4": minor
+"@smithy/core": minor
+"@smithy/experimental-identity-and-auth": patch
+---
+
+switch to static HttpRequest clone method
diff --git a/packages/core/src/util-identity-and-auth/httpAuthSchemes/httpApiKeyAuth.ts b/packages/core/src/util-identity-and-auth/httpAuthSchemes/httpApiKeyAuth.ts
@@ -24,7 +24,7 @@ export class HttpApiKeyAuthSigner implements HttpSigner {
     if (!identity.apiKey) {
       throw new Error("request could not be signed with `apiKey` since the `apiKey` is not defined");
     }
-    const clonedRequest = httpRequest.clone();
+    const clonedRequest = HttpRequest.clone(httpRequest);
     if (signingProperties.in === HttpApiKeyAuthLocation.QUERY) {
       clonedRequest.query[signingProperties.name] = identity.apiKey;
     } else if (signingProperties.in === HttpApiKeyAuthLocation.HEADER) {

diff --git a/packages/core/src/util-identity-and-auth/httpAuthSchemes/httpBearerAuth.ts b/packages/core/src/util-identity-and-auth/httpAuthSchemes/httpBearerAuth.ts
@@ -11,7 +11,7 @@ export class HttpBearerAuthSigner implements HttpSigner {
     identity: TokenIdentity,
     signingProperties: Record<string, any>
   ): Promise<IHttpRequest> {
-    const clonedRequest = httpRequest.clone();
+    const clonedRequest = HttpRequest.clone(httpRequest);
     if (!identity.token) {
       throw new Error("request could not be signed with `token` since the `token` is not defined");
     }

diff --git a/packages/experimental-identity-and-auth/src/SigV4Signer.ts b/packages/experimental-identity-and-auth/src/SigV4Signer.ts
@@ -13,7 +13,7 @@ export class SigV4Signer implements HttpSigner {
     identity: AwsCredentialIdentity,
     signingProperties: Record<string, any>
   ): Promise<IHttpRequest> {
-    const clonedRequest = httpRequest.clone();
+    const clonedRequest = HttpRequest.clone(httpRequest);
     const signer = new SignatureV4({
       applyChecksum: signingProperties.applyChecksum !== undefined ? signingProperties.applyChecksum : true,
       credentials: identity,

diff --git a/packages/experimental-identity-and-auth/src/httpApiKeyAuth.ts b/packages/experimental-identity-and-auth/src/httpApiKeyAuth.ts
@@ -35,7 +35,7 @@ export class HttpApiKeyAuthSigner implements HttpSigner {
     if (!identity.apiKey) {
       throw new Error("request could not be signed with `apiKey` since the `apiKey` is not defined");
     }
-    const clonedRequest = httpRequest.clone();
+    const clonedRequest = HttpRequest.clone(httpRequest);
     if (signingProperties.in === HttpApiKeyAuthLocation.QUERY) {
       clonedRequest.query[signingProperties.name] = identity.apiKey;
     } else if (signingProperties.in === HttpApiKeyAuthLocation.HEADER) {

diff --git a/packages/experimental-identity-and-auth/src/httpBearerAuth.ts b/packages/experimental-identity-and-auth/src/httpBearerAuth.ts
@@ -14,7 +14,7 @@ export class HttpBearerAuthSigner implements HttpSigner {
     identity: TokenIdentity,
     signingProperties: Record<string, any>
   ): Promise<IHttpRequest> {
-    const clonedRequest = httpRequest.clone();
+    const clonedRequest = HttpRequest.clone(httpRequest);
     if (!identity.token) {
       throw new Error("request could not be signed with `token` since the `token` is not defined");
     }

diff --git a/packages/protocol-http/src/httpRequest.spec.ts b/packages/protocol-http/src/httpRequest.spec.ts
@@ -1,3 +1,5 @@
+import { QueryParameterBag } from "@smithy/types";
+
 import { HttpRequest, IHttpRequest } from "./httpRequest";
 
 describe("HttpRequest", () => {
@@ -44,8 +46,8 @@ describe("HttpRequest", () => {
   it("should maintain a deprecated instance clone method", () => {
     const httpRequestInstance = new HttpRequest(httpRequest);
 
-    const clone1 = httpRequestInstance.clone();
-    const clone2 = httpRequestInstance.clone();
+    const clone1 = HttpRequest.clone(httpRequestInstance);
+    const clone2 = HttpRequest.clone(httpRequestInstance);
 
     expect(httpRequestInstance).toEqual(clone1);
     expect(clone1).toEqual(clone2);
@@ -55,3 +57,60 @@ describe("HttpRequest", () => {
     expect(clone1.body).toBe(clone2.body);
   });
 });
+
+const cloneRequest = HttpRequest.clone;
+
+describe("cloneRequest", () => {
+  const request: IHttpRequest = Object.freeze({
+    method: "GET",
+    protocol: "https:",
+    hostname: "foo.us-west-2.amazonaws.com",
+    path: "/",
+    headers: Object.freeze({
+      foo: "bar",
+      compound: "value 1, value 2",
+    }),
+    query: Object.freeze({
+      fizz: "buzz",
+      snap: ["crackle", "pop"],
+    }),
+  });
+
+  it("should return an object matching the provided request", () => {
+    expect(cloneRequest(request)).toEqual(request);
+  });
+
+  it("should return an object that with a different identity", () => {
+    expect(cloneRequest(request)).not.toBe(request);
+  });
+
+  it("should should deep-copy the headers", () => {
+    const clone = cloneRequest(request);
+
+    delete clone.headers.compound;
+    expect(Object.keys(request.headers)).toEqual(["foo", "compound"]);
+    expect(Object.keys(clone.headers)).toEqual(["foo"]);
+  });
+
+  it("should should deep-copy the query", () => {
+    const clone = cloneRequest(request);
+
+    const { snap } = clone.query as QueryParameterBag;
+    (snap as Array<string>).shift();
+
+    expect((request.query as QueryParameterBag).snap).toEqual(["crackle", "pop"]);
+    expect((clone.query as QueryParameterBag).snap).toEqual(["pop"]);
+  });
+
+  it("should not copy the body", () => {
+    const body = new Uint8Array(16);
+    const req = { ...request, body };
+    const clone = cloneRequest(req);
+
+    expect(clone.body).toBe(req.body);
+  });
+
+  it("should handle requests without defined query objects", () => {
+    expect(cloneRequest({ ...request, query: void 0 }).query).toEqual({});
+  });
+});
diff --git a/packages/signature-v4/package.json b/packages/signature-v4/package.json
@@ -25,6 +25,7 @@
   "license": "Apache-2.0",
   "dependencies": {
     "@smithy/is-array-buffer": "workspace:^",
+    "@smithy/protocol-http": "workspace:^",
     "@smithy/types": "workspace:^",
     "@smithy/util-hex-encoding": "workspace:^",
     "@smithy/util-middleware": "workspace:^",
@@ -34,7 +35,6 @@
   },
   "devDependencies": {
     "@aws-crypto/sha256-js": "5.2.0",
-    "@smithy/protocol-http": "workspace:^",
     "concurrently": "7.0.0",
     "downlevel-dts": "0.10.1",
     "rimraf": "3.0.2",

diff --git a/packages/signature-v4/src/SignatureV4.spec.ts b/packages/signature-v4/src/SignatureV4.spec.ts
@@ -410,7 +410,7 @@ describe("SignatureV4", () => {
     });
 
     it("should sign requests without host header", async () => {
-      const request = minimalRequest.clone();
+      const request = HttpRequest.clone(minimalRequest);
       delete request.headers[HOST_HEADER];
       const { headers } = await signer.sign(request, {
         signingDate: new Date("2000-01-01T00:00:00.000Z"),

diff --git a/packages/signature-v4/src/cloneRequest.spec.ts b/packages/signature-v4/src/cloneRequest.spec.ts
diff --git a/packages/signature-v4/src/cloneRequest.ts b/packages/signature-v4/src/cloneRequest.ts
diff --git a/packages/signature-v4/src/moveHeadersToQuery.ts b/packages/signature-v4/src/moveHeadersToQuery.ts
@@ -1,16 +1,14 @@
-import { HttpRequest, QueryParameterBag } from "@smithy/types";
-
-import { cloneRequest } from "./cloneRequest";
+import { HttpRequest } from "@smithy/protocol-http";
+import type { HttpRequest as IHttpRequest, QueryParameterBag } from "@smithy/types";
 
 /**
  * @private
  */
 export const moveHeadersToQuery = (
-  request: HttpRequest,
+  request: IHttpRequest,
   options: { unhoistableHeaders?: Set<string> } = {}
-): HttpRequest & { query: QueryParameterBag } => {
-  const { headers, query = {} as QueryParameterBag } =
-    typeof (request as any).clone === "function" ? (request as any).clone() : cloneRequest(request);
+): IHttpRequest & { query: QueryParameterBag } => {
+  const { headers, query = {} as QueryParameterBag } = HttpRequest.clone(request);
   for (const name of Object.keys(headers)) {
     const lname = name.toLowerCase();
     if (lname.slice(0, 6) === "x-amz-" && !options.unhoistableHeaders?.has(lname)) {

diff --git a/packages/signature-v4/src/prepareRequest.ts b/packages/signature-v4/src/prepareRequest.ts
@@ -1,14 +1,14 @@
-import { HttpRequest } from "@smithy/types";
+import { HttpRequest } from "@smithy/protocol-http";
+import type { HttpRequest as IHttpRequest } from "@smithy/types";
 
-import { cloneRequest } from "./cloneRequest";
 import { GENERATED_HEADERS } from "./constants";
 
 /**
  * @private
  */
-export const prepareRequest = (request: HttpRequest): HttpRequest => {
+export const prepareRequest = (request: IHttpRequest): IHttpRequest => {
   // Create a clone of the request object that does not clone the body
-  request = typeof (request as any).clone === "function" ? (request as any).clone() : cloneRequest(request);
+  request = HttpRequest.clone(request);
 
   for (const headerName of Object.keys(request.headers)) {
     if (GENERATED_HEADERS.indexOf(headerName.toLowerCase()) > -1) {