Skip to content

Commit 0056679

Browse files
galfertgalfert
authored
Merge pull request #1155 from iLiviu/bugfix/1116-read_binary_file_dropbox_googledrive
fix GoogleDrive and Dropbox getFile returning corrupted binary data
2 parents 07f68e3 + 2cb900b commit 0056679

File tree

9 files changed

+189
-156
lines changed

9 files changed

+189
-156
lines changed

‎src/dropbox.js‎

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ const PATH_PREFIX = '/remotestorage';
4545
const isFolder = util.isFolder;
4646
const cleanPath = util.cleanPath;
4747
const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
48-
const readBinaryData = util.readBinaryData;
4948
const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
49+
const getTextFromArrayBuffer = util.getTextFromArrayBuffer;
5050

5151
/**
5252
* Map a local path to a path in Dropbox.
@@ -389,8 +389,9 @@ Dropbox.prototype = {
389389

390390
var params = {
391391
headers: {
392-
'Dropbox-API-Arg': JSON.stringify({path: getDropboxPath(path)})
393-
}
392+
'Dropbox-API-Arg': JSON.stringify({path: getDropboxPath(path)}),
393+
},
394+
responseType: 'arraybuffer'
394395
};
395396
if (options && options.ifNoneMatch) {
396397
params.headers['If-None-Match'] = options.ifNoneMatch;
@@ -403,51 +404,52 @@ Dropbox.prototype = {
403404
return Promise.resolve({statusCode: status});
404405
}
405406
meta = resp.getResponseHeader('Dropbox-API-Result');
406-
body = resp.responseText;
407-
408-
if (status === 409) {
409-
meta = body;
410-
}
411-
412-
try {
413-
meta = JSON.parse(meta);
414-
} catch(e) {
415-
return Promise.reject(e);
416-
}
407+
//first encode the response as text, and later check if
408+
//text appears to actually be binary data
409+
return getTextFromArrayBuffer(resp.response, 'UTF-8').then(function (responseText) {
410+
body = responseText;
411+
if (status === 409) {
412+
meta = body;
413+
}
417414

418-
if (status === 409) {
419-
if (compareApiError(meta, ['path', 'not_found'])) {
420-
return Promise.resolve({statusCode: 404});
415+
try {
416+
meta = JSON.parse(meta);
417+
} catch(e) {
418+
return Promise.reject(e);
421419
}
422-
return Promise.reject(new Error('API error while downloading file ("' + path + '"): ' + meta.error_summary));
423-
}
424420

425-
mime = resp.getResponseHeader('Content-Type');
426-
rev = meta.rev;
427-
self._revCache.set(path, rev);
428-
self._shareIfNeeded(path);
421+
if (status === 409) {
422+
if (compareApiError(meta, ['path', 'not_found'])) {
423+
return {statusCode: 404};
424+
}
425+
return Promise.reject(new Error('API error while downloading file ("' + path + '"): ' + meta.error_summary));
426+
}
429427

430-
// handling binary
431-
if (shouldBeTreatedAsBinary(resp.response, mime)) {
432-
return readBinaryData(resp.response, mime).then((result) => {
433-
return {
434-
statusCode: status,
435-
body: result,
436-
contentType: mime,
437-
revision: rev
438-
};
439-
});
440-
}
428+
mime = resp.getResponseHeader('Content-Type');
429+
rev = meta.rev;
430+
self._revCache.set(path, rev);
431+
self._shareIfNeeded(path);
441432

442-
// handling json (always try)
443-
try {
444-
body = JSON.parse(body);
445-
mime = 'application/json; charset=UTF-8';
446-
} catch(e) {
447-
//Failed parsing Json, assume it is something else then
448-
}
433+
if (shouldBeTreatedAsBinary(responseText, mime)) {
434+
//return unprocessed response
435+
body = resp.response;
436+
} else {
437+
// handling json (always try)
438+
try {
439+
body = JSON.parse(body);
440+
mime = 'application/json; charset=UTF-8';
441+
} catch(e) {
442+
//Failed parsing Json, assume it is something else then
443+
}
444+
}
449445

450-
return Promise.resolve({statusCode: status, body: body, contentType: mime, revision: rev});
446+
return {
447+
statusCode: status,
448+
body: body,
449+
contentType: mime,
450+
revision: rev
451+
};
452+
});
451453
});
452454
},
453455

‎src/googledrive.js‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ const RS_DIR_MIME_TYPE = 'application/json; charset=UTF-8';
3131
const isFolder = util.isFolder;
3232
const cleanPath = util.cleanPath;
3333
const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
34-
const readBinaryData = util.readBinaryData;
3534
const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
35+
const getTextFromArrayBuffer = util.getTextFromArrayBuffer;
3636

3737
let hasLocalStorage;
3838

@@ -457,28 +457,29 @@ GoogleDrive.prototype = {
457457
}
458458
}
459459

460-
return this._request('GET', meta.downloadUrl, {}).then((response) => {
461-
let body = response.response;
462-
if (meta.mimeType.match(/^application\/json/)) {
463-
try {
464-
body = JSON.parse(body);
465-
} catch(e) {
466-
// body couldn't be parsed as JSON, so we'll just return it as is
460+
var params = {
461+
responseType: 'arraybuffer'
462+
};
463+
return this._request('GET', meta.downloadUrl, params).then((response) => {
464+
//first encode the response as text, and later check if
465+
//text appears to actually be binary data
466+
return getTextFromArrayBuffer(response.response, 'UTF-8').then(function (responseText) {
467+
let body = responseText;
468+
if (meta.mimeType.match(/^application\/json/)) {
469+
try {
470+
body = JSON.parse(body);
471+
} catch(e) {
472+
// body couldn't be parsed as JSON, so we'll just return it as is
473+
}
474+
} else {
475+
if (shouldBeTreatedAsBinary(responseText, meta.mimeType)) {
476+
//return unprocessed response
477+
body = response.response;
478+
}
467479
}
468-
} else {
469-
if (shouldBeTreatedAsBinary(body, meta.mimeType)) {
470-
return readBinaryData(body, meta.mimeType).then((result) => {
471-
return {
472-
statusCode: 200,
473-
body: result,
474-
contentType: meta.mimeType,
475-
revision: etagWithoutQuotes
476-
};
477-
});
478-
}
479-
}
480480

481-
return Promise.resolve({statusCode: 200, body: body, contentType: meta.mimeType, revision: etagWithoutQuotes});
481+
return {statusCode: 200, body: body, contentType: meta.mimeType, revision: etagWithoutQuotes};
482+
});
482483
});
483484
});
484485
});

‎src/util.js‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,8 @@ var util = {
223223
},
224224

225225
/**
226-
* Decide if data should be treated as binary based on the content
227-
* and content-type.
226+
* Decide if data should be treated as binary based on the content (presence of non-printable characters
227+
* or replacement character) and content-type.
228228
*
229229
* @param {string} content - The data
230230
* @param {string} mimeType - The data's content-type
@@ -233,41 +233,46 @@ var util = {
233233
*/
234234
shouldBeTreatedAsBinary (content, mimeType) {
235235
// eslint-disable-next-line no-control-regex
236-
return (mimeType && mimeType.match(/charset=binary/)) || /[\x00-\x1F]/.test(content);
236+
return (mimeType && mimeType.match(/charset=binary/)) || /[\x00-\x08\x0E-\x1F\uFFFD]/.test(content);
237237
},
238238

239239
/**
240-
* Read binary data and return it as ArrayBuffer.
241-
*
242-
* @param {string} content - The data
243-
* @param {string} mimeType - The data's content-type
244-
* @returns {Promise} Resolves with an ArrayBuffer containing the data
240+
* Read data from an ArrayBuffer and return it as a string
241+
* @param {ArrayBuffer} arrayBuffer
242+
* @param {string} encoding
243+
* @returns {Promise} Resolves with a string containing the data
245244
*/
246-
readBinaryData (content, mimeType) {
247-
return new Promise((resolve) => {
248-
let blob;
249-
util.globalContext.BlobBuilder = util.globalContext.BlobBuilder || util.globalContext.WebKitBlobBuilder;
250-
if (typeof util.globalContext.BlobBuilder !== 'undefined') {
251-
const bb = new global.BlobBuilder();
252-
bb.append(content);
253-
blob = bb.getBlob(mimeType);
254-
} else {
255-
blob = new Blob([content], { type: mimeType });
256-
}
257-
258-
const reader = new FileReader();
259-
if (typeof reader.addEventListener === 'function') {
260-
reader.addEventListener('loadend', function () {
261-
resolve(reader.result); // reader.result contains the contents of blob as a typed array
262-
});
245+
getTextFromArrayBuffer(arrayBuffer, encoding) {
246+
return new Promise((resolve/*, reject*/) => {
247+
if (typeof Blob === 'undefined') {
248+
var buffer = new Buffer(new Uint8Array(arrayBuffer));
249+
resolve(buffer.toString(encoding));
263250
} else {
264-
reader.onloadend = function() {
265-
resolve(reader.result); // reader.result contains the contents of blob as a typed array
266-
};
251+
var blob;
252+
util.globalContext.BlobBuilder = util.globalContext.BlobBuilder || util.globalContext.WebKitBlobBuilder;
253+
if (typeof util.globalContext.BlobBuilder !== 'undefined') {
254+
var bb = new global.BlobBuilder();
255+
bb.append(arrayBuffer);
256+
blob = bb.getBlob();
257+
} else {
258+
blob = new Blob([arrayBuffer]);
259+
}
260+
261+
var fileReader = new FileReader();
262+
if (typeof fileReader.addEventListener === 'function') {
263+
fileReader.addEventListener('loadend', function (evt) {
264+
resolve(evt.target.result);
265+
});
266+
} else {
267+
fileReader.onloadend = function(evt) {
268+
resolve(evt.target.result);
269+
};
270+
}
271+
fileReader.readAsText(blob, encoding);
267272
}
268-
reader.readAsArrayBuffer(blob);
269273
});
270274
}
275+
271276

272277
};
273278

‎src/wireclient.js‎

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ const isFolder = util.isFolder;
7676
const cleanPath = util.cleanPath;
7777
const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
7878
const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
79+
const getTextFromArrayBuffer = util.getTextFromArrayBuffer;
7980

8081
function addQuotes(str) {
8182
if (typeof(str) !== 'string') {
@@ -96,37 +97,6 @@ function stripQuotes(str) {
9697
return str.replace(/^["']|["']$/g, '');
9798
}
9899

99-
function getTextFromArrayBuffer(arrayBuffer, encoding) {
100-
return new Promise((resolve/*, reject*/) => {
101-
if (typeof Blob === 'undefined') {
102-
var buffer = new Buffer(new Uint8Array(arrayBuffer));
103-
resolve(buffer.toString(encoding));
104-
} else {
105-
var blob;
106-
util.globalContext.BlobBuilder = util.globalContext.BlobBuilder || util.globalContext.WebKitBlobBuilder;
107-
if (typeof util.globalContext.BlobBuilder !== 'undefined') {
108-
var bb = new global.BlobBuilder();
109-
bb.append(arrayBuffer);
110-
blob = bb.getBlob();
111-
} else {
112-
blob = new Blob([arrayBuffer]);
113-
}
114-
115-
var fileReader = new FileReader();
116-
if (typeof fileReader.addEventListener === 'function') {
117-
fileReader.addEventListener('loadend', function (evt) {
118-
resolve(evt.target.result);
119-
});
120-
} else {
121-
fileReader.onloadend = function(evt) {
122-
resolve(evt.target.result);
123-
};
124-
}
125-
fileReader.readAsText(blob, encoding);
126-
}
127-
});
128-
}
129-
130100
function determineCharset(mimeType) {
131101
var charset = 'UTF-8';
132102
var charsetMatch;

‎test/helpers/mocks.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ define([], function() {
9797
this._responses = {};
9898
};
9999

100+
global.ArrayBufferMock = function(str) {
101+
return {
102+
iAmA: 'ArrayBufferMock',
103+
content: str
104+
};
105+
};
106+
100107
},
101108

102109
undefineMocks(env) {

0 commit comments

Comments
 (0)