fix: encoding of special chars when encoding to string (#37)

* fix: encoding of special chars when encoding to string * docs: changelog for #37 Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
package-url · Mar 7, 2023 · d7cbfe6 · d7cbfe6
1 parent 9998e46
commit d7cbfe6
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# UNRELEASED
+### ### Bug Fixes
+* Hardened encoding/decoding of URL special chars like `@` and `#` [#37](https://github.com/package-url/packageurl-js/pull/37)
+
 # 1.0.0
 ### Features
 * Add enum-like static readonly property `KnownQualifierNames` to reflect known qualifier names [#34](https://github.com/package-url/packageurl-js/pull/34)

diff --git a/src/package-url.js b/src/package-url.js
@@ -76,7 +76,7 @@ class PackageURL {
   }
 
   toString() {
-    var purl = ['pkg:', this.type, '/'];
+    var purl = ['pkg:', encodeURIComponent(this.type), '/'];
 
     if (this.type === 'pypi') {
       this._handlePyPi();
@@ -91,11 +91,11 @@ class PackageURL {
       purl.push('/');
     }
 
-    purl.push(encodeURIComponent(this.name).replace('%3A', ':'));
+    purl.push(encodeURIComponent(this.name).replace(/%3A/g, ':'));
 
     if (this.version) {
       purl.push('@');
-      purl.push(encodeURIComponent(this.version).replace('%3A', ':'));
+      purl.push(encodeURIComponent(this.version).replace(/%3A/g, ':'));
     }
 
     if (this.qualifiers) {
@@ -104,22 +104,28 @@ class PackageURL {
       let qualifiers = this.qualifiers;
       let qualifierString = [];
       Object.keys(qualifiers).sort().forEach(key => {
-        qualifierString.push(encodeURIComponent(key).replace('%3A', ':') + '=' + encodeURI(qualifiers[key]));
+        qualifierString.push(
+          encodeURIComponent(key).replace(/%3A/g, ':')
+          + '='
+          + encodeURIComponent(qualifiers[key]).replace(/%2F/g, '/')
+        );
       });
 
       purl.push(qualifierString.join('&'));
     }
 
     if (this.subpath) {
       purl.push('#');
-      purl.push(encodeURI(this.subpath));
+      purl.push(encodeURIComponent(this.subpath)
+        .replace(/%3A/g, ':')
+        .replace(/%2F/g, '/'));
     }
 
     return purl.join('');
   }
 
   static fromString(purl) {
-    if (!purl || !typeof purl === 'string' || !purl.trim()) {
+    if (!purl || typeof purl !== 'string' || !purl.trim()) {
       throw new Error('A purl string argument is required.');
     }
 
@@ -136,6 +142,7 @@ class PackageURL {
     if (!type || !remainder) {
       throw new Error('purl is missing the required "type" component.');
     }
+    type = decodeURIComponent(type)
 
     let url = new URL(purl);
 
@@ -150,9 +157,9 @@ class PackageURL {
     if (subpath.indexOf('#') === 0) {
       subpath = subpath.substring(1);
     }
-    if (subpath.length === 0) {
-      subpath = null;
-    }
+    subpath = subpath.length === 0
+      ? null
+      : decodeURIComponent(subpath)
 
     if (url.username !== '' || url.password !== '') {
       throw new Error('Invalid purl: cannot contain a "user:pass@host:port"');

diff --git a/test/package-url.spec.js b/test/package-url.spec.js
@@ -21,12 +21,33 @@ SOFTWARE.
 */
 
 const assert = require('assert');
+const {describe, it} = require("mocha");
+
 const TEST_FILE = require('./data/test-suite-data.json');
 
 /** @type {import('../src/package-url')} */
 const PackageURL = require('../src/package-url');
 
 describe('PackageURL', function () {
+  describe('toString()', function () {
+    it('all components encode #', function () {
+      /* The # is a delimiter between url and subpath. */
+      var purl = new PackageURL('ty#pe', 'name#space', 'na#me', 'ver#sion', {'foo':'bar#baz'}, 'sub#path');
+      assert.strictEqual(purl.toString(), 'pkg:ty%23pe/name%23space/na%23me@ver%23sion?foo=bar%23baz#sub%23path')
+    })
+    it('all components encode @', function () {
+      /* The @ is a delimiter between package name and version. */
+      var purl = new PackageURL('ty@pe', 'name@space', 'na@me', 'ver@sion', {'foo':'bar@baz'}, 'sub@path');
+      assert.strictEqual(purl.toString(), 'pkg:ty%40pe/name%40space/na%40me@ver%40sion?foo=bar%40baz#sub%40path')
+    })
+
+    it('path components encode /', function () {
+      /* only namespace is allowed to have multiple segments separated by `/`` */
+      var purl = new PackageURL('ty/pe', 'namespace1/namespace2', 'na/me');
+      assert.strictEqual(purl.toString(), 'pkg:ty%2Fpe/namespace1/namespace2/na%2Fme')
+    })
+  })
+
   describe('fromString()', function () {
     it('with qualifiers.checksums', function () {
       const purlString = 'pkg:npm/packageurl-js@0.0.7?checksums=sha512:b9c27369720d948829a98118e9a35fd09d9018711e30dc2df5f8ae85bb19b2ade4679351c4d96768451ee9e841e5f5a36114a9ef98f4fe5256a5f4ca981736a0'
@@ -55,6 +76,33 @@ describe('PackageURL', function () {
         vcs_url: 'git+https://github.com/package-url/packageurl-js.git'
       })
     });
+
+    it('namespace with multiple segments', function () {
+      var purl = PackageURL.fromString('pkg:ty%2Fpe/namespace1/namespace2/na%2Fme')
+      assert.strictEqual('ty/pe', purl.type)
+      assert.strictEqual('namespace1/namespace2', purl.namespace)
+      assert.strictEqual('na/me', purl.name)
+    })
+
+    it('encoded #', function () {
+      var purl = PackageURL.fromString('pkg:ty%23pe/name%23space/na%23me@ver%23sion?foo=bar%23baz#sub%23path')
+      assert.strictEqual('ty#pe', purl.type)
+      assert.strictEqual('name#space', purl.namespace)
+      assert.strictEqual('na#me', purl.name)
+      assert.strictEqual('ver#sion', purl.version)
+      assert.deepStrictEqual({foo:'bar#baz'}, purl.qualifiers)
+      assert.strictEqual('sub#path', purl.subpath)
+    })
+
+    it('encoded @', function () {
+      var purl = PackageURL.fromString('pkg:ty%40pe/name%40space/na%40me@ver%40sion?foo=bar%40baz#sub%40path')
+      assert.strictEqual('ty@pe', purl.type)
+      assert.strictEqual('name@space', purl.namespace)
+      assert.strictEqual('na@me', purl.name)
+      assert.strictEqual('ver@sion', purl.version)
+      assert.deepStrictEqual({foo:'bar@baz'}, purl.qualifiers)
+      assert.strictEqual('sub@path', purl.subpath)
+    })
   });
 
   describe('test-suite-data', function () {
@@ -65,39 +113,39 @@ describe('PackageURL', function () {
             var purl = new PackageURL(obj.type, obj.namespace, obj.name, obj.version, obj.qualifiers, obj.subpath);
             assert.fail();
           } catch (e) {
-            assert.equal(true, e.toString().includes('is a required field') || e.toString().includes('Invalid purl'));
+            assert.ok(e.toString().includes('is a required field') || e.toString().includes('Invalid purl'));
           }
         });
         it('should not be possible to parse invalid PackageURLs', function () {
           try {
             PackageURL.fromString(obj.purl);
           } catch (e) {
-            assert.equal(true, e.toString().includes('Error: purl is missing the required') || e.toString().includes('Invalid purl'));
+            assert.ok(e.toString().includes('Error: purl is missing the required') || e.toString().includes('Invalid purl'));
           }
         });
       } else {
         it('should be able to create valid PackageURLs', function () {
           var purl = new PackageURL(obj.type, obj.namespace, obj.name, obj.version, obj.qualifiers, obj.subpath);
-          assert.equal(obj.type, purl.type);
-          assert.equal(obj.name, purl.name);
-          assert.equal(obj.namespace, purl.namespace);
-          assert.equal(obj.version, purl.version);
-          assert.equal(JSON.stringify(obj.qualifiers), JSON.stringify(purl.qualifiers));
-          assert.equal(obj.subpath, purl.subpath);
+          assert.strictEqual(purl.type, obj.type);
+          assert.strictEqual(purl.name, obj.name);
+          assert.strictEqual(purl.namespace, obj.namespace);
+          assert.strictEqual(purl.version, obj.version);
+          assert.deepStrictEqual(purl.qualifiers, obj.qualifiers);
+          assert.strictEqual(purl.subpath, obj.subpath, );
         });
         it('should be able to convert valid PackageURLs to a string', function () {
           var purl = new PackageURL(obj.type, obj.namespace, obj.name, obj.version, obj.qualifiers, obj.subpath);
-          assert.equal(obj.canonical_purl, purl.toString());
+          assert.strictEqual(purl.toString(), obj.canonical_purl);
         });
         it('should be able to parse valid PackageURLs', function () {
           var purl = PackageURL.fromString(obj.canonical_purl);
-          assert.equal(purl.toString(), obj.canonical_purl);
-          assert.equal(purl.type, obj.type);
-          assert.equal(purl.name, obj.name);
-          assert.equal(purl.namespace, obj.namespace);
-          assert.equal(purl.version, obj.version);
-          assert.equal(JSON.stringify(purl.qualifiers), JSON.stringify(obj.qualifiers));
-          assert.equal(purl.subpath, obj.subpath);
+          assert.strictEqual(purl.toString(), obj.canonical_purl);
+          assert.strictEqual(purl.type, obj.type);
+          assert.strictEqual(purl.name, obj.name);
+          assert.strictEqual(purl.namespace, obj.namespace);
+          assert.strictEqual(purl.version, obj.version);
+          assert.deepStrictEqual(purl.qualifiers, obj.qualifiers);
+          assert.strictEqual(purl.subpath, obj.subpath);
         });
         it('should handle pypi package-urls per the purl-spec', function () {
           const purlMixedCasing = PackageURL.fromString('pkg:pypi/PYYaml@5.3.0');