From ab2f960011d48284956023df3a78a071e3ea40b9 Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 24 Aug 2024 09:08:39 -0400 Subject: [PATCH] fix: error on decode with meaningful message --- src/decode.js | 12 +++--- src/error.js | 34 +++++++++++++++ src/package-url.js | 31 ++++++++------ src/purl-type.js | 17 ++++---- src/validate.js | 27 ++++++------ test/data/contrib-tests.json | 72 ------------------------------- test/package-url.spec.js | 82 ++++++++++++++++++++---------------- 7 files changed, 127 insertions(+), 148 deletions(-) create mode 100644 src/error.js diff --git a/src/decode.js b/src/decode.js index ce52c2b..d884cac 100644 --- a/src/decode.js +++ b/src/decode.js @@ -1,14 +1,16 @@ 'use strict' -const { decodeURIComponent: decodeURIComponent_ } = globalThis +const { PurlError } = require('./error') -function decodeURIComponent(encodedURIComponent) { +const { decodeURIComponent } = globalThis + +function decodePurlComponent(comp, encodedURIComponent) { try { - return decodeURIComponent_(encodedURIComponent) + return decodeURIComponent(encodedURIComponent) } catch {} - return encodedURIComponent + throw new PurlError(`unable to decode "${comp}" component`) } module.exports = { - decodeURIComponent + decodePurlComponent } diff --git a/src/error.js b/src/error.js new file mode 100644 index 0000000..94d6de4 --- /dev/null +++ b/src/error.js @@ -0,0 +1,34 @@ +'use strict' + +function formatPurlErrorMessage(message = '') { + const { length } = message + let formatted = '' + if (length) { + // Lower case start of message. + const code0 = message.charCodeAt(0) + formatted = + code0 >= 65 /*'A'*/ || code0 <= 90 /*'Z'*/ + ? `${message[0].toLowerCase()}${message.slice(1)}` + : message + // Remove period from end of message. + if ( + length > 1 && + message.charCodeAt(length - 1) === 46 /*'.'*/ && + message.charCodeAt(length - 2) !== 46 + ) { + formatted = formatted.slice(0, -1) + } + } + return `Invalid purl: ${formatted}` +} + +class PurlError extends Error { + constructor(message) { + super(formatPurlErrorMessage(message)) + } +} + +module.exports = { + formatPurlErrorMessage, + PurlError +} diff --git a/src/package-url.js b/src/package-url.js index 32b1ed8..6c0a010 100644 --- a/src/package-url.js +++ b/src/package-url.js @@ -21,13 +21,14 @@ SOFTWARE. */ 'use strict' -const { decodeURIComponent } = require('./decode') +const { decodePurlComponent } = require('./decode') const { isObject, recursiveFreeze } = require('./objects') const { isBlank, isNonEmptyString, trimLeadingSlashes } = require('./strings') const { PurlComponent } = require('./purl-component') const { PurlQualifierNames } = require('./purl-qualifier-names') const { PurlType } = require('./purl-type') +const { PurlError } = require('./error') class PackageURL { static Component = recursiveFreeze(PurlComponent) @@ -149,15 +150,15 @@ class PackageURL { ? url : new URL(purlStr) } catch (e) { - throw new Error('Invalid purl: failed to parse as URL', { + throw new PurlError('failed to parse as URL', { cause: e }) } } // The scheme is a constant with the value "pkg". if (url?.protocol !== 'pkg:') { - throw new Error( - 'Invalid purl: missing required "pkg" scheme component' + throw new PurlError( + 'missing required "pkg" scheme component' ) } // A purl must NOT contain a URL Authority i.e. there is no support for @@ -166,14 +167,13 @@ class PackageURL { maybeUrlWithAuth.username !== '' || maybeUrlWithAuth.password !== '' ) { - throw new Error( - 'Invalid purl: cannot contain a "user:pass@host:port"' - ) + throw new PurlError('cannot contain a "user:pass@host:port"') } const { pathname } = url const firstSlashIndex = pathname.indexOf('/') - const rawType = decodeURIComponent( + const rawType = decodePurlComponent( + 'type', firstSlashIndex === -1 ? pathname : pathname.slice(0, firstSlashIndex) @@ -206,7 +206,10 @@ class PackageURL { ) if (atSignIndex !== -1) { // Split the remainder once from right on '@'. - rawVersion = decodeURIComponent(pathname.slice(atSignIndex + 1)) + rawVersion = decodePurlComponent( + 'version', + pathname.slice(atSignIndex + 1) + ) } let rawNamespace @@ -214,14 +217,16 @@ class PackageURL { const lastSlashIndex = beforeVersion.lastIndexOf('/') if (lastSlashIndex === -1) { // Split the remainder once from right on '/'. - rawName = decodeURIComponent(beforeVersion) + rawName = decodePurlComponent('name', beforeVersion) } else { // Split the remainder once from right on '/'. - rawName = decodeURIComponent( + rawName = decodePurlComponent( + 'name', beforeVersion.slice(lastSlashIndex + 1) ) // Split the remainder on '/'. - rawNamespace = decodeURIComponent( + rawNamespace = decodePurlComponent( + 'namespace', beforeVersion.slice(0, lastSlashIndex) ) } @@ -237,7 +242,7 @@ class PackageURL { const { hash } = url if (hash.length !== 0) { // Split the purl string once from right on '#'. - rawSubpath = decodeURIComponent(hash.slice(1)) + rawSubpath = decodePurlComponent('subpath', hash.slice(1)) } return [ diff --git a/src/purl-type.js b/src/purl-type.js index 7d3d694..031f585 100644 --- a/src/purl-type.js +++ b/src/purl-type.js @@ -14,6 +14,7 @@ const { } = require('./strings') const { validateEmptyByType, validateRequiredByType } = require('./validate') +const { PurlError } = require('./error') const PurlTypNormalizer = (purl) => purl @@ -149,16 +150,16 @@ module.exports = { if (isNullishOrEmptyString(purl.namespace)) { if (purl.qualifiers?.channel) { if (throws) { - throw new Error( - 'Invalid purl: conan requires a "namespace" field when a "channel" qualifier is present.' + throw new PurlError( + 'conan requires a "namespace" component when a "channel" qualifier is present' ) } return false } } else if (isNullishOrEmptyString(purl.qualifiers)) { if (throws) { - throw new Error( - 'Invalid purl: conan requires a "qualifiers" field when a namespace is present.' + throw new PurlError( + 'conan requires a "qualifiers" component when a namespace is present' ) } return false @@ -190,8 +191,8 @@ module.exports = { !isSemverString(version.slice(1)) ) { if (throws) { - throw new Error( - 'Invalid purl: golang "version" field starting with a "v" must be followed by a valid semver version' + throw new PurlError( + 'golang "version" component starting with a "v" must be followed by a valid semver version' ) } return false @@ -241,8 +242,8 @@ module.exports = { ) ) { if (throws) { - throw new Error( - 'Invalid purl: pub "name" field may only contain [a-z0-9_] characters' + throw new PurlError( + 'pub "name" component may only contain [a-z0-9_] characters' ) } return false diff --git a/src/validate.js b/src/validate.js index 70dff28..8af73ed 100644 --- a/src/validate.js +++ b/src/validate.js @@ -1,13 +1,14 @@ 'use strict' +const { PurlError } = require('./error') const { isNullishOrEmptyString } = require('./lang') const { isNonEmptyString } = require('./strings') function validateEmptyByType(type, name, value, throws) { if (!isNullishOrEmptyString(value)) { if (throws) { - throw new Error( - `Invalid purl: ${type} "${name}" field must be empty.` + throw new PurlError( + `${type} "${name}" component must be empty` ) } return false @@ -32,9 +33,7 @@ function validateQualifiers(qualifiers, throws) { } if (typeof qualifiers !== 'object') { if (throws) { - throw new Error( - 'Invalid purl: "qualifiers" argument must be an object.' - ) + throw new PurlError('"qualifiers" must be an object') } return false } @@ -74,8 +73,8 @@ function validateQualifierKey(key, throws) { ) ) { if (throws) { - throw new Error( - `Invalid purl: qualifier "${key}" contains an illegal character.` + throw new PurlError( + `qualifier "${key}" contains an illegal character` ) } return false @@ -87,7 +86,7 @@ function validateQualifierKey(key, throws) { function validateRequired(name, value, throws) { if (isNullishOrEmptyString(value)) { if (throws) { - throw new Error(`Invalid purl: "${name}" is a required field.`) + throw new PurlError(`"${name}" is a required component`) } return false } @@ -97,7 +96,7 @@ function validateRequired(name, value, throws) { function validateRequiredByType(type, name, value, throws) { if (isNullishOrEmptyString(value)) { if (throws) { - throw new Error(`Invalid purl: ${type} requires a "${name}" field.`) + throw new PurlError(`${type} requires a "${name}" component`) } return false } @@ -109,8 +108,8 @@ function validateStartsWithoutNumber(name, value, throws) { const code = value.charCodeAt(0) if (code >= 48 /*'0'*/ && code <= 57 /*'9'*/) { if (throws) { - throw new Error( - `Invalid purl: ${name} "${value}" cannot start with a number.` + throw new PurlError( + `${name} "${value}" cannot start with a number` ) } return false @@ -124,7 +123,7 @@ function validateStrings(name, value, throws) { return true } if (throws) { - throw new Error(`Invalid purl: "'${name}" argument must be a string.`) + throw new PurlError(`"'${name}" must be a string`) } return false } @@ -160,8 +159,8 @@ function validateType(type, throws) { ) ) { if (throws) { - throw new Error( - `Invalid purl: type "${type}" contains an illegal character.` + throw new PurlError( + `type "${type}" contains an illegal character` ) } return false diff --git a/test/data/contrib-tests.json b/test/data/contrib-tests.json index 2c77707..4a17053 100644 --- a/test/data/contrib-tests.json +++ b/test/data/contrib-tests.json @@ -119,18 +119,6 @@ "subpath": null, "is_invalid": true }, - { - "description": "improperly encoded version string", - "purl": "pkg:maven/org.apache.commons/io@1.4.0-$@", - "canonical_purl": "pkg:maven/org.apache.commons/io@1.4.0-$@", - "type": null, - "namespace": null, - "name": "io", - "version": "1.4.0-$@", - "qualifiers": null, - "subpath": null, - "is_invalid": true - }, { "description": "leading and trailing slashes '/' are not significant and should be stripped in the canonical form", "purl": "pkg:golang//github.com///ll////xlog@v2.0.0", @@ -142,65 +130,5 @@ "qualifiers": null, "subpath": null, "is_invalid": false - }, - { - "description": "percent encoded namespace", - "purl": "pkg:type/100%/name", - "canonical_purl": "pkg:type/100%25/name", - "type": "type", - "namespace": "100%", - "name": "name", - "version": null, - "qualifiers": null, - "subpath": null, - "is_invalid": false - }, - { - "description": "percent encoded name", - "purl": "pkg:type/namespace/100%", - "canonical_purl": "pkg:type/namespace/100%25", - "type": "type", - "namespace": "namespace", - "name": "100%", - "version": null, - "qualifiers": null, - "subpath": null, - "is_invalid": false - }, - { - "description": "percent encoded version", - "purl": "pkg:type/namespace/name@100%", - "canonical_purl": "pkg:type/namespace/name@100%25", - "type": "type", - "namespace": "namespace", - "name": "name", - "version": "100%", - "qualifiers": null, - "subpath": null, - "is_invalid": false - }, - { - "description": "percent encoded qualifiers", - "purl": "pkg:type/namespace/name@1.0?a=100%", - "canonical_purl": "pkg:type/namespace/name@1.0?a=100%25", - "type": "type", - "namespace": "namespace", - "name": "name", - "version": "1.0", - "qualifiers": { "a": "100%" }, - "subpath": null, - "is_invalid": false - }, - { - "description": "percent encoded subpath", - "purl": "pkg:type/namespace/name@1.0#100%", - "canonical_purl": "pkg:type/namespace/name@1.0#100%25", - "type": "type", - "namespace": "namespace", - "name": "name", - "version": "1.0", - "qualifiers": null, - "subpath": "100%", - "is_invalid": false } ] \ No newline at end of file diff --git a/test/package-url.spec.js b/test/package-url.spec.js index b17121d..a5a2d04 100644 --- a/test/package-url.spec.js +++ b/test/package-url.spec.js @@ -216,18 +216,13 @@ describe('PackageURL', function () { describe('toString()', function () { it('type is validated', function () { ;['ty#pe', 'ty@pe', 'ty/pe', '1type'].forEach((type) => { - try { - new PackageURL(type, undefined, 'name') - assert.fail() - } catch (e) { - // prettier-ignore - assert.ok( - e.toString().includes('contains an illegal character') || - e.toString().includes('cannot start with a number.') - ) - } + assert.throws( + () => new PackageURL(type, undefined, 'name'), + /contains an illegal character|cannot start with a number/ + ) }) }) + it('encode #', function () { /* The # is a delimiter between url and subpath. */ const purl = new PackageURL( @@ -243,6 +238,7 @@ describe('PackageURL', function () { 'pkg:type/name%23space/na%23me@ver%23sion?foo=bar%23baz#sub%23path' ) }) + it('encode @', function () { /* The @ is a delimiter between package name and version. */ const purl = new PackageURL( @@ -353,6 +349,30 @@ describe('PackageURL', function () { }) assert.strictEqual(purl.subpath, 'sub@path') }) + + it('should error on decode failures', () => { + assert.throws( + () => PackageURL.fromString('pkg:type/100%/name'), + /unable to decode "namespace" component/ + ) + assert.throws( + () => PackageURL.fromString('pkg:type/namespace/100%'), + /unable to decode "name" component/ + ) + assert.throws( + () => PackageURL.fromString('pkg:type/namespace/name@100%'), + /unable to decode "version" component/ + ) + // assert.throws( + // () => + // PackageURL.fromString('pkg:type/namespace/name@1.0?a=100%'), + // /unable to decode "qualifiers" component/ + // ) + assert.throws( + () => PackageURL.fromString('pkg:type/namespace/name@1.0#100%'), + /unable to decode "subpath" component/ + ) + }) }) describe('test-suite-data', function () { @@ -360,34 +380,24 @@ describe('PackageURL', function () { describe(obj.description, function () { if (obj.is_invalid) { it(`should not be possible to create invalid ${obj.type} PackageURLs`, function () { - try { - new PackageURL( - obj.type, - obj.namespace, - obj.name, - obj.version, - obj.qualifiers, - obj.subpath - ) - assert.fail() - } catch (e) { - // prettier-ignore - assert.ok( - e.toString().includes('is a required field') || - e.toString().includes('Invalid purl') - ) - } + assert.throws( + () => + new PackageURL( + obj.type, + obj.namespace, + obj.name, + obj.version, + obj.qualifiers, + obj.subpath + ), + /is a required|Invalid purl/ + ) }) it(`should not be possible to parse invalid ${obj.type} PackageURLs`, function () { - try { - PackageURL.fromString(obj.purl) - } catch (e) { - // prettier-ignore - assert.ok( - e.toString().includes('Error: purl is missing the required') || - e.toString().includes('Invalid purl') - ) - } + assert.throws( + () => PackageURL.fromString(obj.purl), + /missing the required|Invalid purl/ + ) }) } else { it(`should be able to create valid ${obj.type} PackageURLs`, function () {