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

Closed
matt-phylum opened this issue Nov 16, 2023 · 8 comments · Fixed by #19
Closed

Incomplete percent encoding and decoding of package name #10

matt-phylum opened this issue Nov 16, 2023 · 8 comments · Fixed by #19
Assignees
Labels
bug Something isn't working

Comments

@matt-phylum
Copy link
Contributor

package-url/purl-spec#254 proposes a new package type which sometimes uses PURLs like pkg:brew/openssl%[email protected]. This implementation parses that PURL as having a name openssl%401.1 instead of [email protected]. Serializing that PURL from its expected parts results in the invalid PURL pkg:brew/[email protected]@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
Contributor 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
Contributor 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/[email protected]@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.

@matt-phylum
Copy link
Contributor Author

This is already a problem with published types: pkg:maven/net.databinder/dispatch-http%[email protected]

The name should be dispatch-http%2Bjson_2.7.3 but this implementation returns dispatch-http%252Bjson_2.7.3 instead.

@maennchen
Copy link
Owner

@matt-phylum I updated to the latest spec and we are agin passing the reference test suite.

It uncovered an issue in a non-maven specific test, but could potentially solve your issue as well:
d7c675a#diff-a953342af3262033f4b199fd49ce358c8cd9f9ec1235269fe4b64c1670d3fde0R50

Can you verify if that works for you?

@maennchen maennchen self-assigned this Mar 4, 2025
@maennchen maennchen added bug Something isn't working and removed enhancement New feature or request labels Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants