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.

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

fix: escape / in names and versions #123

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

melotic
Copy link

@melotic melotic commented Jun 21, 2023

This will properly escape slashes in both names and versions of packages.

In addition, I've synced the tests with the ones in https://github.com/package-url/packageurl-dotnet.

@matt-phylum
Copy link

AFAIK / in the name is generally forbidden. Escaping it seems reasonable, but it's not part of the spec. Some implementations support it.

althonos/packageurl.rs: pkg:npm/a/b
anchore/packageurl-go: pkg:npm/a/b
giterlizzi/perl-URI-PackageURL: pkg:npm/a/b
maennchen/purl: pkg:npm/a/b
package-url/packageurl-dotnet: pkg:npm/a%2Fb
package-url/packageurl-go: pkg:npm/a%2Fb
package-url/packageurl-java: pkg:npm/a%2Fb
package-url/packageurl-js: pkg:npm/a%2Fb
package-url/packageurl-php: pkg:npm/a/b
package-url/packageurl-python: pkg:npm/a/b
package-url/packageurl-ruby: pkg:npm/a%2Fb
package-url/packageurl-swift: pkg:npm/a/b
phylum-dev/purl: pkg:npm/a%2Fb
sonatype/package-url-java: pkg:npm/a%2Fb

/ in the version is not supposed to be escaped, but there are no tests for it and some implementations implement parsing incorrectly.

althonos/packageurl.rs: {"type": "npm", "name": "a", "version": "b/c"}
anchore/packageurl-go: {"type": "npm", "name": "c", "namespace": "a@b"}
giterlizzi/perl-URI-PackageURL: {"type": "npm", "name": "a", "version": "b/c"}
maennchen/purl: {"type": "npm", "name": "c", "namespace": "a@b"}
package-url/packageurl-dotnet: {"type": "npm", "name": "a", "version": "b/c"}
package-url/packageurl-go: {"type": "npm", "name": "c", "namespace": "a@b"}
package-url/packageurl-java: {"type": "npm", "name": "a", "version": "b/c"}
package-url/packageurl-js: {"error": "Error: Invalid purl: version must be percent-encoded"}
package-url/packageurl-php: {"type": "npm", "name": "a", "version": "b/c"}
package-url/packageurl-python: {"type": "npm", "name": "a", "version": "b/c"}
package-url/packageurl-ruby: {"type": "npm", "name": "a", "version": "b/c"}
package-url/packageurl-swift: {"type": "npm", "name": "a", "version": "b/c"}
phylum-dev/purl: {"type": "npm", "name": "c", "namespace": "a@b"}
sonatype/package-url-java: {"type": "npm", "name": "a", "version": "b/c"}

In addition, I've synced the tests with the ones in https://github.com/package-url/packageurl-dotnet.

The tests come from https://github.com/package-url/purl-spec/blob/master/test-suite-data.json .

@jkugler
Copy link

jkugler commented Apr 26, 2024

AFAIK / in the name is generally forbidden. Escaping it seems reasonable, but it's not part of the spec. Some implementations support it.

Yes, I would say the spec is somewhat ambiguous. The spec does say this:

name:

  • The name is prefixed by a '/' separator when the namespace is not empty
  • This '/' is not part of the name
  • A name must be a percent-encoded string

The second line refers to the separator. And given that the name must be a percent-encoded string, I would think a / would be allowed. Either way, the spec needs clarifying. :)

@matt-phylum
Copy link

I mean, I don't think there's a package type which allows slashes in the name, and I don't think it would be a good idea to make a package type that would include slashes in the name. The implementations should do something consistent and the tests in the spec should specify what that is.

@jkugler
Copy link

jkugler commented Apr 26, 2024

In the spec, no, there isn't a package type that has / in the name. But because of how we're using PURLs internally at $job, there are / in the names. Given the the spec says to "append the percent-encoded {name|version} to the purl," when building the purl, I think it's reasonable that packageurl-python do so.

@jkugler
Copy link

jkugler commented Apr 26, 2024

Just thought of another use case that might want slashes in the name. If a company is using a monorepo with multiple product in one repo, you might have, say, github/org/repo_name/product_name where the actual name would be "repo_name/product_name."

if name:
name = name.replace("/", "%2F")
if version:
version = version.replace("/", "%2F")
Copy link

Choose a reason for hiding this comment

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

These should actually use urllib.parse.quote(the_string, safe='')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants