Skip to content

Commit

Permalink
[core] Fix handling of binary data in core-client-rest (Azure#28054)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
timovv authored Dec 22, 2023
1 parent ec0f34d commit c4825cc
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 41 deletions.
2 changes: 2 additions & 0 deletions sdk/core/core-client-rest/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
14 changes: 0 additions & 14 deletions sdk/core/core-client-rest/src/helpers/getBinaryBody.ts

This file was deleted.

15 changes: 5 additions & 10 deletions sdk/core/core-client-rest/src/sendRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down
14 changes: 0 additions & 14 deletions sdk/core/core-client-rest/test/getBinaryBody.spec.ts

This file was deleted.

10 changes: 7 additions & 3 deletions sdk/core/core-client-rest/test/sendRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
};

Expand Down

0 comments on commit c4825cc

Please sign in to comment.