From ca3fc95d0d3c4e8a35e3402471a59d65681433cd Mon Sep 17 00:00:00 2001 From: George Fu Date: Mon, 24 Jun 2024 21:12:16 +0000 Subject: [PATCH] address pr comments --- packages/core/package.json | 1 + .../core/src/submodules/cbor/cbor-decode.ts | 10 ++++- .../core/src/submodules/cbor/cbor.spec.ts | 2 +- .../core/src/submodules/cbor/parseCborBody.ts | 40 ++++++++++++++++++- .../src/protocols/Rpcv2cbor.ts | 29 +------------- .../typescript/codegen/TypeScriptWriter.java | 4 +- .../protocols/cbor/SmithyRpcV2Cbor.java | 36 ++--------------- yarn.lock | 3 +- 8 files changed, 56 insertions(+), 69 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 30f8b7314f3..b72e5ba635f 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -51,6 +51,7 @@ "@smithy/protocol-http": "workspace:^", "@smithy/smithy-client": "workspace:^", "@smithy/types": "workspace:^", + "@smithy/util-body-length-browser": "workspace:^", "@smithy/util-middleware": "workspace:^", "@smithy/util-utf8": "workspace:^", "tslib": "^2.6.2" diff --git a/packages/core/src/submodules/cbor/cbor-decode.ts b/packages/core/src/submodules/cbor/cbor-decode.ts index 603f2634141..4f36d69ad2c 100644 --- a/packages/core/src/submodules/cbor/cbor-decode.ts +++ b/packages/core/src/submodules/cbor/cbor-decode.ts @@ -167,6 +167,8 @@ function bytesToUtf8(bytes: Uint8Array, at: number, to: number): string { } function demote(bigInteger: bigint): number { + // cast is safe for string and array lengths, which do not + // exceed safe integer range. const num = Number(bigInteger); if (num < Number.MIN_SAFE_INTEGER || Number.MAX_SAFE_INTEGER < num) { console.warn(new Error(`@smithy/core/cbor - truncating BigInt(${bigInteger}) to ${num} with loss of precision.`)); @@ -297,7 +299,9 @@ function decodeUtf8StringIndefinite(at: Uint32, to: Uint32): string { const bytes = decodeUnstructuredByteString(at, to); const length = _offset; at += length; - vector.push(...bytes); + for (let i = 0; i < bytes.length; ++i) { + vector.push(bytes[i]); + } } throw new Error("expected break marker."); } @@ -340,7 +344,9 @@ function decodeUnstructuredByteStringIndefinite(at: Uint32, to: Uint32): CborUns const bytes = decodeUnstructuredByteString(at, to); const length = _offset; at += length; - vector.push(...bytes); + for (let i = 0; i < bytes.length; ++i) { + vector.push(bytes[i]); + } } throw new Error("expected break marker."); } diff --git a/packages/core/src/submodules/cbor/cbor.spec.ts b/packages/core/src/submodules/cbor/cbor.spec.ts index d0532475625..9345b4bff81 100644 --- a/packages/core/src/submodules/cbor/cbor.spec.ts +++ b/packages/core/src/submodules/cbor/cbor.spec.ts @@ -18,7 +18,7 @@ const successTests = JSONbig({ useNativeBigInt: true, alwaysParseAsBig: false }) describe("cbor", () => { const allocByteArray = (dataOrSize: ArrayBuffer | ArrayLike | number, offset?: number, length?: number) => { if (typeof offset === "number" && typeof length === "number") { - typeof Buffer !== "undefined" + return typeof Buffer !== "undefined" ? Buffer.from(dataOrSize as ArrayBuffer, offset, length) : new Uint8Array(dataOrSize as ArrayBuffer, offset, length); } diff --git a/packages/core/src/submodules/cbor/parseCborBody.ts b/packages/core/src/submodules/cbor/parseCborBody.ts index 8e30b19e1f4..32bbe0b7ba5 100644 --- a/packages/core/src/submodules/cbor/parseCborBody.ts +++ b/packages/core/src/submodules/cbor/parseCborBody.ts @@ -1,5 +1,7 @@ +import { HttpRequest as __HttpRequest } from "@smithy/protocol-http"; import { collectBody } from "@smithy/smithy-client"; -import type { HttpResponse, SerdeContext } from "@smithy/types"; +import { HeaderBag as __HeaderBag, HttpResponse, SerdeContext as __SerdeContext, SerdeContext } from "@smithy/types"; +import { calculateBodyLength } from "@smithy/util-body-length-browser"; import { cbor } from "./cbor"; @@ -71,8 +73,42 @@ export const loadSmithyRpcV2CborErrorCode = (output: HttpResponse, data: any): s } }; +/** + * @internal + */ export const checkCborResponse = (response: HttpResponse): void => { - if (response.headers["smithy-protocol"] !== "rpc-v2-cbor") { + if (String(response.headers["smithy-protocol"]).toLowerCase() !== "rpc-v2-cbor") { throw new Error("Malformed RPCv2 CBOR response, status: " + response.statusCode); } }; + +/** + * @internal + */ +export const buildHttpRpcRequest = async ( + context: __SerdeContext, + headers: __HeaderBag, + path: string, + resolvedHostname: string | undefined, + body: any +): Promise<__HttpRequest> => { + const { hostname, protocol = "https", port, path: basePath } = await context.endpoint(); + const contents: any = { + protocol, + hostname, + port, + method: "POST", + path: basePath.endsWith("/") ? basePath.slice(0, -1) + path : basePath + path, + headers, + }; + if (resolvedHostname !== undefined) { + contents.hostname = resolvedHostname; + } + if (body !== undefined) { + contents.body = body; + try { + contents.headers["content-length"] = String(calculateBodyLength(body)); + } catch (e) {} + } + return new __HttpRequest(contents); +}; diff --git a/private/smithy-rpcv2-cbor/src/protocols/Rpcv2cbor.ts b/private/smithy-rpcv2-cbor/src/protocols/Rpcv2cbor.ts index dc9e88eccd5..40608f7e532 100644 --- a/private/smithy-rpcv2-cbor/src/protocols/Rpcv2cbor.ts +++ b/private/smithy-rpcv2-cbor/src/protocols/Rpcv2cbor.ts @@ -53,6 +53,7 @@ import { } from "../models/models_0"; import { dateToTag as __dateToTag, + buildHttpRpcRequest, cbor, checkCborResponse as cr, loadSmithyRpcV2CborErrorCode, @@ -83,7 +84,6 @@ import { ResponseMetadata as __ResponseMetadata, SerdeContext as __SerdeContext, } from "@smithy/types"; -import { calculateBodyLength } from "@smithy/util-body-length-browser"; /** * serializeRpcv2cborEmptyInputOutputCommand @@ -1194,33 +1194,6 @@ const deserializeMetadata = (output: __HttpResponse): __ResponseMetadata => ({ }); const throwDefaultError = withBaseException(__BaseException); -const buildHttpRpcRequest = async ( - context: __SerdeContext, - headers: __HeaderBag, - path: string, - resolvedHostname: string | undefined, - body: any -): Promise<__HttpRequest> => { - const { hostname, protocol = "https", port, path: basePath } = await context.endpoint(); - const contents: any = { - protocol, - hostname, - port, - method: "POST", - path: basePath.endsWith("/") ? basePath.slice(0, -1) + path : basePath + path, - headers, - }; - if (resolvedHostname !== undefined) { - contents.hostname = resolvedHostname; - } - if (body !== undefined) { - contents.body = body; - try { - contents.headers["content-length"] = String(calculateBodyLength(body)); - } catch (e) {} - } - return new __HttpRequest(contents); -}; const SHARED_HEADERS: __HeaderBag = { "content-type": "application/cbor", "smithy-protocol": "rpc-v2-cbor", diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptWriter.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptWriter.java index d5a3b82ee59..eb5296bf491 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptWriter.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptWriter.java @@ -158,8 +158,8 @@ public TypeScriptWriter addImport(String name, String as, PackageContainer from) * submodule path, for example "@smithy/core/cbor". */ public TypeScriptWriter addImportSubmodule(String name, String as, PackageContainer from, String submodule) { - if (from instanceof Dependency) { - addDependency((Dependency) from); + if (from instanceof Dependency dependency) { + addDependency(dependency); } return this.addImport(name, as, from.getPackageName() + submodule); } diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/protocols/cbor/SmithyRpcV2Cbor.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/protocols/cbor/SmithyRpcV2Cbor.java index b02bef3730c..eb1eb42f366 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/protocols/cbor/SmithyRpcV2Cbor.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/protocols/cbor/SmithyRpcV2Cbor.java @@ -103,39 +103,9 @@ public void generateSharedComponents(GenerationContext context) { writer.addUseImports(requestType); writer.addImport("SerdeContext", "__SerdeContext", TypeScriptDependency.SMITHY_TYPES); writer.addImport("HeaderBag", "__HeaderBag", TypeScriptDependency.SMITHY_TYPES); - writer.openBlock(""" - const buildHttpRpcRequest = async ( - context: __SerdeContext, - headers: __HeaderBag, - path: string, - resolvedHostname: string | undefined, - body: any, - ): Promise<$T> => {""", "};", requestType, () -> { - writer.addImport("calculateBodyLength", null, TypeScriptDependency.AWS_SDK_UTIL_BODY_LENGTH_BROWSER); - writer.write(""" - const { hostname, protocol = "https", port, path: basePath } = await context.endpoint(); - const contents: any = { - protocol, - hostname, - port, - method: "POST", - path: basePath.endsWith("/") ? basePath.slice(0, -1) + path : basePath + path, - headers, - }; - if (resolvedHostname !== undefined) { - contents.hostname = resolvedHostname; - } - if (body !== undefined) { - contents.body = body; - try { - contents.headers["content-length"] = String(calculateBodyLength(body)); - } catch (e) {} - } - return new $T(contents); - """, - requestType - ); - } + writer.addImportSubmodule( + "buildHttpRpcRequest", null, + TypeScriptDependency.SMITHY_CORE, SmithyCoreSubmodules.CBOR ); writeSharedRequestHeaders(context); writer.write(""); diff --git a/yarn.lock b/yarn.lock index e5133f2d0aa..5dc46261aff 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2276,6 +2276,7 @@ __metadata: "@smithy/protocol-http": "workspace:^" "@smithy/smithy-client": "workspace:^" "@smithy/types": "workspace:^" + "@smithy/util-body-length-browser": "workspace:^" "@smithy/util-middleware": "workspace:^" "@smithy/util-utf8": "workspace:^" "@types/node": ^16.18.96 @@ -2881,7 +2882,7 @@ __metadata: languageName: unknown linkType: soft -"@smithy/util-body-length-browser@^3.0.0, @smithy/util-body-length-browser@workspace:packages/util-body-length-browser": +"@smithy/util-body-length-browser@^3.0.0, @smithy/util-body-length-browser@workspace:^, @smithy/util-body-length-browser@workspace:packages/util-body-length-browser": version: 0.0.0-use.local resolution: "@smithy/util-body-length-browser@workspace:packages/util-body-length-browser" dependencies: