Skip to content

Commit 4530a74

Browse files
iLiviuiLiviu
committed
fix get binary files for Dropbox and GoogleDrive #1116
Dropbox and Googledrive backends first read the response as text and then tried to convert it back to raw value, however not all bytes can be encoded as text, and are lost during the conversion to text and back. Now, they request the response as ArrayBuffer and encode it to text and if they detect binary data, they return the unprocessed ArrayBuffer. moved getTextFromArrayBuffer from wireclient to util to be used by Dropbox and GoogleDrive fixed tests for the new change.
1 parent a6d5ee2 commit 4530a74

File tree

8 files changed

+203
-131
lines changed

8 files changed

+203
-131
lines changed

‎src/dropbox.js‎

Lines changed: 51 additions & 48 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,53 @@ 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-
}
417-
418-
if (status === 409) {
419-
if (compareApiError(meta, ['path', 'not_found'])) {
420-
return Promise.resolve({statusCode: 404});
421-
}
422-
return Promise.reject(new Error('API error while downloading file ("' + path + '"): ' + meta.error_summary));
423-
}
424-
425-
mime = resp.getResponseHeader('Content-Type');
426-
rev = meta.rev;
427-
self._revCache.set(path, rev);
428-
self._shareIfNeeded(path);
429-
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-
}
441-
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-
}
449-
450-
return Promise.resolve({statusCode: status, body: body, contentType: mime, revision: rev});
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')
410+
.then(function (responseText) {
411+
body = responseText;
412+
if (status === 409) {
413+
meta = body;
414+
}
415+
416+
try {
417+
meta = JSON.parse(meta);
418+
} catch(e) {
419+
return Promise.reject(e);
420+
}
421+
422+
if (status === 409) {
423+
if (compareApiError(meta, ['path', 'not_found'])) {
424+
return {statusCode: 404};
425+
}
426+
return Promise.reject(new Error('API error while downloading file ("' + path + '"): ' + meta.error_summary));
427+
}
428+
429+
mime = resp.getResponseHeader('Content-Type');
430+
rev = meta.rev;
431+
self._revCache.set(path, rev);
432+
self._shareIfNeeded(path);
433+
434+
if (shouldBeTreatedAsBinary(responseText, mime)) {
435+
//return unprocessed response
436+
body = resp.response;
437+
} else {
438+
// handling json (always try)
439+
try {
440+
body = JSON.parse(body);
441+
mime = 'application/json; charset=UTF-8';
442+
} catch(e) {
443+
//Failed parsing Json, assume it is something else then
444+
}
445+
}
446+
447+
return {
448+
statusCode: status,
449+
body: body,
450+
contentType: mime,
451+
revision: rev
452+
};
453+
});
451454
});
452455
},
453456

‎src/googledrive.js‎

Lines changed: 25 additions & 23 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,30 @@ 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
467-
}
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-
}
480-
481-
return Promise.resolve({statusCode: 200, body: body, contentType: meta.mimeType, revision: etagWithoutQuotes});
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')
467+
.then(function (responseText) {
468+
let body = responseText;
469+
if (meta.mimeType.match(/^application\/json/)) {
470+
try {
471+
body = JSON.parse(body);
472+
} catch(e) {
473+
// body couldn't be parsed as JSON, so we'll just return it as is
474+
}
475+
} else {
476+
if (shouldBeTreatedAsBinary(responseText, meta.mimeType)) {
477+
//return unprocessed response
478+
body = response.response;
479+
}
480+
}
481+
482+
return {statusCode: 200, body: body, contentType: meta.mimeType, revision: etagWithoutQuotes};
483+
});
482484
});
483485
});
484486
});

‎src/util.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,45 @@ var util = {
267267
}
268268
reader.readAsArrayBuffer(blob);
269269
});
270+
},
271+
272+
/**
273+
* Read data from an ArrayBuffer and return it as a string
274+
* @param {ArrayBuffer} arrayBuffer
275+
* @param {string} encoding
276+
* @returns {Promise} Resolves with a string containing the data
277+
*/
278+
getTextFromArrayBuffer(arrayBuffer, encoding) {
279+
return new Promise((resolve/*, reject*/) => {
280+
if (typeof Blob === 'undefined') {
281+
var buffer = new Buffer(new Uint8Array(arrayBuffer));
282+
resolve(buffer.toString(encoding));
283+
} else {
284+
var blob;
285+
util.globalContext.BlobBuilder = util.globalContext.BlobBuilder || util.globalContext.WebKitBlobBuilder;
286+
if (typeof util.globalContext.BlobBuilder !== 'undefined') {
287+
var bb = new global.BlobBuilder();
288+
bb.append(arrayBuffer);
289+
blob = bb.getBlob();
290+
} else {
291+
blob = new Blob([arrayBuffer]);
292+
}
293+
294+
var fileReader = new FileReader();
295+
if (typeof fileReader.addEventListener === 'function') {
296+
fileReader.addEventListener('loadend', function (evt) {
297+
resolve(evt.target.result);
298+
});
299+
} else {
300+
fileReader.onloadend = function(evt) {
301+
resolve(evt.target.result);
302+
};
303+
}
304+
fileReader.readAsText(blob, encoding);
305+
}
306+
});
270307
}
308+
271309

272310
};
273311

‎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)