Skip to content

Conversation

@johnmhoran
Copy link
Member

Reference: #378

Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

I've just pushed my initial drafting for the type component update.

There is more work to do, including at least removing my effort to incorporate inline-CSS in the new faq.rst file -- I'd hoped to make the FAQ easier to read with some basic CSS to add color to each Question: string, but the color does not show up. (The faq.rst I just pushed is displayed at https://github.com/package-url/purl-spec/blob/540617a86793ad43f405f2e926c112ffd01d2d3e/faq.rst. I'll back out the inline-CSS in my next push today.)

All comments are welcome and encouraged, including suggestions on how to make the faq.rst -- which likely will become rather well-populated over time -- as easy to read as possible.

@johnmhoran johnmhoran changed the title Update core spec and add faq.rst #378 Update 'type' component in core spec and add faq.rst #378 Jan 24, 2025
Copy link

@DennisClark DennisClark left a comment

Choose a reason for hiding this comment

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

@johnmhoran all of the changes look good to me and contribute a lot of clarity to the spec.

Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

Thanks @DennisClark . BTW, I've removed the inline CSS from faq.rst and pushed the update. Simpler is better.

Reference: #378
Signed-off-by: John M. Horan johnmhoran@gmail.com
Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Here are some nits for your consideration.

mprpic added a commit that referenced this pull request Feb 5, 2025
This section contained duplicate information that is already present for
each component in the "Rules" section and included confusing sentences
that applied too generally to certain purl components.

Each component's set of rules should clearly define how the content of
that component should (or should not) be encoded. These improvements are
being made in PRs such as #383 (more will be coming for other component
sections).
Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
Reference: #378
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

@pombredanne I've made your suggested changes, as well as additional changes to the faq.rst and a few minor changes to existing tests. In addition, I've added three new type-focused tests. Please note that each of these has is_invalid set to true, but when I use a branch of packageurl-python to run them through the test process, each of them fails.

These failures could be due in part to my inability to understand clearly how the test objects are meant to be constructed -- but I think that the Python implementation is also not correctly implementing all aspects of the core spec. Among other things, that implementation seems to allow the type rules to be violated, e.g., seems to accept these input strings as proper purls:

  • pkg:n&g?inx/nginx@0.8.9
  • pkg:3nginx/nginx@0.8.9
  • pkg:nginx:a/nginx@0.8.9

@mprpic @jkowalleck @matt-phylum I'd be very interested in your feedback as well.

@jkowalleck jkowalleck self-requested a review February 18, 2025 13:30
@matt-phylum
Copy link
Contributor

packageurl-python is an outlier.

For pkg:n&g?inx/nginx@0.8.9, althonos/packageurl.rs maennchenn/purl package-url/packageurl-dotnet package-url/packageurl-go package-url/packageurl-java package-url/packageurl-ruby complain about a missing type and/or name, anchore/packageurl-go and phylum-dev/purl complain about an invalid qualifier, package-url/packageurl-js complains about invalid characters in the type, and package-url/packageurl-php package-url/packageurl-swift sonatype/package-url-java give a generic error. No other implementation returns a successful parse. The Python implementation is not following the documented parsing algorithm which would result in something like {"subpath": null, "qualifiers": {"inx/nginx@0.8.9": null}, "type": "n&g", "name": null, "version": null}.

For pkg:3nginx/nginx@0.8.9, althonos/packageurl.rs maennchen/purl package-url/packageurl-go package-url/packageurl-java package-url/packageurl-js complain about an invalid package type, giterlizzi/perl-URI-PackageURL package-url/packageurl-dotnet sonatype/package-url-java give a generic error, and anchore/packageurl-go package-url/packageurl-php package-url/packageurl-ruby package-url/packageurl-swift phylum-dev/purl parse successfully. I think this PURL is structurally valid and therefore should be parseable, but "3nginx" is a forbidden package type so some implementations reject it.

For pkg:nginx:a/nginx@0.8.9, althonos/packageurl.rs maennchen/purl package-url/packageurl-go package-url/packageurl-java package-url/packageurl-js phylum-dev/purl complain about an invalid package type, giterlizzi/perl-URI-PackageURL package-url/packageurl-dotnet sonatype/package-url-java give a generic error, and anchore/packageurl-go package-url/packageurl-php package-url/packageurl-ruby package-url/packageurl-swift parse successfully. This case is similar to the previous one but more implementations reject this one because : is never allowed whereas 3 is allowed if it is not the first character.

'.', '+' and '-' (period, plus, and dash).
- The ``type`` MUST start with an ASCII letter.
- The ``type`` MUST NOT be percent-encoded.
- The ``type`` is case insensitive. The canonical form is lowercase.
Copy link
Member

@jkowalleck jkowalleck Feb 18, 2025

Choose a reason for hiding this comment

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

✔️ we have the canonicalization in the test suite already:

"description": "valid go purl with version and subpath",
"purl": "pkg:GOLANG/google.golang.org/genproto@abcdedf#/googleapis/api/annotations/",
"canonical_purl": "pkg:golang/google.golang.org/genproto@abcdedf#googleapis/api/annotations",
"type": "golang",
"namespace": "google.golang.org",
"name": "genproto",
"version": "abcdedf",
"qualifiers": null,
"subpath": "googleapis/api/annotations",
"is_invalid": false

@johnmhoran
Copy link
Member Author

@mprpic @jkowalleck @matt-phylum In case you have time tomorrow, I hope you'll be able to join the next biweekly 30-minute purl community meeting -- tomorrow 2025-02-19 at 17:00 UTC/09:00 PST (details are here).

Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Great work! Thanks!

@pombredanne pombredanne merged commit 25c8fe5 into master Feb 19, 2025
@pombredanne pombredanne deleted the 378-update-type-component branch February 19, 2025 17:22
@johnmhoran johnmhoran mentioned this pull request Apr 3, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants