Don't require encoding for namespaced packages

[scovetta]

Currently, we require namespaced packages to have the @ and / symbols encoded. I don't recall precisely why this was needed, but we should fix this so that you can just have, for example:

oss-download pkg:npm/@microsoft/fast-web-utilities

Ref: #418

[pmalmsten]

@scovetta @gfs You might want to be careful before doing this. As I mentioned on #418, the Package URL spec expects @ to be percent-encoded in package names in order to avoid parsing ambiguity, since @ indicates that characters to the right should be interpreted as a version string. The implications of accepting package URLs that deviate from the spec might be difficult to predict.

Maybe instead we could improve the error messaging? E.g. if a given Package URL fails to parse, we could check for cases like this (e.g. the package URL looks like an npm package, with @ in the namespace position) and print a more helpful error message explaining that namespace and `name components of package URLs need to be percent-encoded?

[gfs]

I think the reporting user makes a fair point that even if that is what the spec says it's not very intuitive.

Particularly for handling perhaps manually typed input on a command line if we can reasonably detect this case where the @ for the namespace needs to be encoded why don't we just automatically do it on the users behalf?

[scovetta]

Actually, I think we deviate from the spec too a bit - in at least some cases, we require the slash separator between namespace and name to be encoded. This means we get namespace=[blank] and name=foo%2fbar. I don't recall why this was needed.

// FAILS
oss-download pkg:npm/%40microsoft/fast-web-utilities

// SUCCESS
oss-download pkg:npm/%40microsoft%2ffast-web-utilities

According to the spec, that / separator between namespace and name must not be encoded.

[pmalmsten]

Particularly for handling perhaps manually typed input on a command line if we can reasonably detect this case where the @ for the namespace needs to be encoded why don't we just automatically do it on the users behalf?

@gfs That's not an unreasonable point. I can't think of any way that might go wrong - but that said, the types of subtle bugs that can result from tolerating deviations from specs tend to be hard to predict.

If you go forward with that approach, would you mind keeping that tolerant parsing behavior at the CLI layer? For Terrapin (which consumes the Shared and oss-find-squats-lib libraries), strict compliance with spec is part of how we make sure subtle bugs in our system get surfaced while handling millions of distinct package versions. (I think those libraries primarly work with already-parsed Package URL objects so this point might be a non-issue - but I thought I would mention it just in case any part(s) of those libraries accepts package URLs as strings).

[gfs]

Yes, I can keep it at the CLI layer. Makes sense to me.

…

On Tue, May 16, 2023 at 10:35, Paul Malmsten ***@***.***(mailto:On Tue, May 16, 2023 at 10:35, Paul Malmsten <<a href=)> wrote: > Particularly for handling perhaps manually typed input on a command line if we can reasonably detect this case where the @ for the namespace needs to be encoded why don't we just automatically do it on the users behalf? ***@***.***(https://github.com/gfs) That's not an unreasonable point. I can't think of any way that might go wrong - but that said, the types of subtle bugs that can result from tolerating deviations from specs tend to be hard to predict. If you go forward with that approach, would you mind keeping that tolerant parsing behavior at the CLI layer? For Terrapin (which consumes the [Shared](https://github.com/microsoft/OSSGadget/tree/main/src/Shared) and [oss-find-squats-lib](https://github.com/microsoft/OSSGadget/tree/main/src/oss-find-squats-lib) libraries), strict compliance with spec is part of how we make sure subtle bugs in our system get surfaced while handling millions of distinct package versions. (I think those libraries primarly work with already-parsed Package URL objects so this point might be a non-issue - but I thought I would mention it just in case any part(s) of those libraries accepts package URLs as strings). — Reply to this email directly, [view it on GitHub](#420 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAYEVE3HIJKH7YB4AVNWXLXGO3GXANCNFSM6AAAAAAYBPCERM). You are receiving this because you were mentioned.Message ID: ***@***.***>