From 947c8bce2798ae5ddc022d34f62aeeb60b4e6fde Mon Sep 17 00:00:00 2001 From: George Fu Date: Tue, 18 Oct 2022 15:27:01 -0400 Subject: [PATCH] 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 --- clients/client-s3/test/S3.spec.ts | 7 +-- .../AddEndpointsV2ParameterNameMap.java | 5 +- .../util-endpoints/src/lib/parseURL.spec.ts | 58 ++++++++++++++++--- packages/util-endpoints/src/lib/parseURL.ts | 27 +++++++-- 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/clients/client-s3/test/S3.spec.ts b/clients/client-s3/test/S3.spec.ts index 06166d90192c..f76b9f1a9666 100644 --- 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/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddEndpointsV2ParameterNameMap.java b/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddEndpointsV2ParameterNameMap.java index b36b75088ab6..3376a7f370e0 100644 --- a/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddEndpointsV2ParameterNameMap.java +++ b/codegen/smithy-aws-typescript-codegen/src/main/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 index e3058b3c0cff..a8f030f74d5a 100644 --- 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 index b5789ea4d8b9..4cf30fe253dd 100644 --- 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,11 +8,22 @@ const DEFAULT_PORTS: Record = { }; /** - * 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; @@ -20,9 +31,12 @@ export const parseURL = (value: string): EndpointURL | 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,