New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(endpoint): dedupe clientContext/builtIn params, fix s3 unit test #4051
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,10 @@ public AddEndpointsV2ParameterNameMap() { | |
"ForcePathStyle", "forcePathStyle", | ||
"Accelerate", "useAccelerateEndpoint", | ||
"DisableMRAP", "disableMultiregionAccessPoints", | ||
"UseArnRegion", "useArnRegion" | ||
"DisableMultiRegionAccessPoints", "disableMultiregionAccessPoints", | ||
"UseArnRegion", "useArnRegion", | ||
"Endpoint", "endpoint", | ||
"UseGlobalEndpoint", "useGlobalEndpoint" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm setting more param names to match our existing param names. |
||
)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 => { | ||
kuhe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is to cover deprecated behavior: Here we convert it back to a URL to enforce consistency in the endpoints 2.0 resolver. |
||
return new URL(value); | ||
} catch (error) { | ||
return null; | ||
} | ||
})(); | ||
|
||
if (!whatwgURL) { | ||
console.error(`Unable to parse ${JSON.stringify(value)} as a whatwg URL.`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a message here because the endpoints resolved error gives too little info. |
||
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, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we no longer test the omission of the query param, because endpoints 2.0 requires that a query param custom endpoint be rejected instead of modified.