Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: address decodeURIComponent errors #76

Merged
merged 2 commits into from
Aug 28, 2024
Merged

fix: address decodeURIComponent errors #76

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b17f575
fix: only decode in parseString
jdalton Aug 20, 2024
6dd4e51
fix: error on decode with meaningful message
jdalton Aug 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

const LOOP_SENTINEL = 1_000_000

const REUSED_SEARCH_PARAMS = new URLSearchParams()

const REUSED_SEARCH_PARAMS_KEY = '_'

const REUSED_SEARCH_PARAMS_OFFSET = 2 // '_='.length

module.exports = {
LOOP_SENTINEL
LOOP_SENTINEL,
REUSED_SEARCH_PARAMS,
REUSED_SEARCH_PARAMS_KEY,
REUSED_SEARCH_PARAMS_OFFSET
}
14 changes: 14 additions & 0 deletions src/decode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict'

const { decodeURIComponent: decodeURIComponent_ } = globalThis

function decodeURIComponent(encodedURIComponent) {
try {
return decodeURIComponent_(encodedURIComponent)
} catch {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried about this. If somebody writes pkg:generic/whatever#100% it will do what they want, but if somebody writes pkg:generic/whatever#100%/100%25, the subpath will be 100%/100%25 because the decoding error applies to the entire component.

Implementations are inconsistent.

  • error: anchore/packageurl-go, package-url/packageurl-go, package-url/packageurl-java, package-url/packageurl-js (2.0.0), sonatype/package-url-java
  • 100%/100%: althonos/packageurl.rs, giterlizzi/perl-URL-PackageURL, package-url/packageurl-dotnet, package-url/packageurl-php, package-url/packageurl-python, package-url/packageurl-swift, phylum-dev/purl
  • 100%/100%25: maennchen/purl, package-url/packageurl-ruby, package-url/packageurl-js (this code)

It probably doesn't matter because it's an invalid PURL to begin with.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matt-phylum Thank you for digging into this. It is interesting.

new URL('pkg:generic/whatever#100%/100%25').toString()
// -> 'pkg:generic/whatever#100%/100%25'

While

PackageURL.fromString('pkg:generic/whatever#100%/100%25').toString()
-> 'pkg:generic/whatever#100%25/100%2525'

Updated PR to error:

PurlError: Invalid purl: unable to decode "subpath" component

return encodedURIComponent
}

module.exports = {
decodeURIComponent
}
13 changes: 7 additions & 6 deletions src/encode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
'use strict'

const {
REUSED_SEARCH_PARAMS,
REUSED_SEARCH_PARAMS_KEY,
REUSED_SEARCH_PARAMS_OFFSET
} = require('./constants')
const { isObject } = require('./objects')
const { isNonEmptyString } = require('./strings')

const reusedSearchParams = new URLSearchParams()
const reusedSearchParamKey = '_'
const reusedSearchParamOffset = 2 // '_='.length

const { encodeURIComponent } = globalThis

function encodeNamespace(namespace) {
Expand All @@ -22,9 +23,9 @@ function encodeQualifierParam(param) {
// Param key and value are encoded with `percentEncodeSet` of
// 'application/x-www-form-urlencoded' and `spaceAsPlus` of `true`.
// https://url.spec.whatwg.org/#urlencoded-serializing
reusedSearchParams.set(reusedSearchParamKey, param)
REUSED_SEARCH_PARAMS.set(REUSED_SEARCH_PARAMS_KEY, param)
return replacePlusSignWithPercentEncodedSpace(
reusedSearchParams.toString().slice(reusedSearchParamOffset)
REUSED_SEARCH_PARAMS.toString().slice(REUSED_SEARCH_PARAMS_OFFSET)
)
}
return ''
Expand Down
16 changes: 5 additions & 11 deletions src/normalize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@
const { isObject } = require('./objects')
const { isBlank } = require('./strings')

const { decodeURIComponent } = globalThis

function normalizeName(rawName) {
return typeof rawName === 'string'
? decodeURIComponent(rawName).trim()
: undefined
return typeof rawName === 'string' ? rawName.trim() : undefined
}

function normalizeNamespace(rawNamespace) {
return typeof rawNamespace === 'string'
? normalizePath(decodeURIComponent(rawNamespace))
? normalizePath(rawNamespace)
: undefined
}

Expand Down Expand Up @@ -74,22 +70,20 @@ function normalizeQualifiers(rawQualifiers) {

function normalizeSubpath(rawSubpath) {
return typeof rawSubpath === 'string'
? normalizePath(decodeURIComponent(rawSubpath), subpathFilter)
? normalizePath(rawSubpath, subpathFilter)
: undefined
}

function normalizeType(rawType) {
// The type must NOT be percent-encoded.
// The type is case insensitive. The canonical form is lowercase.
return typeof rawType === 'string'
? decodeURIComponent(rawType).trim().toLowerCase()
? rawType.trim().toLowerCase()
: undefined
}

function normalizeVersion(rawVersion) {
return typeof rawVersion === 'string'
? decodeURIComponent(rawVersion).trim()
: undefined
return typeof rawVersion === 'string' ? rawVersion.trim() : undefined
}

function qualifiersToEntries(rawQualifiers) {
Expand Down
18 changes: 12 additions & 6 deletions src/package-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ SOFTWARE.
*/
'use strict'

const { decodeURIComponent } = require('./decode')
const { isObject, recursiveFreeze } = require('./objects')
const { isBlank, isNonEmptyString, trimLeadingSlashes } = require('./strings')

Expand Down Expand Up @@ -172,10 +173,11 @@ class PackageURL {

const { pathname } = url
const firstSlashIndex = pathname.indexOf('/')
const rawType =
const rawType = decodeURIComponent(
firstSlashIndex === -1
? pathname
: pathname.slice(0, firstSlashIndex)
)
if (firstSlashIndex < 1) {
return [
rawType,
Expand Down Expand Up @@ -204,20 +206,24 @@ class PackageURL {
)
if (atSignIndex !== -1) {
// Split the remainder once from right on '@'.
rawVersion = pathname.slice(atSignIndex + 1)
rawVersion = decodeURIComponent(pathname.slice(atSignIndex + 1))
}

let rawNamespace
let rawName
const lastSlashIndex = beforeVersion.lastIndexOf('/')
if (lastSlashIndex === -1) {
// Split the remainder once from right on '/'.
rawName = beforeVersion
rawName = decodeURIComponent(beforeVersion)
} else {
// Split the remainder once from right on '/'.
rawName = beforeVersion.slice(lastSlashIndex + 1)
rawName = decodeURIComponent(
beforeVersion.slice(lastSlashIndex + 1)
)
// Split the remainder on '/'.
rawNamespace = beforeVersion.slice(0, lastSlashIndex)
rawNamespace = decodeURIComponent(
beforeVersion.slice(0, lastSlashIndex)
)
}

let rawQualifiers
Expand All @@ -231,7 +237,7 @@ class PackageURL {
const { hash } = url
if (hash.length !== 0) {
// Split the purl string once from right on '#'.
rawSubpath = hash.slice(1)
rawSubpath = decodeURIComponent(hash.slice(1))
}

return [
Expand Down
60 changes: 60 additions & 0 deletions test/data/contrib-tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,65 @@
"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
}
]
42 changes: 42 additions & 0 deletions test/package-url.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,48 @@ describe('PackageURL', function () {
testInvalid(paramName)
})
})

it('should not decode params', () => {
assert.strictEqual(
new PackageURL('type', '%21', 'name').toString(),
'pkg:type/%2521/name'
)
assert.strictEqual(
new PackageURL('type', 'namespace', '%21').toString(),
'pkg:type/namespace/%2521'
)
assert.strictEqual(
new PackageURL('type', 'namespace', 'name', '%21').toString(),
'pkg:type/namespace/name@%2521'
)
assert.strictEqual(
new PackageURL('type', 'namespace', 'name', '1.0', {
a: '%21'
}).toString(),
'pkg:type/namespace/name@1.0?a=%2521'
)
assert.strictEqual(
new PackageURL(
'type',
'namespace',
'name',
'1.0',
'a=%2521'
).toString(),
'pkg:type/namespace/name@1.0?a=%2521'
)
assert.strictEqual(
new PackageURL(
'type',
'namespace',
'name',
'1.0',
null,
'%21'
).toString(),
'pkg:type/namespace/name@1.0#%2521'
)
})
})

describe('toString()', function () {
Expand Down
Loading