From e22ecf6af5ce32cb5afdf0ec64121ec033062c4a Mon Sep 17 00:00:00 2001 From: Jingdan Date: Mon, 16 Apr 2018 16:36:53 +0800 Subject: [PATCH] fix(Browser): multipartUpload callback resumble parse error (#442) * opti: use internal headers, copy options headers * opti: use internal headers, copy options headers * opti: use internal headers, copy options headers * refactor(callback): use options params with copy * feat: add test case * feat: add test case prefix time timestamp * feat: rm publish to cdn about latest --- lib/browser/managed_upload.js | 2 +- lib/browser/object.js | 15 ++++---- lib/common/callback.js | 10 +++--- lib/common/multipart.js | 54 +++++++++++++++++----------- lib/managed_upload.js | 2 +- lib/object.js | 16 ++++----- publish.js | 2 -- test/browser/browser.test.js | 66 +++++++++++++++++++++++++++++++++++ test/node/callback.test.js | 19 ++++++++++ test/node/utils.js | 4 +-- 10 files changed, 142 insertions(+), 48 deletions(-) diff --git a/lib/browser/managed_upload.js b/lib/browser/managed_upload.js index ac7b68bb0..6fc2860d5 100644 --- a/lib/browser/managed_upload.js +++ b/lib/browser/managed_upload.js @@ -68,7 +68,7 @@ proto.multipartUpload = function* multipartUpload(name, file, options) { etag: result.res.headers.etag, }; - if (options.headers && options.headers['x-oss-callback']) { + if ((options.headers && options.headers['x-oss-callback']) || options.callback) { ret.data = result.data; } diff --git a/lib/browser/object.js b/lib/browser/object.js index 23fd543d0..0032928f3 100644 --- a/lib/browser/object.js +++ b/lib/browser/object.js @@ -12,6 +12,7 @@ const path = require('path'); const mime = require('mime'); const callback = require('../common/callback'); const signHelper = require('../common/signUtils'); + // var assert = require('assert'); @@ -86,12 +87,11 @@ proto.put = function* put(name, file, options) { const method = options.method || 'PUT'; const params = this._objectRequestParams(method, name, options); + callback.encodeCallback(params, options); params.mime = options.mime; params.content = content; params.successStatuses = [200]; - callback.encodeCallback(options); - const result = yield this.request(params); const ret = { @@ -100,7 +100,7 @@ proto.put = function* put(name, file, options) { res: result.res, }; - if (options.headers && options.headers['x-oss-callback']) { + if (params.headers && params.headers['x-oss-callback']) { ret.data = JSON.parse(result.data.toString()); } @@ -127,13 +127,11 @@ proto.putStream = function* putStream(name, stream, options) { const method = options.method || 'PUT'; const params = this._objectRequestParams(method, name, options); - + callback.encodeCallback(params, options); params.mime = options.mime; params.stream = stream; params.successStatuses = [200]; - callback.encodeCallback(options); - const result = yield this.request(params); const ret = { @@ -142,7 +140,7 @@ proto.putStream = function* putStream(name, stream, options) { res: result.res, }; - if (options.headers && options.headers['x-oss-callback']) { + if (params.headers && params.headers['x-oss-callback']) { ret.data = JSON.parse(result.data.toString()); } @@ -504,7 +502,8 @@ proto._objectRequestParams = function (method, name, options) { }; if (options.headers) { - params.headers = options.headers; + params.headers = {}; + copy(options.headers).to(params.headers); } return params; }; diff --git a/lib/common/callback.js b/lib/common/callback.js index de3952c79..34b857869 100644 --- a/lib/common/callback.js +++ b/lib/common/callback.js @@ -1,8 +1,8 @@ -exports.encodeCallback = function (options) { - options.headers = options.headers || {}; - if (!Object.prototype.hasOwnProperty.call(options.headers, 'x-oss-callback')) { +exports.encodeCallback = function encodeCallback(reqParams, options) { + reqParams.headers = reqParams.headers || {}; + if (!Object.prototype.hasOwnProperty.call(reqParams.headers, 'x-oss-callback')) { if (options.callback) { const json = { callbackUrl: encodeURI(options.callback.url), @@ -15,14 +15,14 @@ exports.encodeCallback = function (options) { json.callbackBodyType = options.callback.contentType; } const callback = new Buffer(JSON.stringify(json)).toString('base64'); - options.headers['x-oss-callback'] = callback; + reqParams.headers['x-oss-callback'] = callback; if (options.callback.customValue) { const callbackVar = {}; Object.keys(options.callback.customValue).forEach((key) => { callbackVar[`x:${key}`] = options.callback.customValue[key]; }); - options.headers['x-oss-callback-var'] = new Buffer(JSON.stringify(callbackVar)).toString('base64'); + reqParams.headers['x-oss-callback-var'] = new Buffer(JSON.stringify(callbackVar)).toString('base64'); } } } diff --git a/lib/common/multipart.js b/lib/common/multipart.js index ffd1d1f41..a76396bc2 100644 --- a/lib/common/multipart.js +++ b/lib/common/multipart.js @@ -1,5 +1,5 @@ - +const copy = require('copy-to'); const callback = require('./callback'); const proto = exports; @@ -13,8 +13,10 @@ const proto = exports; */ proto.listUploads = function* listUploads(query, options) { options = options || {}; - options.subres = 'uploads'; - const params = this._objectRequestParams('GET', '', options); + const opt = {}; + copy(options).to(opt); + opt.subres = 'uploads'; + const params = this._objectRequestParams('GET', '', opt); params.query = query; params.xmlResponse = true; params.successStatuses = [200]; @@ -53,10 +55,12 @@ proto.listUploads = function* listUploads(query, options) { */ proto.listParts = function* listParts(name, uploadId, query, options) { options = options || {}; - options.subres = { + const opt = {}; + copy(options).to(opt); + opt.subres = { uploadId, }; - const params = this._objectRequestParams('GET', name, options); + const params = this._objectRequestParams('GET', name, opt); params.query = query; params.xmlResponse = true; params.successStatuses = [200]; @@ -85,8 +89,10 @@ proto.listParts = function* listParts(name, uploadId, query, options) { proto.abortMultipartUpload = function* abortMultipartUpload(name, uploadId, options) { this.cancel(); options = options || {}; - options.subres = { uploadId }; - const params = this._objectRequestParams('DELETE', name, options); + const opt = {}; + copy(options).to(opt); + opt.subres = { uploadId }; + const params = this._objectRequestParams('DELETE', name, opt); params.successStatuses = [204]; const result = yield this.request(params); @@ -103,11 +109,13 @@ proto.abortMultipartUpload = function* abortMultipartUpload(name, uploadId, opti */ proto.initMultipartUpload = function* initMultipartUpload(name, options) { options = options || {}; - options.headers = options.headers || {}; - this._convertMetaToHeaders(options.meta, options.headers); + const opt = {}; + copy(options).to(opt); + opt.headers = opt.headers || {}; + this._convertMetaToHeaders(options.meta, opt.headers); - options.subres = 'uploads'; - const params = this._objectRequestParams('POST', name, options); + opt.subres = 'uploads'; + const params = this._objectRequestParams('POST', name, opt); params.mime = options.mime; params.xmlResponse = true; params.successStatuses = [200]; @@ -173,14 +181,16 @@ proto.completeMultipartUpload = function* completeMultipartUpload(name, uploadId xml += ''; options = options || {}; - options.subres = { uploadId }; - const params = this._objectRequestParams('POST', name, options); + const opt = {}; + copy(options).to(opt); + opt.subres = { uploadId }; + + const params = this._objectRequestParams('POST', name, opt); + callback.encodeCallback(params, opt); params.mime = 'xml'; params.content = xml; - callback.encodeCallback(options); - - if (!(options.headers && options.headers['x-oss-callback'])) { + if (!(params.headers && params.headers['x-oss-callback'])) { params.xmlResponse = true; } params.successStatuses = [200]; @@ -193,7 +203,7 @@ proto.completeMultipartUpload = function* completeMultipartUpload(name, uploadId etag: result.res.headers.etag, }; - if (options.headers && options.headers['x-oss-callback']) { + if (params.headers && params.headers['x-oss-callback']) { ret.data = JSON.parse(result.data.toString()); } @@ -210,16 +220,18 @@ proto.completeMultipartUpload = function* completeMultipartUpload(name, uploadId */ proto._uploadPart = function* _uploadPart(name, uploadId, partNo, data, options) { options = options || {}; - options.headers = { + const opt = {}; + copy(options).to(opt); + opt.headers = { 'Content-Length': data.size, }; - options.subres = { + opt.subres = { partNumber: partNo, uploadId, }; - const params = this._objectRequestParams('PUT', name, options); - params.mime = options.mime; + const params = this._objectRequestParams('PUT', name, opt); + params.mime = opt.mime; params.stream = data.stream; params.successStatuses = [200]; diff --git a/lib/managed_upload.js b/lib/managed_upload.js index 031d9e46b..026a03a19 100644 --- a/lib/managed_upload.js +++ b/lib/managed_upload.js @@ -58,7 +58,7 @@ proto.multipartUpload = function* multipartUpload(name, file, options) { etag: result.res.headers.etag, }; - if (options.headers && options.headers['x-oss-callback']) { + if ((options.headers && options.headers['x-oss-callback']) || options.callback) { ret.data = result.data; } diff --git a/lib/object.js b/lib/object.js index 694b2fae6..70c87bb62 100644 --- a/lib/object.js +++ b/lib/object.js @@ -77,10 +77,11 @@ proto.put = function* put(name, file, options) { options.headers = options.headers || {}; this._convertMetaToHeaders(options.meta, options.headers); - callback.encodeCallback(options); - const method = options.method || 'PUT'; const params = this._objectRequestParams(method, name, options); + + callback.encodeCallback(params, options); + params.mime = options.mime; params.content = content; params.successStatuses = [200]; @@ -93,7 +94,7 @@ proto.put = function* put(name, file, options) { res: result.res, }; - if (options.headers && options.headers['x-oss-callback']) { + if (params.headers && params.headers['x-oss-callback']) { ret.data = JSON.parse(result.data.toString()); } @@ -118,11 +119,9 @@ proto.putStream = function* putStream(name, stream, options) { } this._convertMetaToHeaders(options.meta, options.headers); - callback.encodeCallback(options); - const method = options.method || 'PUT'; const params = this._objectRequestParams(method, name, options); - + callback.encodeCallback(params, options); params.mime = options.mime; params.stream = stream; params.successStatuses = [200]; @@ -135,7 +134,7 @@ proto.putStream = function* putStream(name, stream, options) { res: result.res, }; - if (options.headers && options.headers['x-oss-callback']) { + if (params.headers && params.headers['x-oss-callback']) { ret.data = JSON.parse(result.data.toString()); } @@ -503,7 +502,8 @@ proto._objectRequestParams = function (method, name, options) { }; if (options.headers) { - params.headers = options.headers; + params.headers = {}; + copy(options.headers).to(params.headers); } return params; }; diff --git a/publish.js b/publish.js index 3bf889713..5511caaca 100644 --- a/publish.js +++ b/publish.js @@ -10,12 +10,10 @@ var store = oss({ bucket: env.ALI_SDK_OSS_CDN_BUCKET, }); -var latest = 'aliyun-oss-sdk.min.js'; var current = 'aliyun-oss-sdk-' + pkg.version + '.min.js'; var dist = './dist/aliyun-oss-sdk.min.js'; co(function* () { - yield store.put(latest, dist); yield store.put(current, dist); }).catch(function (err) { console.log(err); diff --git a/test/browser/browser.test.js b/test/browser/browser.test.js index 9d6bd2e93..f1d5dc495 100644 --- a/test/browser/browser.test.js +++ b/test/browser/browser.test.js @@ -1049,6 +1049,72 @@ describe('browser', () => { assert.equal(result.res.status, 200); assert.equal(result.data.Status, 'OK'); }); + + it('should upload file with cancel and callback', function* () { + const client = this.store; + // create a file with 1M random data + const fileContent = Array(1024 * 1024).fill('a').join(''); + const file = new File([fileContent], 'multipart-upload-file'); + + const name = `${prefix}multipart/upload-file-cancel-callback`; + + let tempCheckpoint = null; + const options = { + progress(p, checkpoint) { + return function (done) { + tempCheckpoint = checkpoint; + if (p > 0.5) { + client.cancel(); + } + done(); + }; + }, + partSize: 100 * 1024, + callback: { + url: 'http://oss-demo.aliyuncs.com:23450', + host: 'oss-cn-hangzhou.aliyuncs.com', + /* eslint no-template-curly-in-string: [0] */ + body: 'bucket=${bucket}&object=${object}&var1=${x:var1}', + contentType: 'application/x-www-form-urlencoded', + customValue: { + var1: 'value1', + var2: 'value2', + }, + }, + }; + try { + yield client.multipartUpload(name, file, options); + } catch (err) { + assert.equal(true, client.isCancel()); + } + + assert.equal(true, tempCheckpoint && Object.keys(tempCheckpoint).length !== 0); + + const options2 = { + progress(p) { + return function (done) { + assert.equal(true, p > 0.5); + done(); + }; + }, + partSize: 100 * 1024, + checkpoint: tempCheckpoint, + callback: { + url: 'http://oss-demo.aliyuncs.com:23450', + host: 'oss-cn-hangzhou.aliyuncs.com', + /* eslint no-template-curly-in-string: [0] */ + body: 'bucket=${bucket}&object=${object}&var1=${x:var1}', + contentType: 'application/x-www-form-urlencoded', + customValue: { + var1: 'value1', + var2: 'value2', + }, + }, + }; + const result = yield client.multipartUpload(name, file, options2); + + assert.equal(result.res.status, 200); + }); }); }); diff --git a/test/node/callback.test.js b/test/node/callback.test.js index 9f632e7cc..05c51d5ae 100644 --- a/test/node/callback.test.js +++ b/test/node/callback.test.js @@ -142,5 +142,24 @@ describe('test/callback.test.js', () => { assert.equal(result.res.status, 200); assert.equal(result.data.Status, 'OK'); }); + + it('should multipart upload with no more 100k file use header x-oss-callback', function* () { + // create a file with 1M random data + const fileName = yield utils.createTempFile('upload-with-callback', 50 * 1024); + const callback = { + url: callbackServer, + body: 'bucket=${bucket}&object=${object}&var1=${x:var1}', + }; + const name = `${prefix}multipart/upload-with-callback`; + /* eslint no-mixed-spaces-and-tabs: [0] */ + const result = yield this.store.multipartUpload(name, fileName, { + partSize: 100 * 1024, + headers: { + 'x-oss-callback': utils.encodeCallback(callback), + }, + }); + assert.equal(result.res.status, 200); + assert.equal(result.data.Status, 'OK'); + }); }); }); diff --git a/test/node/utils.js b/test/node/utils.js index 47a8857ae..3c0000a12 100644 --- a/test/node/utils.js +++ b/test/node/utils.js @@ -65,9 +65,9 @@ exports.cleanBucket = function* (store, bucket, region) { }; if (process && process.browser) { - exports.prefix = `${platform.name}-${platform.version}/`; + exports.prefix = `${platform.name}-${platform.version}-${new Date().getTime()}/`; } else { - exports.prefix = `${process.platform}-${process.version}/`; + exports.prefix = `${process.platform}-${process.version}-${new Date().getTime()}/`;// unique prefix add time timestamp if (process && process.execPath.indexOf('iojs') >= 0) { exports.prefix = `iojs-${exports.prefix}`; }