From 5be52d78d1ae404287a280c05fd89a0bbb15383d Mon Sep 17 00:00:00 2001 From: antikalk Date: Tue, 12 May 2020 14:44:22 +0200 Subject: [PATCH 1/9] + handle empty success responses --- www/helpers.js | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/www/helpers.js b/www/helpers.js index ecdb100..7e17616 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -293,22 +293,25 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, } try { - // json - if (responseType === validResponseTypes[1]) { - response.data = JSON.parse(response.data); - } - - // arraybuffer - else if (responseType === validResponseTypes[2]) { - response.data = base64.toArrayBuffer(response.data); - } - - // blob - else if (responseType === validResponseTypes[3]) { - var buffer = base64.toArrayBuffer(response.data); - var type = response.headers['content-type'] || ''; - var blob = new Blob([ buffer ], { type: type }); - response.data = blob; + // to allow empty responses: only post process data if it is set + if (response.data) { + // json + if (responseType === validResponseTypes[1]) { + response.data = JSON.parse(response.data); + } + + // arraybuffer + else if (responseType === validResponseTypes[2]) { + response.data = base64.toArrayBuffer(response.data); + } + + // blob + else if (responseType === validResponseTypes[3]) { + var buffer = base64.toArrayBuffer(response.data); + var type = response.headers['content-type'] || ''; + var blob = new Blob([ buffer ], { type: type }); + response.data = blob; + } } success(response); From a1e4be37d4b1686f7208dab1219d94373d29c144 Mon Sep 17 00:00:00 2001 From: antikalk Date: Wed, 13 May 2020 08:07:13 +0200 Subject: [PATCH 2/9] + early return pattern --- www/helpers.js | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/www/helpers.js b/www/helpers.js index 7e17616..c930e67 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -292,26 +292,27 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, return success(response); } + if (!response.data) { + return success(response); + } + try { - // to allow empty responses: only post process data if it is set - if (response.data) { - // json - if (responseType === validResponseTypes[1]) { - response.data = JSON.parse(response.data); - } - - // arraybuffer - else if (responseType === validResponseTypes[2]) { - response.data = base64.toArrayBuffer(response.data); - } - - // blob - else if (responseType === validResponseTypes[3]) { - var buffer = base64.toArrayBuffer(response.data); - var type = response.headers['content-type'] || ''; - var blob = new Blob([ buffer ], { type: type }); - response.data = blob; - } + // json + if (responseType === validResponseTypes[1]) { + response.data = JSON.parse(response.data); + } + + // arraybuffer + else if (responseType === validResponseTypes[2]) { + response.data = base64.toArrayBuffer(response.data); + } + + // blob + else if (responseType === validResponseTypes[3]) { + var buffer = base64.toArrayBuffer(response.data); + var type = response.headers['content-type'] || ''; + var blob = new Blob([ buffer ], { type: type }); + response.data = blob; } success(response); From 658bbd8b28f1c020017664d2913a3d73d068cf02 Mon Sep 17 00:00:00 2001 From: antikalk Date: Wed, 13 May 2020 09:12:59 +0200 Subject: [PATCH 3/9] + set data to null if response data is not set --- www/helpers.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/www/helpers.js b/www/helpers.js index c930e67..66e20aa 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -293,6 +293,8 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, } if (!response.data) { + // return null as data if response data is not set + response.data = null; return success(response); } From 81874c6bc8ad7c6881caf87d694faa637f23eba4 Mon Sep 17 00:00:00 2001 From: antikalk Date: Wed, 13 May 2020 09:13:51 +0200 Subject: [PATCH 4/9] + test if empty response data is handled correctly --- test/js-specs.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/js-specs.js b/test/js-specs.js index 2507dd6..253841e 100644 --- a/test/js-specs.js +++ b/test/js-specs.js @@ -444,6 +444,17 @@ describe('Common helpers', function () { handler({ data: JSON.stringify(fakeData) }); }); + it('handles empty "json" response correctly', () => { + const emptyData = ""; + const helpers = require('../www/helpers')(null, jsUtil, null, messages, null, errorCodes); + const handler = helpers.injectRawResponseHandler( + 'json', + response => should.equal(null, response.data) + ); + + handler({ data: emptyData }); + }); + it('handles response type "arraybuffer" correctly', () => { const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); const handler = helpers.injectRawResponseHandler( @@ -454,6 +465,16 @@ describe('Common helpers', function () { handler({ data: 'myString' }); }); + it('handles empty "arraybuffer" response correctly', () => { + const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); + const handler = helpers.injectRawResponseHandler( + 'arraybuffer', + response => should.equal(null, response.data) + ); + + handler({ data: '' }); + }); + it('handles response type "blob" correctly', () => { const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); const handler = helpers.injectRawResponseHandler( @@ -468,6 +489,18 @@ describe('Common helpers', function () { handler({ data: 'myString', headers: { 'content-type': 'fakeType'} }); }); + it('handles empty "blob" response correctly', () => { + const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); + const handler = helpers.injectRawResponseHandler( + 'blob', + (response) => { + should.equal(null, response.data) + } + ); + + handler({ data: '', headers: { 'content-type': 'fakeType'} }); + }); + it('calls failure callback when post-processing fails', () => { const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); const handler = helpers.injectRawResponseHandler( From e77505738984cf8e70935b9d5d1c7d2ac56ab959 Mon Sep 17 00:00:00 2001 From: antikalk Date: Thu, 14 May 2020 22:22:03 +0200 Subject: [PATCH 5/9] + added e2e test --- test/e2e-specs.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/e2e-specs.js b/test/e2e-specs.js index 4e1bcc9..ec1349d 100644 --- a/test/e2e-specs.js +++ b/test/e2e-specs.js @@ -890,6 +890,17 @@ const tests = [ result.data.headers['access-control-allow-origin'].should.be.equal('*'); } }, + { + description: 'should allow empty response body even though responseType is set #334', + expected: 'resolved: {"status":200, ...', + func: function (resolve, reject) { cordova.plugin.http.get('https://httpbin.org/status/200', {}, { + 'responseType': 'json' + }, resolve, reject); }, + validationFunc: function (driver, result) { + result.type.should.be.equal('resolved'); + should.equal(null, result.data.data); + } + }, // TODO: not ready yet // { From bfc6ba2008db02c3683933dd8c420cab3f963f3c Mon Sep 17 00:00:00 2001 From: antikalk Date: Sat, 16 May 2020 16:54:58 +0200 Subject: [PATCH 6/9] + dont manipulate response data --- test/e2e-specs.js | 2 +- test/js-specs.js | 6 +++--- www/helpers.js | 2 -- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/e2e-specs.js b/test/e2e-specs.js index ec1349d..a54b453 100644 --- a/test/e2e-specs.js +++ b/test/e2e-specs.js @@ -898,7 +898,7 @@ const tests = [ }, resolve, reject); }, validationFunc: function (driver, result) { result.type.should.be.equal('resolved'); - should.equal(null, result.data.data); + should.equal('', result.data.data); } }, diff --git a/test/js-specs.js b/test/js-specs.js index 253841e..b8710a9 100644 --- a/test/js-specs.js +++ b/test/js-specs.js @@ -449,7 +449,7 @@ describe('Common helpers', function () { const helpers = require('../www/helpers')(null, jsUtil, null, messages, null, errorCodes); const handler = helpers.injectRawResponseHandler( 'json', - response => should.equal(null, response.data) + response => should.equal('', response.data) ); handler({ data: emptyData }); @@ -469,7 +469,7 @@ describe('Common helpers', function () { const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); const handler = helpers.injectRawResponseHandler( 'arraybuffer', - response => should.equal(null, response.data) + response => should.equal('', response.data) ); handler({ data: '' }); @@ -494,7 +494,7 @@ describe('Common helpers', function () { const handler = helpers.injectRawResponseHandler( 'blob', (response) => { - should.equal(null, response.data) + should.equal('', response.data) } ); diff --git a/www/helpers.js b/www/helpers.js index 66e20aa..c930e67 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -293,8 +293,6 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, } if (!response.data) { - // return null as data if response data is not set - response.data = null; return success(response); } From fed2b03cc657dc3ed40dca782ed08beee647c494 Mon Sep 17 00:00:00 2001 From: antikalk Date: Thu, 21 May 2020 22:41:49 +0200 Subject: [PATCH 7/9] Revert "+ dont manipulate response data" This reverts commit bfc6ba2008db02c3683933dd8c420cab3f963f3c. --- test/e2e-specs.js | 2 +- test/js-specs.js | 6 +++--- www/helpers.js | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/e2e-specs.js b/test/e2e-specs.js index 8ff248c..b3691c3 100644 --- a/test/e2e-specs.js +++ b/test/e2e-specs.js @@ -898,7 +898,7 @@ const tests = [ }, resolve, reject); }, validationFunc: function (driver, result) { result.type.should.be.equal('resolved'); - should.equal('', result.data.data); + should.equal(null, result.data.data); } }, { diff --git a/test/js-specs.js b/test/js-specs.js index b8710a9..253841e 100644 --- a/test/js-specs.js +++ b/test/js-specs.js @@ -449,7 +449,7 @@ describe('Common helpers', function () { const helpers = require('../www/helpers')(null, jsUtil, null, messages, null, errorCodes); const handler = helpers.injectRawResponseHandler( 'json', - response => should.equal('', response.data) + response => should.equal(null, response.data) ); handler({ data: emptyData }); @@ -469,7 +469,7 @@ describe('Common helpers', function () { const helpers = require('../www/helpers')(null, jsUtil, null, messages, fakeBase64, errorCodes); const handler = helpers.injectRawResponseHandler( 'arraybuffer', - response => should.equal('', response.data) + response => should.equal(null, response.data) ); handler({ data: '' }); @@ -494,7 +494,7 @@ describe('Common helpers', function () { const handler = helpers.injectRawResponseHandler( 'blob', (response) => { - should.equal('', response.data) + should.equal(null, response.data) } ); diff --git a/www/helpers.js b/www/helpers.js index c930e67..66e20aa 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -293,6 +293,8 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, } if (!response.data) { + // return null as data if response data is not set + response.data = null; return success(response); } From 65e4a4d4dccb3a7e27e8111e5a2fb065aaf59a1a Mon Sep 17 00:00:00 2001 From: Sefa Ilkimen Date: Fri, 29 May 2020 03:35:20 +0200 Subject: [PATCH 8/9] fix: e2e spec "should allow empty response body even though responseType is set #334" - `cordova.plugin.http.get` does not support passing an options object => using `cordova.plugin.http.sendRequest` instead - but still fails on test "should return empty body string correctly (GET)" --- test/e2e-specs.js | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/test/e2e-specs.js b/test/e2e-specs.js index b3691c3..3e509fe 100644 --- a/test/e2e-specs.js +++ b/test/e2e-specs.js @@ -2,7 +2,7 @@ const hooks = { onBeforeEachTest: function (resolve, reject) { cordova.plugin.http.clearCookies(); - helpers.enableFollowingRedirect(function() { + helpers.enableFollowingRedirect(function () { // server trust mode is not supported on browser platform if (cordova.platformId === 'browser') { return resolve(); @@ -23,7 +23,7 @@ const helpers = { setPinnedServerTrustMode: function (resolve, reject) { cordova.plugin.http.setServerTrustMode('pinned', resolve, reject); }, setNoneClientAuthMode: function (resolve, reject) { cordova.plugin.http.setClientAuthMode('none', resolve, reject); }, setBufferClientAuthMode: function (resolve, reject) { - helpers.getWithXhr(function(pkcs) { + helpers.getWithXhr(function (pkcs) { cordova.plugin.http.setClientAuthMode('buffer', { rawPkcs: pkcs, pkcsPassword: 'badssl.com' @@ -34,9 +34,9 @@ const helpers = { setUtf8StringSerializer: function (resolve) { resolve(cordova.plugin.http.setDataSerializer('utf8')); }, setUrlEncodedSerializer: function (resolve) { resolve(cordova.plugin.http.setDataSerializer('urlencoded')); }, setMultipartSerializer: function (resolve) { resolve(cordova.plugin.http.setDataSerializer('multipart')); }, - setRawSerializer: function(resolve) { resolve(cordova.plugin.http.setDataSerializer('raw')); }, + setRawSerializer: function (resolve) { resolve(cordova.plugin.http.setDataSerializer('raw')); }, disableFollowingRedirect: function (resolve) { resolve(cordova.plugin.http.setFollowRedirect(false)); }, - enableFollowingRedirect: function(resolve) { resolve(cordova.plugin.http.setFollowRedirect(true)); }, + enableFollowingRedirect: function (resolve) { resolve(cordova.plugin.http.setFollowRedirect(true)); }, getWithXhr: function (done, url, type) { var xhr = new XMLHttpRequest(); @@ -71,7 +71,7 @@ const helpers = { var byteArray = new Uint8Array(buffer); for (var i = 0; i < byteArray.length; i++) { - hash = ((hash << 5) - hash) + byteArray[i]; + hash = ((hash << 5) - hash) + byteArray[i]; hash |= 0; // Convert to 32bit integer } @@ -300,7 +300,7 @@ const tests = [ { description: 'should not follow 302 redirect when following redirects is disabled', expected: 'rejected: {"status": 302, ...', - before: function(resolve, reject) { cordova.plugin.http.disableRedirect(true, resolve, reject)}, + before: function (resolve, reject) { cordova.plugin.http.disableRedirect(true, resolve, reject) }, func: function (resolve, reject) { cordova.plugin.http.get('http://httpbin.org/redirect-to?url=http://httpbin.org/anything', {}, {}, resolve, reject); }, validationFunc: function (driver, result) { result.type.should.be.equal('rejected'); @@ -374,7 +374,7 @@ const tests = [ var targetUrl = 'http://httpbin.org/post'; helpers.writeToFile(function () { - helpers.writeToFile(function() { + helpers.writeToFile(function () { cordova.plugin.http.uploadFile(targetUrl, {}, {}, [sourcePath, sourcePath2], [fileName, fileName2], resolve, reject); }, fileName2, fileContent2); }, fileName, fileContent); @@ -824,7 +824,7 @@ const tests = [ before: helpers.setMultipartSerializer, func: function (resolve, reject) { var ponyfills = cordova.plugin.http.ponyfills; - helpers.getWithXhr(function(blob) { + helpers.getWithXhr(function (blob) { var formData = new ponyfills.FormData(); formData.append('CordovaLogo', blob); @@ -850,7 +850,7 @@ const tests = [ expected: 'resolved: {"status":200,"data:application/octet-stream;base64,iVBORw0KGgoAAAANSUhEUg ...', before: helpers.setRawSerializer, func: function (resolve, reject) { - helpers.getWithXhr(function(buffer) { + helpers.getWithXhr(function (buffer) { cordova.plugin.http.post('http://httpbin.org/anything', buffer, {}, resolve, reject); }, './res/cordova_logo.png', 'arraybuffer'); }, @@ -893,9 +893,11 @@ const tests = [ { description: 'should allow empty response body even though responseType is set #334', expected: 'resolved: {"status":200, ...', - func: function (resolve, reject) { cordova.plugin.http.get('https://httpbin.org/status/200', {}, { - 'responseType': 'json' - }, resolve, reject); }, + func: function (resolve, reject) { + var url = 'https://httpbin.org/status/200'; + var options = { method: 'get', responseType: 'json' }; + cordova.plugin.http.sendRequest(url, options, resolve, reject); + }, validationFunc: function (driver, result) { result.type.should.be.equal('resolved'); should.equal(null, result.data.data); @@ -914,22 +916,22 @@ const tests = [ result.data.status.should.be.equal(200); result.data.data.should.be.an('object'); result.data.data.slideshow.should.be.eql({ - author: 'Yours Truly', - date: 'date of publication', + author: 'Yours Truly', + date: 'date of publication', slides: [ { - title: 'Wake up to WonderWidgets!', + title: 'Wake up to WonderWidgets!', type: 'all' - }, + }, { items: [ 'Why WonderWidgets are great', 'Who buys WonderWidgets' - ], - title: 'Overview', + ], + title: 'Overview', type: 'all' } - ], + ], title: 'Sample Slide Show' }); } From 4f4e7ffa337d80d5fa976bef47c74468218c11ff Mon Sep 17 00:00:00 2001 From: Sefa Ilkimen Date: Fri, 29 May 2020 04:03:20 +0200 Subject: [PATCH 9/9] fix: broken handling for empty strings --- test/js-specs.js | 18 +++++++++--------- www/helpers.js | 30 ++++++++++++++++-------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/test/js-specs.js b/test/js-specs.js index 253841e..67343a8 100644 --- a/test/js-specs.js +++ b/test/js-specs.js @@ -419,7 +419,7 @@ describe('Common helpers', function () { response => response.data.should.be.equal('fakeData') ); - handler({ data: 'fakeData' }); + handler({ data: 'fakeData' }); }); it('does not change response data if response type is "text"', () => { @@ -449,7 +449,7 @@ describe('Common helpers', function () { const helpers = require('../www/helpers')(null, jsUtil, null, messages, null, errorCodes); const handler = helpers.injectRawResponseHandler( 'json', - response => should.equal(null, response.data) + response => should.equal(undefined, response.data) ); handler({ data: emptyData }); @@ -486,7 +486,7 @@ describe('Common helpers', function () { } ); - handler({ data: 'myString', headers: { 'content-type': 'fakeType'} }); + handler({ data: 'myString', headers: { 'content-type': 'fakeType' } }); }); it('handles empty "blob" response correctly', () => { @@ -498,7 +498,7 @@ describe('Common helpers', function () { } ); - handler({ data: '', headers: { 'content-type': 'fakeType'} }); + handler({ data: '', headers: { 'content-type': 'fakeType' } }); }); it('calls failure callback when post-processing fails', () => { @@ -579,7 +579,7 @@ describe('Common helpers', function () { it('processes data correctly when serializer "utf8" is configured', (cb) => { helpers.processData('myString', 'utf8', (data) => { - data.should.be.eql({text: 'myString'}); + data.should.be.eql({ text: 'myString' }); cb(); }) }); @@ -624,7 +624,7 @@ describe('Common helpers', function () { }); it('processes data correctly when serializer "raw" is configured', (cb) => { - const byteArray = new Uint8Array([1,2,3]); + const byteArray = new Uint8Array([1, 2, 3]); helpers.processData(byteArray, 'raw', (data) => { data.should.be.a('ArrayBuffer'); data.should.be.equal(byteArray.buffer); @@ -680,7 +680,7 @@ describe('Dependency Validator', function () { describe('checkFormDataInstance()', function () { it('throws an error if FormData.entries() is not supported on given instance', function () { const console = new ConsoleMock(); - const validator = require('../www/dependency-validator')({ FormData: {}}, console, messages); + const validator = require('../www/dependency-validator')({ FormData: {} }, console, messages); (() => validator.checkFormDataInstance({})).should.throw(messages.MISSING_FORMDATA_ENTRIES_API); }); @@ -717,12 +717,12 @@ describe('Ponyfills', function () { const iterator = new ponyfills.Iterator([]); iterator.next().should.be.eql({ done: true, value: undefined }); }); - + it('returns iteration object correctly when end posititon of list is not reached yet', () => { const iterator = new ponyfills.Iterator([['first', 'this is the first item']]); iterator.next().should.be.eql({ done: false, value: ['first', 'this is the first item'] }); }); - + it('returns iteration object correctly when end posititon of list is already reached', () => { const iterator = new ponyfills.Iterator([['first', 'this is the first item']]); iterator.next(); diff --git a/www/helpers.js b/www/helpers.js index 66e20aa..a578211 100644 --- a/www/helpers.js +++ b/www/helpers.js @@ -292,29 +292,31 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, return success(response); } - if (!response.data) { - // return null as data if response data is not set - response.data = null; - return success(response); - } - try { // json if (responseType === validResponseTypes[1]) { - response.data = JSON.parse(response.data); + response.data = response.data === '' + ? undefined + : JSON.parse(response.data); } // arraybuffer else if (responseType === validResponseTypes[2]) { - response.data = base64.toArrayBuffer(response.data); + response.data = response.data === '' + ? null + : base64.toArrayBuffer(response.data); } // blob else if (responseType === validResponseTypes[3]) { - var buffer = base64.toArrayBuffer(response.data); - var type = response.headers['content-type'] || ''; - var blob = new Blob([ buffer ], { type: type }); - response.data = blob; + if (response.data === '') { + response.data = null; + } else { + var buffer = base64.toArrayBuffer(response.data); + var type = response.headers['content-type'] || ''; + var blob = new Blob([buffer], { type: type }); + response.data = blob; + } } success(response); @@ -390,7 +392,7 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, if (allowedInstanceTypes) { var isCorrectInstanceType = false; - allowedInstanceTypes.forEach(function(type) { + allowedInstanceTypes.forEach(function (type) { if ((global[type] && data instanceof global[type]) || (ponyfills[type] && data instanceof ponyfills[type])) { isCorrectInstanceType = true; } @@ -446,7 +448,7 @@ module.exports = function init(global, jsUtil, cookieHandler, messages, base64, if (entry.value[1] instanceof global.Blob || entry.value[1] instanceof global.File) { var reader = new global.FileReader(); - reader.onload = function() { + reader.onload = function () { result.buffers.push(base64.fromArrayBuffer(reader.result)); result.names.push(entry.value[0]); result.fileNames.push(entry.value[1].name || 'blob');