feat: add strict object name validation (#1265)

* feat: add strict object name validation * docs: update README and CHANGELOG * fix: strict object name validation use optional parameters instead of global variables --------- Co-authored-by: zhengzuoyu.zzy <zhengzuoyu.zzy@alibaba-inc.com>
ali-sdk · Dec 15, 2023 · 830e36e · 830e36e
1 parent ea2c2b9
commit 830e36e
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
+## [6.19.0](https://github.com/ali-sdk/ali-oss/compare/v6.18.1...v6.19.0) (2023-12-15)
+
+### Features
+
+* verify object names strictly when signing URLs and enabled by default ([#1265](https://github.com/ali-sdk/ali-oss/issues/1265)) ([ff03bbb](https://github.com/ali-sdk/ali-oss/pull/1265/commits/ff03bbb62b041dd34d30ca2503745ba5d7c0a216))
+
 ### [6.18.1](https://github.com/ali-sdk/ali-oss/compare/v6.18.0...v6.18.1) (2023-09-08)
 
 ### Features

diff --git a/README.md b/README.md
@@ -343,7 +343,7 @@ options:
 - [region] {String} the bucket data region location, please see [Data Regions](#data-regions),
   default is `oss-cn-hangzhou`.
 - [internal] {Boolean} access OSS with aliyun internal network or not, default is `false`.
-  If your servers are running on aliyun too, you can set `true` to save lot of money.
+  If your servers are running on aliyun too, you can set `true` to save a lot of money.
 - [secure] {Boolean} instruct OSS client to use HTTPS (secure: true) or HTTP (secure: false) protocol.
 - [timeout] {String|Number} instance level timeout for all operations, default is `60s`.
 - [cname] {Boolean}, default false, access oss with custom domain name. if true, you can fill `endpoint` field with your custom domain name,
@@ -2579,7 +2579,7 @@ console.log(result.objects);
 console.log(result.deleteMarker);
 ```
 
-### .signatureUrl(name[, options])
+### .signatureUrl(name[, options, strictObjectNameValidation])
 
 Create a signature url for download or upload object. When you put object with signatureUrl ,you need to pass `Content-Type`.Please look at the example.
 
@@ -2605,6 +2605,7 @@ parameters:
     - body {String} set the body for callback
     - [contentType] {String} set the type for body
     - [customValue] {Object} set the custom value for callback,eg. {var1: value1,var2:value2}
+- [strictObjectNameValidation] {boolean} the flag of verifying object name strictly, default is true
 
 Success will return signature url.
 
@@ -2639,7 +2640,7 @@ const url = store.signatureUrl('ossdemo.txt', {
     'content-type': 'text/custom',
     'content-disposition': 'attachment'
   }
-});
+}, false);
 console.log(url);
 
 // put operation
@@ -2660,7 +2661,7 @@ const url = store.signatureUrl('ossdemo.png', {
 console.log(url);
 ```
 
-### .asyncSignatureUrl(name[, options])
+### .asyncSignatureUrl(name[, options, strictObjectNameValidation])
 
 Basically the same as signatureUrl, if refreshSTSToken is configured asyncSignatureUrl will refresh stsToken
 
@@ -2686,6 +2687,7 @@ parameters:
     - body {String} set the body for callback
     - [contentType] {String} set the type for body
     - [customValue] {Object} set the custom value for callback,eg. {var1: value1,var2:value2}
+- [strictObjectNameValidation] {boolean} the flag of verifying object name strictly, default is true
 
 Success will return signature url.
 
@@ -2717,7 +2719,7 @@ const url = await store.asyncSignatureUrl('ossdemo.txt', {
     'content-type': 'text/custom',
     'content-disposition': 'attachment'
   }
-});
+}, false);
 console.log(url);
 // put operation
 ```

diff --git a/lib/common/object/asyncSignatureUrl.js b/lib/common/object/asyncSignatureUrl.js
@@ -8,10 +8,21 @@ const { isFunction } = require('../utils/isFunction');
 
 const proto = exports;
 
-proto.asyncSignatureUrl = async function asyncSignatureUrl(name, options) {
+/**
+ * asyncSignatureUrl
+ * @param {String} name object name
+ * @param {Object} options options
+ * @param {boolean} [strictObjectNameValidation=true] the flag of verifying object name strictly
+ */
+proto.asyncSignatureUrl = async function asyncSignatureUrl(name, options, strictObjectNameValidation = true) {
   if (isIP(this.options.endpoint.hostname)) {
     throw new Error('can not get the object URL when endpoint is IP');
   }
+
+  if (strictObjectNameValidation && /^\?/.test(name)) {
+    throw new Error(`Invalid object name ${name}`);
+  }
+
   options = options || {};
   name = this._objectName(name);
   options.method = options.method || 'GET';

diff --git a/lib/common/object/signatureUrl.js b/lib/common/object/signatureUrl.js
@@ -10,12 +10,18 @@ const proto = exports;
  *  signatureUrl
  * @deprecated will be deprecated in 7.x
  * @param {String} name object name
- * @param {Object} options  options
+ * @param {Object} options options
+ * @param {boolean} [strictObjectNameValidation=true] the flag of verifying object name strictly
  */
-proto.signatureUrl = function signatureUrl(name, options) {
+proto.signatureUrl = function signatureUrl(name, options, strictObjectNameValidation = true) {
   if (isIP(this.options.endpoint.hostname)) {
     throw new Error('can not get the object URL when endpoint is IP');
   }
+
+  if (strictObjectNameValidation && /^\?/.test(name)) {
+    throw new Error(`Invalid object name ${name}`);
+  }
+
   options = options || {};
   name = this._objectName(name);
   options.method = options.method || 'GET';

diff --git a/test/browser/browser.test.js b/test/browser/browser.test.js
@@ -981,6 +981,7 @@ describe('browser', () => {
     let store;
     let name;
     let needEscapeName;
+    const testSignatureObjectName = `?{测}\r\n[试];,/?:@&=+$<中>-_.!~*'(文)"￥#%！（字）^ \`符|\\${prefix}test.txt`;
     before(async () => {
       store = oss(ossConfig);
       name = `${prefix}ali-sdk/oss/signatureUrl.js`;
@@ -1006,6 +1007,9 @@ describe('browser', () => {
       });
       assert.equal(object.res.status, 200);
       // assert.equal(typeof object.res.headers['x-oss-request-id'], 'string');
+
+      const testSignatureObject = await store.put(testSignatureObjectName, Buffer.from('Hello World!', 'utf8'));
+      assert.equal(typeof testSignatureObject.res.headers['x-oss-request-id'], 'string');
     });
 
     it('should signature url get object ok', async () => {
@@ -1015,6 +1019,40 @@ describe('browser', () => {
       assert.equal(urlRes.data.toString(), result.content.toString());
     });
 
+    it('should verify object name strictly by default', () => {
+      assert.throws(() => {
+        try {
+          store.signatureUrl(testSignatureObjectName);
+        } catch (err) {
+          assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
+          throw err;
+        }
+      }, Error);
+
+      store
+        .asyncSignatureUrl(testSignatureObjectName)
+        .then(() => {
+          assert.fail('Expected asyncSignatureUrl to throw an error');
+        })
+        .catch(err => {
+          assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
+        });
+    });
+
+    it('should verify object name loosely', async () => {
+      const testSignatureObjectFromGet = await store.get(testSignatureObjectName);
+      const testSignatureObjectUrl = store.signatureUrl(testSignatureObjectName, undefined, false);
+      const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
+      assert.strictEqual(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());
+
+      const testSignatureObjectUrlAsync = await store.asyncSignatureUrl(testSignatureObjectName, undefined, false);
+      const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
+      assert.strictEqual(
+        testSignatureObjectFromUrlAsync.data.toString(),
+        testSignatureObjectFromGet.content.toString()
+      );
+    });
+
     // it('should signature url with image processed and get object ok', function* () {
     //   var name = prefix + 'ali-sdk/oss/nodejs-test-signature-1024x768.png';
     //   var originImagePath = path.join(__dirname, 'nodejs-1024x768.png');
@@ -1057,7 +1095,7 @@ describe('browser', () => {
       assert.equal(urlRes.data.toString(), result.content.toString());
     });
 
-    it('should signature url with reponse limitation', async () => {
+    it('should signature url with response limitation', async () => {
       const response = {
         'content-type': 'xml',
         'content-language': 'zh-cn'

diff --git a/test/node/object.test.js b/test/node/object.test.js
@@ -1025,6 +1025,7 @@ describe('test/object.test.js', () => {
   describe('signatureUrl()', () => {
     let name;
     let needEscapeName;
+    const testSignatureObjectName = `?{测}\r\n[试];,/?:@&=+$<中>-_.!~*'(文)"￥#%！（字）^ \`符|\\${prefix}test.txt`;
     before(async () => {
       name = `${prefix}ali-sdk/oss/signatureUrl.js`;
       let object = await store.put(name, __filename, {
@@ -1045,6 +1046,9 @@ describe('test/object.test.js', () => {
         }
       });
       assert.equal(typeof object.res.headers['x-oss-request-id'], 'string');
+
+      const testSignatureObject = await store.put(testSignatureObjectName, Buffer.from('Hello World!', 'utf8'));
+      assert.equal(testSignatureObject.res.status, 200);
     });
 
     it('should signature url get object ok', async () => {
@@ -1058,6 +1062,37 @@ describe('test/object.test.js', () => {
       }
     });
 
+    it('should verify object name strictly by default', () => {
+      assert.throws(() => {
+        try {
+          store.signatureUrl(testSignatureObjectName);
+        } catch (err) {
+          assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
+          throw err;
+        }
+      }, Error);
+
+      assert.rejects(store.asyncSignatureUrl(testSignatureObjectName), err => {
+        assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
+
+        return true;
+      });
+    });
+
+    it('should verify object name loosely', async () => {
+      const testSignatureObjectFromGet = await store.get(testSignatureObjectName);
+      const testSignatureObjectUrl = store.signatureUrl(testSignatureObjectName, undefined, false);
+      const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
+      assert.strictEqual(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());
+
+      const testSignatureObjectUrlAsync = await store.asyncSignatureUrl(testSignatureObjectName, undefined, false);
+      const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
+      assert.strictEqual(
+        testSignatureObjectFromUrlAsync.data.toString(),
+        testSignatureObjectFromGet.content.toString()
+      );
+    });
+
     it('should signature url with response limitation', () => {
       try {
         const response = {