Skip to content

Commit f2874d5

Browse files
committed
fix(client-s3): address PR feedbacks
1 parent 674957d commit f2874d5

File tree

4 files changed

+52
-60
lines changed

4 files changed

+52
-60
lines changed

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ public final class AddS3Config implements TypeScriptIntegration {
6969
"CompleteMultipartUpload"
7070
);
7171

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

7676
@Override
7777
public Model preprocessModel(PluginContext context, TypeScriptSettings settings) {

packages/middleware-sdk-s3/src/S3SignatureV4.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export type S3SignerV4Init = SignatureV4Init &
2323
*/
2424
export class S3SignatureV4 implements RequestPresigner, RequestSigner {
2525
private readonly sigv4Signer: SignatureV4;
26-
private sigv4aSigner?: RequestSigner & RequestPresigner;
26+
private sigv4aSigner?: CrtSignerV4;
2727
private readonly signerOptions: S3SignerV4Init;
2828

2929
constructor(options: S3SignerV4Init) {
@@ -35,14 +35,7 @@ export class S3SignatureV4 implements RequestPresigner, RequestSigner {
3535
if (options.signingRegion === "*") {
3636
if (this.signerOptions.runtime !== "node")
3737
throw new Error("This request requires signing with SigV4Asymmetric algorithm. It's only available in Node.js");
38-
if (!this.sigv4aSigner) {
39-
const CrtSignerV4 = await expectSigv4aSigner();
40-
this.sigv4aSigner = new CrtSignerV4({
41-
...this.signerOptions,
42-
signingAlgorithm: 1,
43-
});
44-
}
45-
return this.sigv4aSigner.sign(requestToSign, options);
38+
return (await this.getSigv4aSigner()).sign(requestToSign, options);
4639
}
4740
return this.sigv4Signer.sign(requestToSign, options);
4841
}
@@ -51,26 +44,27 @@ export class S3SignatureV4 implements RequestPresigner, RequestSigner {
5144
if (options.signingRegion === "*") {
5245
if (this.signerOptions.runtime !== "node")
5346
throw new Error("This request requires signing with SigV4Asymmetric algorithm. It's only available in Node.js");
54-
if (!this.sigv4aSigner) {
55-
const CrtSignerV4 = await expectSigv4aSigner();
56-
this.sigv4aSigner = new CrtSignerV4({
57-
...this.signerOptions,
58-
signingAlgorithm: 1,
59-
});
60-
}
61-
return this.sigv4aSigner.presign(originalRequest, options);
47+
return (await this.getSigv4aSigner()).presign(originalRequest, options);
6248
}
6349
return this.sigv4Signer.presign(originalRequest, options);
6450
}
65-
}
6651

67-
const expectSigv4aSigner = async (): Promise<new (options: CrtSignerV4Init & SignatureV4CryptoInit) => CrtSignerV4> => {
68-
try {
69-
return (await import("@aws-sdk/signature-v4-crt")).CrtSignerV4;
70-
} catch (e) {
71-
e.message =
72-
`${e.message}\nPlease check if you have installed "@aws-sdk/signature-v4-crt" package explicitly. ` +
73-
"For more information please go to https://github.com/aws/aws-sdk-js-v3#known-issues";
74-
throw e;
52+
private async getSigv4aSigner(): Promise<CrtSignerV4> {
53+
if (!this.sigv4aSigner) {
54+
let CrtSignerV4: new (options: CrtSignerV4Init & SignatureV4CryptoInit) => CrtSignerV4;
55+
try {
56+
CrtSignerV4 = (await import("@aws-sdk/signature-v4-crt")).CrtSignerV4;
57+
} catch (e) {
58+
e.message =
59+
`${e.message}\nPlease check if you have installed "@aws-sdk/signature-v4-crt" package explicitly. \n` +
60+
"For more information please go to https://github.com/aws/aws-sdk-js-v3#known-issues";
61+
throw e;
62+
}
63+
this.sigv4aSigner = new CrtSignerV4({
64+
...this.signerOptions,
65+
signingAlgorithm: 1,
66+
});
67+
}
68+
return this.sigv4aSigner;
7569
}
76-
};
70+
}

packages/middleware-user-agent/src/user-agent-middleware.ts

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,39 +31,38 @@ export const userAgentMiddleware =
3131
<Output extends MetadataBearer>(
3232
next: BuildHandler<any, any>,
3333
context: HandlerExecutionContext
34-
): BuildHandler<any, any> => {
35-
return async (args: BuildHandlerArguments<any>): Promise<BuildHandlerOutput<Output>> => {
36-
const { request } = args;
37-
if (!HttpRequest.isInstance(request)) return next(args);
38-
const { headers } = request;
39-
const userAgent = context?.userAgent?.map(escapeUserAgent) || [];
40-
const defaultUserAgent = (await options.defaultUserAgentProvider()).map(escapeUserAgent);
41-
const customUserAgent = options?.customUserAgent?.map(escapeUserAgent) || [];
34+
): BuildHandler<any, any> =>
35+
async (args: BuildHandlerArguments<any>): Promise<BuildHandlerOutput<Output>> => {
36+
const { request } = args;
37+
if (!HttpRequest.isInstance(request)) return next(args);
38+
const { headers } = request;
39+
const userAgent = context?.userAgent?.map(escapeUserAgent) || [];
40+
const defaultUserAgent = (await options.defaultUserAgentProvider()).map(escapeUserAgent);
41+
const customUserAgent = options?.customUserAgent?.map(escapeUserAgent) || [];
4242

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

51-
if (options.runtime !== "browser") {
52-
if (normalUAValue) {
53-
headers[X_AMZ_USER_AGENT] = headers[X_AMZ_USER_AGENT]
54-
? `${headers[USER_AGENT]} ${normalUAValue}`
55-
: normalUAValue;
56-
}
57-
headers[USER_AGENT] = sdkUserAgentValue;
58-
} else {
59-
headers[X_AMZ_USER_AGENT] = sdkUserAgentValue;
51+
if (options.runtime !== "browser") {
52+
if (normalUAValue) {
53+
headers[X_AMZ_USER_AGENT] = headers[X_AMZ_USER_AGENT]
54+
? `${headers[USER_AGENT]} ${normalUAValue}`
55+
: normalUAValue;
6056
}
57+
headers[USER_AGENT] = sdkUserAgentValue;
58+
} else {
59+
headers[X_AMZ_USER_AGENT] = sdkUserAgentValue;
60+
}
6161

62-
return next({
63-
...args,
64-
request,
65-
});
66-
};
62+
return next({
63+
...args,
64+
request,
65+
});
6766
};
6867

6968
/**

packages/util-user-agent-node/src/is-crt-available.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ export const isCrtAvailable = (): UserAgentPair | null => {
1313
}
1414
return null;
1515
} catch (e) {
16-
console.log("||||||", process.versions, e);
1716
// No aws-crt package available in the runtime.
1817
return null;
1918
}

0 commit comments

Comments
 (0)