fix(endpoint): dedupe clientContext/builtIn params, fix s3 unit test (#…

…4051) * fix(endpoint): dedupe clientContext/builtIn params, fix s3 unit test * fix(endpoint): fix unit tests for parseURL * fix(endpoint): parseURL unit tests
aws · Oct 18, 2022 · 947c8bc · 947c8bc
1 parent 79e11d7
commit 947c8bc
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 17 deletions.
diff --git a/clients/client-s3/test/S3.spec.ts b/clients/client-s3/test/S3.spec.ts
@@ -19,12 +19,11 @@ describe("endpoint", () => {
       expect(request.protocol).to.equal("http:");
       expect(request.hostname).to.equal("localhost");
       expect(request.port).to.equal(8080);
-      //query and path should not be overwritten
-      expect(request.query).not.to.contain({ foo: "bar" });
-      expect(request.path).not.to.equal("/path");
+      expect(request.path).to.equal("/path/bucket/key");
       return Promise.resolve({ output: {} as any, response: {} as any });
     };
-    const client = new S3({ endpoint: "http://localhost:8080/path?foo=bar" });
+    const client = new S3({ endpoint: "http://localhost:8080/path", forcePathStyle: true });
+
     client.middlewareStack.add(endpointValidator, {
       step: "serialize",
       name: "endpointValidator",

diff --git a/...in/java/software/amazon/smithy/aws/typescript/codegen/AddEndpointsV2ParameterNameMap.java b/...in/java/software/amazon/smithy/aws/typescript/codegen/AddEndpointsV2ParameterNameMap.java
@@ -30,7 +30,10 @@ public AddEndpointsV2ParameterNameMap() {
             "ForcePathStyle", "forcePathStyle",
             "Accelerate", "useAccelerateEndpoint",
             "DisableMRAP", "disableMultiregionAccessPoints",
-            "UseArnRegion", "useArnRegion"
+            "DisableMultiRegionAccessPoints", "disableMultiregionAccessPoints",
+            "UseArnRegion", "useArnRegion",
+            "Endpoint", "endpoint",
+            "UseGlobalEndpoint", "useGlobalEndpoint"
         ));
     }
 }
diff --git a/packages/util-endpoints/src/lib/parseURL.spec.ts b/packages/util-endpoints/src/lib/parseURL.spec.ts
@@ -1,23 +1,42 @@
+import { Endpoint, EndpointURL, EndpointURLScheme } from "@aws-sdk/types";
+
 import { parseURL } from "./parseURL";
 
 describe(parseURL.name, () => {
-  it.each([
-    ["https://example.com", { scheme: "https", authority: "example.com", path: "/", normalizedPath: "/", isIp: false }],
+  const testCases: [string, EndpointURL][] = [
+    [
+      "https://example.com",
+      { scheme: EndpointURLScheme.HTTPS, authority: "example.com", path: "/", normalizedPath: "/", isIp: false },
+    ],
     [
       "http://example.com:80/foo/bar",
-      { scheme: "http", authority: "example.com:80", path: "/foo/bar", normalizedPath: "/foo/bar/", isIp: false },
+      {
+        scheme: EndpointURLScheme.HTTP,
+        authority: "example.com:80",
+        path: "/foo/bar",
+        normalizedPath: "/foo/bar/",
+        isIp: false,
+      },
+    ],
+    [
+      "https://127.0.0.1",
+      { scheme: EndpointURLScheme.HTTPS, authority: "127.0.0.1", path: "/", normalizedPath: "/", isIp: true },
     ],
-    ["https://127.0.0.1", { scheme: "https", authority: "127.0.0.1", path: "/", normalizedPath: "/", isIp: true }],
     [
       "https://127.0.0.1:8443",
-      { scheme: "https", authority: "127.0.0.1:8443", path: "/", normalizedPath: "/", isIp: true },
+      { scheme: EndpointURLScheme.HTTPS, authority: "127.0.0.1:8443", path: "/", normalizedPath: "/", isIp: true },
+    ],
+    [
+      "https://[fe80::1]",
+      { scheme: EndpointURLScheme.HTTPS, authority: "[fe80::1]", path: "/", normalizedPath: "/", isIp: true },
     ],
-    ["https://[fe80::1]", { scheme: "https", authority: "[fe80::1]", path: "/", normalizedPath: "/", isIp: true }],
     [
       "https://[fe80::1]:8443",
-      { scheme: "https", authority: "[fe80::1]:8443", path: "/", normalizedPath: "/", isIp: true },
+      { scheme: EndpointURLScheme.HTTPS, authority: "[fe80::1]:8443", path: "/", normalizedPath: "/", isIp: true },
     ],
-  ])("test '%s'", (input, output) => {
+  ];
+
+  it.each(testCases)("test '%s'", (input: string, output: EndpointURL) => {
     expect(parseURL(input)).toEqual(output);
   });
 
@@ -32,4 +51,27 @@ describe(parseURL.name, () => {
   it("returns null for invalid URL", () => {
     expect(parseURL("invalid")).toBeNull();
   });
+
+  it.each(testCases)("test as URL '%s'", (input: string, output: EndpointURL) => {
+    const url = new URL(input);
+    expect(parseURL(url)).toEqual({
+      ...output,
+      authority: url.hostname + (url.port ? `:${url.port}` : ""),
+    });
+  });
+
+  it.each(testCases)("test as EndpointV1 '%s'", (input: string, output: EndpointURL) => {
+    const url = new URL(input);
+    const endpointV1: Endpoint = {
+      protocol: url.protocol,
+      hostname: url.hostname,
+      port: url.port ? Number(url.port) : undefined,
+      path: url.pathname,
+    };
+
+    expect(parseURL(endpointV1)).toEqual({
+      ...output,
+      authority: url.hostname + (url.port ? `:${url.port}` : ""),
+    });
+  });
 });
diff --git a/packages/util-endpoints/src/lib/parseURL.ts b/packages/util-endpoints/src/lib/parseURL.ts
@@ -1,4 +1,4 @@
-import { EndpointURL, EndpointURLScheme } from "@aws-sdk/types";
+import { Endpoint, EndpointURL, EndpointURLScheme } from "@aws-sdk/types";
 
 import { isIpAddress } from "./isIpAddress";
 
@@ -8,21 +8,35 @@ const DEFAULT_PORTS: Record<EndpointURLScheme, number> = {
 };
 
 /**
- * Parses a string into it’s Endpoint URL components.
+ * Parses a string, URL, or Endpoint into it’s Endpoint URL components.
  */
-export const parseURL = (value: string): EndpointURL | null => {
+export const parseURL = (value: string | URL | Endpoint): EndpointURL | null => {
   const whatwgURL = (() => {
     try {
+      if (value instanceof URL) {
+        return value;
+      }
+      if (typeof value === "object" && "hostname" in value) {
+        const { hostname, port, protocol = "", path = "", query = {} } = value as Endpoint;
+        const url = new URL(`${protocol}//${hostname}${port ? `:${port}` : ""}${path}`);
+        url.search = Object.entries(query)
+          .map(([k, v]) => `${k}=${v}`)
+          .join("&");
+        return url;
+      }
       return new URL(value);
     } catch (error) {
       return null;
     }
   })();
 
   if (!whatwgURL) {
+    console.error(`Unable to parse ${JSON.stringify(value)} as a whatwg URL.`);
     return null;
   }
 
+  const urlString = whatwgURL.href;
+
   const { host, hostname, pathname, protocol, search } = whatwgURL;
 
   if (search) {
@@ -35,7 +49,12 @@ export const parseURL = (value: string): EndpointURL | null => {
   }
 
   const isIp = isIpAddress(hostname);
-  const authority = `${host}${value.includes(`${host}:${DEFAULT_PORTS[scheme]}`) ? `:${DEFAULT_PORTS[scheme]}` : ``}`;
+
+  const inputContainsDefaultPort =
+    urlString.includes(`${host}:${DEFAULT_PORTS[scheme]}`) ||
+    (typeof value === "string" && value.includes(`${host}:${DEFAULT_PORTS[scheme]}`));
+
+  const authority = `${host}${inputContainsDefaultPort ? `:${DEFAULT_PORTS[scheme]}` : ``}`;
 
   return {
     scheme,