Skip to content

Commit

Permalink
fix(client-s3): address PR feedbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
AllanZhengYP committed Sep 20, 2021
1 parent 674957d commit d42f69e
Show file tree
Hide file tree
Showing 6 changed files with 228 additions and 148 deletions.
252 changes: 168 additions & 84 deletions clients/client-s3/models/models_0.ts

Large diffs are not rendered by default.

12 changes: 8 additions & 4 deletions clients/client-s3/models/models_1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,8 @@ export interface RestoreObjectRequest {
* <p>The bucket name containing the object to restore. </p>
* <p>When using this action with an access point, you must direct requests to the access point hostname. The access point hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.s3-accesspoint.<i>Region</i>.amazonaws.com. When using this action with an access point through the Amazon Web Services SDKs, you provide the access point ARN in place of the bucket name. For more information about access point ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-access-points.html">Using access points</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>When using this action with Amazon S3 on Outposts, you must direct requests to the S3 on Outposts hostname. The S3 on Outposts hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.<i>outpostID</i>.s3-outposts.<i>Region</i>.amazonaws.com. When using this action using S3 on Outposts through the Amazon Web Services SDKs, you provide the Outposts bucket ARN in place of the bucket name. For more information about S3 on Outposts ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/S3onOutposts.html">Using S3 on Outposts</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>Note: To supply the Multi-region Access Point(MRAP) to Bucket, you need to install the "aws-crt" package sparately. For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
* <p>Note: To supply the Multi-region Access Point (MRAP) to Bucket, you need to install the "@aws-sdk/signature-v4-crt" package to your project dependencies.
* For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
*/
Bucket: string | undefined;

Expand Down Expand Up @@ -899,7 +900,8 @@ export namespace ScanRange {
export interface SelectObjectContentRequest {
/**
* <p>The S3 bucket.</p>
* <p>Note: To supply the Multi-region Access Point(MRAP) to Bucket, you need to install the "aws-crt" package sparately. For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
* <p>Note: To supply the Multi-region Access Point (MRAP) to Bucket, you need to install the "@aws-sdk/signature-v4-crt" package to your project dependencies.
* For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
*/
Bucket: string | undefined;

Expand Down Expand Up @@ -1057,7 +1059,8 @@ export interface UploadPartRequest {
* <p>The name of the bucket to which the multipart upload was initiated.</p>
* <p>When using this action with an access point, you must direct requests to the access point hostname. The access point hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.s3-accesspoint.<i>Region</i>.amazonaws.com. When using this action with an access point through the Amazon Web Services SDKs, you provide the access point ARN in place of the bucket name. For more information about access point ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-access-points.html">Using access points</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>When using this action with Amazon S3 on Outposts, you must direct requests to the S3 on Outposts hostname. The S3 on Outposts hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.<i>outpostID</i>.s3-outposts.<i>Region</i>.amazonaws.com. When using this action using S3 on Outposts through the Amazon Web Services SDKs, you provide the Outposts bucket ARN in place of the bucket name. For more information about S3 on Outposts ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/S3onOutposts.html">Using S3 on Outposts</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>Note: To supply the Multi-region Access Point(MRAP) to Bucket, you need to install the "aws-crt" package sparately. For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
* <p>Note: To supply the Multi-region Access Point (MRAP) to Bucket, you need to install the "@aws-sdk/signature-v4-crt" package to your project dependencies.
* For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
*/
Bucket: string | undefined;

Expand Down Expand Up @@ -1224,7 +1227,8 @@ export interface UploadPartCopyRequest {
* <p>The bucket name.</p>
* <p>When using this action with an access point, you must direct requests to the access point hostname. The access point hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.s3-accesspoint.<i>Region</i>.amazonaws.com. When using this action with an access point through the Amazon Web Services SDKs, you provide the access point ARN in place of the bucket name. For more information about access point ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-access-points.html">Using access points</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>When using this action with Amazon S3 on Outposts, you must direct requests to the S3 on Outposts hostname. The S3 on Outposts hostname takes the form <i>AccessPointName</i>-<i>AccountId</i>.<i>outpostID</i>.s3-outposts.<i>Region</i>.amazonaws.com. When using this action using S3 on Outposts through the Amazon Web Services SDKs, you provide the Outposts bucket ARN in place of the bucket name. For more information about S3 on Outposts ARNs, see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/S3onOutposts.html">Using S3 on Outposts</a> in the <i>Amazon S3 User Guide</i>.</p>
* <p>Note: To supply the Multi-region Access Point(MRAP) to Bucket, you need to install the "aws-crt" package sparately. For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
* <p>Note: To supply the Multi-region Access Point (MRAP) to Bucket, you need to install the "@aws-sdk/signature-v4-crt" package to your project dependencies.
* For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>
*/
Bucket: string | undefined;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public final class AddS3Config implements TypeScriptIntegration {
"CompleteMultipartUpload"
);

private static final String CRT_NOTIFICATION = "<p>Note: To supply the Multi-region Access Point(MRAP) to Bucket,"
+ " you need to install the \"aws-crt\" package sparately. For more information, please go to "
+ "https://github.com/aws/aws-sdk-js-v3#known-issues</p>";
private static final String CRT_NOTIFICATION = "<p>Note: To supply the Multi-region Access Point (MRAP) to Bucket,"
+ " you need to install the \"@aws-sdk/signature-v4-crt\" package to your project dependencies. \n"
+ "For more information, please go to https://github.com/aws/aws-sdk-js-v3#known-issues</p>";

@Override
public Model preprocessModel(PluginContext context, TypeScriptSettings settings) {
Expand Down
48 changes: 21 additions & 27 deletions packages/middleware-sdk-s3/src/S3SignatureV4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export type S3SignerV4Init = SignatureV4Init &
*/
export class S3SignatureV4 implements RequestPresigner, RequestSigner {
private readonly sigv4Signer: SignatureV4;
private sigv4aSigner?: RequestSigner & RequestPresigner;
private sigv4aSigner?: CrtSignerV4;
private readonly signerOptions: S3SignerV4Init;

constructor(options: S3SignerV4Init) {
Expand All @@ -35,14 +35,7 @@ export class S3SignatureV4 implements RequestPresigner, RequestSigner {
if (options.signingRegion === "*") {
if (this.signerOptions.runtime !== "node")
throw new Error("This request requires signing with SigV4Asymmetric algorithm. It's only available in Node.js");
if (!this.sigv4aSigner) {
const CrtSignerV4 = await expectSigv4aSigner();
this.sigv4aSigner = new CrtSignerV4({
...this.signerOptions,
signingAlgorithm: 1,
});
}
return this.sigv4aSigner.sign(requestToSign, options);
return (await this.getSigv4aSigner()).sign(requestToSign, options);
}
return this.sigv4Signer.sign(requestToSign, options);
}
Expand All @@ -51,26 +44,27 @@ export class S3SignatureV4 implements RequestPresigner, RequestSigner {
if (options.signingRegion === "*") {
if (this.signerOptions.runtime !== "node")
throw new Error("This request requires signing with SigV4Asymmetric algorithm. It's only available in Node.js");
if (!this.sigv4aSigner) {
const CrtSignerV4 = await expectSigv4aSigner();
this.sigv4aSigner = new CrtSignerV4({
...this.signerOptions,
signingAlgorithm: 1,
});
}
return this.sigv4aSigner.presign(originalRequest, options);
return (await this.getSigv4aSigner()).presign(originalRequest, options);
}
return this.sigv4Signer.presign(originalRequest, options);
}
}

const expectSigv4aSigner = async (): Promise<new (options: CrtSignerV4Init & SignatureV4CryptoInit) => CrtSignerV4> => {
try {
return (await import("@aws-sdk/signature-v4-crt")).CrtSignerV4;
} catch (e) {
e.message =
`${e.message}\nPlease check if you have installed "@aws-sdk/signature-v4-crt" package explicitly. ` +
"For more information please go to https://github.com/aws/aws-sdk-js-v3#known-issues";
throw e;
private async getSigv4aSigner(): Promise<CrtSignerV4> {
if (!this.sigv4aSigner) {
let CrtSignerV4: new (options: CrtSignerV4Init & SignatureV4CryptoInit) => CrtSignerV4;
try {
CrtSignerV4 = (await import("@aws-sdk/signature-v4-crt")).CrtSignerV4;
} catch (e) {
e.message =
`${e.message}\nPlease check if you have installed "@aws-sdk/signature-v4-crt" package explicitly. \n` +
"For more information please go to https://github.com/aws/aws-sdk-js-v3#known-issues";
throw e;
}
this.sigv4aSigner = new CrtSignerV4({
...this.signerOptions,
signingAlgorithm: 1,
});
}
return this.sigv4aSigner;
}
};
}
57 changes: 28 additions & 29 deletions packages/middleware-user-agent/src/user-agent-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,38 @@ export const userAgentMiddleware =
<Output extends MetadataBearer>(
next: BuildHandler<any, any>,
context: HandlerExecutionContext
): BuildHandler<any, any> => {
return async (args: BuildHandlerArguments<any>): Promise<BuildHandlerOutput<Output>> => {
const { request } = args;
if (!HttpRequest.isInstance(request)) return next(args);
const { headers } = request;
const userAgent = context?.userAgent?.map(escapeUserAgent) || [];
const defaultUserAgent = (await options.defaultUserAgentProvider()).map(escapeUserAgent);
const customUserAgent = options?.customUserAgent?.map(escapeUserAgent) || [];
): BuildHandler<any, any> =>
async (args: BuildHandlerArguments<any>): Promise<BuildHandlerOutput<Output>> => {
const { request } = args;
if (!HttpRequest.isInstance(request)) return next(args);
const { headers } = request;
const userAgent = context?.userAgent?.map(escapeUserAgent) || [];
const defaultUserAgent = (await options.defaultUserAgentProvider()).map(escapeUserAgent);
const customUserAgent = options?.customUserAgent?.map(escapeUserAgent) || [];

// Set value to AWS-specific user agent header
const sdkUserAgentValue = [...defaultUserAgent, ...userAgent, ...customUserAgent].join(SPACE);
// Get value to be sent with non-AWS-specific user agent header.
const normalUAValue = [
...defaultUserAgent.filter((section) => section.startsWith("aws-sdk-")),
...customUserAgent,
].join(SPACE);
// Set value to AWS-specific user agent header
const sdkUserAgentValue = [...defaultUserAgent, ...userAgent, ...customUserAgent].join(SPACE);
// Get value to be sent with non-AWS-specific user agent header.
const normalUAValue = [
...defaultUserAgent.filter((section) => section.startsWith("aws-sdk-")),
...customUserAgent,
].join(SPACE);

if (options.runtime !== "browser") {
if (normalUAValue) {
headers[X_AMZ_USER_AGENT] = headers[X_AMZ_USER_AGENT]
? `${headers[USER_AGENT]} ${normalUAValue}`
: normalUAValue;
}
headers[USER_AGENT] = sdkUserAgentValue;
} else {
headers[X_AMZ_USER_AGENT] = sdkUserAgentValue;
if (options.runtime !== "browser") {
if (normalUAValue) {
headers[X_AMZ_USER_AGENT] = headers[X_AMZ_USER_AGENT]
? `${headers[USER_AGENT]} ${normalUAValue}`
: normalUAValue;
}
headers[USER_AGENT] = sdkUserAgentValue;
} else {
headers[X_AMZ_USER_AGENT] = sdkUserAgentValue;
}

return next({
...args,
request,
});
};
return next({
...args,
request,
});
};

/**
Expand Down
1 change: 0 additions & 1 deletion packages/util-user-agent-node/src/is-crt-available.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export const isCrtAvailable = (): UserAgentPair | null => {
}
return null;
} catch (e) {
console.log("||||||", process.versions, e);
// No aws-crt package available in the runtime.
return null;
}
Expand Down

0 comments on commit d42f69e

Please sign in to comment.