From c4825ccb7339d2cf6559236ab0a53483b6dd2ca5 Mon Sep 17 00:00:00 2001 From: Timo van Veenendaal Date: Sat, 23 Dec 2023 12:27:32 +1300 Subject: [PATCH] [core] Fix handling of binary data in core-client-rest (#28054) ### Packages impacted by this PR - `@azure-rest/core-client` ### Describe the problem that is addressed by this PR - Remove conversion of Uint8Array to binary string with `application/octet-stream` content type - `core-rest-pipeline` handles Uint8Array as a request body type so this conversion should not be necessary - Remove conversion of Uint8Array to binary string in `multipart/form-data` requests and convert to `Blob` instead - With the new multipart work, support for `Blob` is much improved and this workaround should no longer be required. --- sdk/core/core-client-rest/CHANGELOG.md | 2 ++ .../core-client-rest/src/helpers/getBinaryBody.ts | 14 -------------- sdk/core/core-client-rest/src/sendRequest.ts | 15 +++++---------- .../core-client-rest/test/getBinaryBody.spec.ts | 14 -------------- .../core-client-rest/test/sendRequest.spec.ts | 10 +++++++--- 5 files changed, 14 insertions(+), 41 deletions(-) delete mode 100644 sdk/core/core-client-rest/src/helpers/getBinaryBody.ts delete mode 100644 sdk/core/core-client-rest/test/getBinaryBody.spec.ts diff --git a/sdk/core/core-client-rest/CHANGELOG.md b/sdk/core/core-client-rest/CHANGELOG.md index 84ecbf92b9f5..9b8029036548 100644 --- a/sdk/core/core-client-rest/CHANGELOG.md +++ b/sdk/core/core-client-rest/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +- Fix serialization of binary data in `multipart/form-data` requests and in binary request bodies. + ### Other Changes - Upgrade dependency `@azure/abort-controller` to `^2.0.0`. diff --git a/sdk/core/core-client-rest/src/helpers/getBinaryBody.ts b/sdk/core/core-client-rest/src/helpers/getBinaryBody.ts deleted file mode 100644 index 17731d1b8005..000000000000 --- a/sdk/core/core-client-rest/src/helpers/getBinaryBody.ts +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -/** - * Converts binary content to its string representation - */ -export function binaryArrayToString(content: Uint8Array): string { - let decodedBody = ""; - for (const element of content) { - decodedBody += String.fromCharCode(element); - } - - return decodedBody; -} diff --git a/sdk/core/core-client-rest/src/sendRequest.ts b/sdk/core/core-client-rest/src/sendRequest.ts index ea205d771f9d..69b6edc3465d 100644 --- a/sdk/core/core-client-rest/src/sendRequest.ts +++ b/sdk/core/core-client-rest/src/sendRequest.ts @@ -17,7 +17,6 @@ import { import { getCachedDefaultHttpsClient } from "./clientHelpers"; import { isReadableStream } from "./helpers/isReadableStream"; import { HttpResponse, RequestParameters } from "./common"; -import { binaryArrayToString } from "./helpers/getBinaryBody"; /** * Helper function to send request used by the client @@ -173,13 +172,7 @@ function getRequestBody(body?: unknown, contentType: string = ""): RequestBody { } if (ArrayBuffer.isView(body)) { - if (body instanceof Uint8Array) { - return firstType === "application/octet-stream" - ? { body } - : { body: binaryArrayToString(body) }; - } else { - return { body: JSON.stringify(body) }; - } + return { body: body instanceof Uint8Array ? body : JSON.stringify(body) }; } switch (firstType) { @@ -202,7 +195,7 @@ function isFormData(body: unknown): body is FormDataMap { } /** - * Checks if binary data is in Uint8Array format, if so decode it to a binary string + * Checks if binary data is in Uint8Array format, if so wrap it in a Blob * to send over the wire */ function processFormData(formData?: FormDataMap) { @@ -215,7 +208,9 @@ function processFormData(formData?: FormDataMap) { for (const element in formData) { const item = formData[element]; if (item instanceof Uint8Array) { - processedFormData[element] = binaryArrayToString(item); + // Some RLCs take a Uint8Array for the parameter, whereas FormDataMap expects + // a File or a Blob, so we need to wrap it. + processedFormData[element] = new Blob([item]); } else { processedFormData[element] = item; } diff --git a/sdk/core/core-client-rest/test/getBinaryBody.spec.ts b/sdk/core/core-client-rest/test/getBinaryBody.spec.ts deleted file mode 100644 index fcbffd26ae10..000000000000 --- a/sdk/core/core-client-rest/test/getBinaryBody.spec.ts +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { assert } from "chai"; -import { binaryArrayToString } from "../src/helpers/getBinaryBody"; - -describe("getBinaryBody", () => { - describe("decodeBinaryContent", () => { - it("should handle uint8array content", () => { - const decoded = binaryArrayToString(new Uint8Array([0x66, 0x6f, 0x6f])); - assert.equal(decoded, "foo"); - }); - }); -}); diff --git a/sdk/core/core-client-rest/test/sendRequest.spec.ts b/sdk/core/core-client-rest/test/sendRequest.spec.ts index d1e148946a52..93312dd91656 100644 --- a/sdk/core/core-client-rest/test/sendRequest.spec.ts +++ b/sdk/core/core-client-rest/test/sendRequest.spec.ts @@ -18,9 +18,8 @@ describe("sendRequest", () => { describe("Binary content", () => { it("should handle request body as Uint8Array", async () => { const mockPipeline: Pipeline = createEmptyPipeline(); - const expectedBody = "foo"; mockPipeline.sendRequest = async (_client, request) => { - assert.equal(request.body, expectedBody); + assert.sameOrderedMembers([...(request.body as Uint8Array)], [...foo]); return { headers: createHttpHeaders() } as PipelineResponse; }; @@ -70,7 +69,12 @@ describe("sendRequest", () => { const expectedFormData = { fileName: "foo.txt", file: "foo" }; const mockPipeline: Pipeline = createEmptyPipeline(); mockPipeline.sendRequest = async (_client, request) => { - assert.deepEqual(request.formData, expectedFormData); + assert.equal(request.formData?.fileName, "foo.txt"); + assert.instanceOf(request.formData?.file, Blob); + assert.sameOrderedMembers( + [...new Uint8Array(await (request.formData?.file as Blob).arrayBuffer())], + [...foo] + ); return { headers: createHttpHeaders() } as PipelineResponse; };