Skip to content

Commit

Permalink
Fix for binary data getting corrupted in node request provider (dojo#315
Browse files Browse the repository at this point in the history
)

* Buffer data no longer gets converted to text in node request provider

* Ignoring content-encoding: none

* Code review feedback
  • Loading branch information
rorticus authored Apr 21, 2017
1 parent ee34d42 commit 417df90
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
17 changes: 6 additions & 11 deletions src/request/providers/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ interface HttpsOptions extends Options {
interface RequestData {
task: Task<http.IncomingMessage>;
buffer: any[];
data: string;
data: Buffer;
size: number;
used: boolean;
nativeResponse: http.IncomingMessage;
Expand Down Expand Up @@ -172,12 +172,7 @@ export class NodeResponse extends Response {
arrayBuffer(): Task<ArrayBuffer> {
return <any> getDataTask(this).then(data => {
if (data) {
if (<any> data.data instanceof Buffer) {
return data.data;
}
else {
return Buffer.from(data.data, 'utf8');
}
return data.data;
}

return new Buffer([]);
Expand Down Expand Up @@ -471,9 +466,9 @@ export default function node(url: string, options: NodeRequestOptions = {}): Tas
content, so do undo the encoding we have to start at the end and work backwards.
*/
if (contentEncodings.length) {
const encoding = contentEncodings.pop()!.trim();
const encoding = contentEncodings.pop()!.trim().toLowerCase();

if (encoding === '' || encoding === 'identity') {
if (encoding === '' || encoding === 'none' || encoding === 'identity') {
// do nothing, response stream is as-is
handleEncoding();
}
Expand Down Expand Up @@ -502,7 +497,7 @@ export default function node(url: string, options: NodeRequestOptions = {}): Tas
}
}
else {
data.data = String(dataAsBuffer);
data.data = dataAsBuffer;

response.emit({
type: 'end',
Expand All @@ -523,7 +518,7 @@ export default function node(url: string, options: NodeRequestOptions = {}): Tas
const data: RequestData = {
task,
buffer: [],
data: '',
data: Buffer.alloc(0),
size: 0,
used: false,
url: url,
Expand Down
22 changes: 19 additions & 3 deletions tests/unit/request/node.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as fs from 'fs';
import { createServer } from 'http';
import * as registerSuite from 'intern!object';
import * as assert from 'intern/chai!assert';
Expand All @@ -13,6 +14,8 @@ let server: any;
let proxy: any;
let requestData: string;

const blobFileSize = fs.statSync('tests/support/data/blob.gif').size;

interface DummyResponse {
body?: string | ((callback: Function) => void);
headers?: { [key: string]: string };
Expand Down Expand Up @@ -190,6 +193,12 @@ const responseData: { [url: string]: DummyResponse } = {
headers: {
'Content-Encoding': 'gzip, deflate'
}
},
'blob.gif': {
statusCode: 200,
body: function (callback: any) {
callback(fs.readFileSync('tests/support/data/blob.gif'));
}
}
};

Expand Down Expand Up @@ -633,11 +642,18 @@ registerSuite({
},

'response types': {
'arrayBuffer'() {
'arrayBuffer with binary content'() {
return nodeRequest(getRequestUrl('blob.gif')).then((response: any) => {
return response.arrayBuffer().then((arrayBuffer: any) => {
assert.strictEqual(arrayBuffer.byteLength, blobFileSize);
});
});
},

'arrayBuffer with text content'() {
return nodeRequest(getRequestUrl('foo.json')).then((response: any) => {
return response.arrayBuffer().then((arrayBuffer: any) => {
assert.isAbove(arrayBuffer.byteLength, 0);
assert.isAbove(arrayBuffer.length, 0);
assert.strictEqual(arrayBuffer.byteLength, JSON.stringify({ foo: 'bar' }).length);
});
});
},
Expand Down

0 comments on commit 417df90

Please sign in to comment.