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

Incomplete percent encoding and decoding of package name #10

Open
matt-phylum opened this issue Nov 16, 2023 · 6 comments
Open

Incomplete percent encoding and decoding of package name #10

matt-phylum opened this issue Nov 16, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@matt-phylum
Copy link

package-url/purl-spec#254 proposes a new package type which sometimes uses PURLs like pkg:brew/openssl%401.1@1.1.1w. This implementation parses that PURL as having a name openssl%401.1 instead of openssl@1.1. Serializing that PURL from its expected parts results in the invalid PURL pkg:brew/openssl@1.1@1.1.1w.

@maennchen maennchen added the bug Something isn't working label Nov 16, 2023
@maennchen
Copy link
Owner

@matt-phylum Good catch. Would you be able to provide a PR?

@matt-phylum
Copy link
Author

Is the name just not decoded at all? https://github.com/maennchen/purl/blob/main/lib/purl/parser.ex#L96-L100

@maennchen
Copy link
Owner

@matt-phylum I assumed it should be done by URI.new/1 here:
https://github.com/maennchen/purl/blob/e080be5cafd0084ca9c9a20667816173b56a45c2/lib/purl/parser.ex#L9C10-L9C17

But I haven't verified if that is actually the case.

@maennchen maennchen added enhancement New feature or request and removed bug Something isn't working labels Nov 22, 2023
@maennchen
Copy link
Owner

Currently none of the package types that are part of the docs seem to allow this. This library is able to run the complete & up-to-date test suite from the specification.

I propose to wait until the format is decided and the test cases added. This way we're sure that everything is implemented correctly.

@maennchen
Copy link
Owner

I just saw package-url/purl-spec#273, waiting for that also works :)

@matt-phylum
Copy link
Author

I don't think this is blocked by spec ambiguity. The character encoding section says "the '@' version separator must be encoded as %40 elsewhere," making pkg:brew/openssl@1.1@1.1.1w invalid, even if it parses correctly when using the algorithm defined in the spec. The "How to build purl string from its components" and "How to parse a purl string in its components" sections unambiguously state that the name and version are to be percent encoded. The test suite just doesn't include tests covering this part of the spec.

The examples for pkg:swid are wrong because they encode spaces as '+' in the namespace and name (package-url/purl-spec#261), but this implementation doesn't encode spaces in names at all, so if asked to produce a pkg:swid for software that contains a space in the name, it will produce a PURL with a space in it, which is often a problem when PURLs are written into text documents. I don't know if any of the other officially supported types also allow characters that must be encoded in the name.

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

No branches or pull requests

2 participants