Skip to content

Commit

Permalink
Fix handling of typed arrays in request bodies (#15904)
Browse files Browse the repository at this point in the history
NodeJS does not support directly passing typed arrays or ArrayBuffers to `http.ClientRequest` streams. This means we must correctly wrap these types in a `Buffer` for them to be serialized correctly.
  • Loading branch information
xirzec authored Jun 23, 2021
1 parent 6c59958 commit 137c671
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 1 deletion.
9 changes: 8 additions & 1 deletion sdk/core/core-rest-pipeline/src/nodeHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,14 @@ class NodeHttpClient implements HttpClient {
if (body && isReadableStream(body)) {
body.pipe(req);
} else if (body) {
req.end(body);
if (typeof body === "string" || Buffer.isBuffer(body)) {
req.end(body);
} else if (isArrayBuffer(body)) {
req.end(ArrayBuffer.isView(body) ? Buffer.from(body.buffer) : Buffer.from(body));
} else {
logger.error("Unrecognized body type", body);
throw new RestError("Unrecognized body type");
}
} else {
// streams don't like "undefined" being passed as data
req.end();
Expand Down
73 changes: 73 additions & 0 deletions sdk/core/core-rest-pipeline/test/node/nodeHttpClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ class FakeRequest extends PassThrough {
}
}

/**
* Generic NodeJS streams accept typed arrays just fine,
* but `http.ClientRequest` objects *only* support chunks
* of `Buffer` and `string`, so we must convert them first.
*
* This fake asserts we have only passed the correct types.
*/
const httpRequestChecker = {
on() {
/* no op */
},
end(chunk: unknown) {
const isString = typeof chunk === "string";
assert(isString || Buffer.isBuffer(chunk), "Expected either string or Buffer");
}
};

function createResponse(statusCode: number, body = ""): IncomingMessage {
const response = new FakeResponse();
response.headers = {};
Expand Down Expand Up @@ -259,4 +276,60 @@ describe("NodeHttpClient", function() {
assert.strictEqual(response.status, 200);
assert.strictEqual(response.bodyAsText, inputString);
});

it("should handle typed array bodies correctly", async function() {
const client = createDefaultHttpClient();
stubbedHttpsRequest.returns(httpRequestChecker);

const data = new Uint8Array(10);
for (let i = 0; i < 10; i++) {
data[i] = i;
}

const request = createPipelineRequest({ url: "https://example.com", body: data });
const promise = client.sendRequest(request);
stubbedHttpsRequest.yield(createResponse(200));
const response = await promise;
assert.strictEqual(response.status, 200);
});

it("should handle ArrayBuffer bodies correctly", async function() {
const client = createDefaultHttpClient();
stubbedHttpsRequest.returns(httpRequestChecker);

const data = new Uint8Array(10);
for (let i = 0; i < 10; i++) {
data[i] = i;
}

const request = createPipelineRequest({ url: "https://example.com", body: data.buffer });
const promise = client.sendRequest(request);
stubbedHttpsRequest.yield(createResponse(200));
const response = await promise;
assert.strictEqual(response.status, 200);
});

it("should handle Buffer bodies correctly", async function() {
const client = createDefaultHttpClient();
stubbedHttpsRequest.returns(httpRequestChecker);

const data = Buffer.from("example text");

const request = createPipelineRequest({ url: "https://example.com", body: data });
const promise = client.sendRequest(request);
stubbedHttpsRequest.yield(createResponse(200));
const response = await promise;
assert.strictEqual(response.status, 200);
});

it("should handle string bodies correctly", async function() {
const client = createDefaultHttpClient();
stubbedHttpsRequest.returns(httpRequestChecker);

const request = createPipelineRequest({ url: "https://example.com", body: "test data" });
const promise = client.sendRequest(request);
stubbedHttpsRequest.yield(createResponse(200));
const response = await promise;
assert.strictEqual(response.status, 200);
});
});

0 comments on commit 137c671

Please sign in to comment.