Fix colon encoding in tests again

[matt-phylum]

About colons, the spec says:

purl-spec/PURL-SPECIFICATION.rst

Lines 228 to 232 in 7f7e82f

	- the '#', '?', '@' and ':' characters must NOT be encoded when used as
	separators. They may need to be encoded elsewhere
	- the ':' ``scheme`` and ``type`` separator does not need to and must NOT be encoded.
	It is unambiguous unencoded everywhere

Therefore, the colon should not be encoded. I've read #364 and I can't understand the reasoning for changing the unencoded colon to be encoded. #364 also quotes the spec and references another issue, contradicting its own implementation.

9/14 implementations agree that this colon is not to be encoded.

[johnmhoran]

@matt-phylum @pombredanne These test changes are putting the cart before the horse. Let's first clarify the qualifiers component section and the related "Character encoding" section (see #398), and we can then simply apply the clarified rules to this and other tests. That would address not just the long-discussed colon but all other characters that are permitted and need to be addressed. For example, if the colon MUST be unencoded throughout the purl, we can say so, clearly.

[johnmhoran]

@pombredanne What did you mean by the language @matt-phylum cites at the top: It is unambiguous unencoded everywhere? The colon separator, i.e., the colon when (and only when) used as that defined separator? Or the colon, wherever used in the purl? Or something else? Why refer at all to "the ':' scheme and type separator" if we mean the literal colon character wherever used?

[matt-phylum]

This PR is just changing the test case back to be consistent with the existing documentation and other tests. The test used to be this way for 7 years from 2017-11-16 when the test suite was introduced until 2025-02-05. As it currently stands, you cannot pass the tests without a special case to encode : in the version but not in the name or qualifiers, which is not part of the spec.

The colon character is unambiguous unencoded everywhere. Not the colon separator. Colons only have meaning when used after pkg.

The wording of these rules has always been problematic. The first rule says that the separator must not be encoded, but that the : may need to be encoded when it's not a separator, and then second rule immediately clarifies that the case where the character :, not the separator, requires encoding is never.

Compare to the other rules:

purl-spec/PURL-SPECIFICATION.rst

Lines 234 to 236 in 7f7e82f

	- the '/' used as ``type``/``namespace``/``name`` and ``subpath`` segments separator
	does not need to and must NOT be percent-encoded. It is unambiguous unencoded
	everywhere

This one is technically wrong. There is one time when slashes are ambiguous and do need to be encoded: when they are within the name. However, it follows the same pattern of referencing the character / when not used as a separator by calling it the separator. "It is unambiguous unencoded everywhere" means slashes within qualifiers are not encoded, and the tests agree.

purl-spec/PURL-SPECIFICATION.rst

Lines 238 to 241 in 7f7e82f

	- the '@' ``version`` separator must be encoded as ``%40`` elsewhere
	- the '?' ``qualifiers`` separator must be encoded as ``%3F`` elsewhere
	- the '=' ``qualifiers`` key/value separator must NOT be encoded
	- the '#' ``subpath`` separator must be encoded as ``%23`` elsewhere

For these ones it is more obvious what's going on. @ is only the version separator when it is used as the version separator, and it must never be encoded when it is the version separator. However, the rule says that the @ character must be encoded as %40 by saying that the version separator character must be encoded when it's not the version separator, and the same pattern is used for the other characters that must be encoded.

These tests are in contradiction to the change being reverted:

purl-spec/test-suite-data.json

Lines 506 to 517 in 7f7e82f

	{
	"description": "Hugging Face model with staging endpoint",
	"purl": "pkg:huggingface/microsoft/deberta-v3-base@559062ad13d311b87b2c455e67dcd5f1c8f65111?repository_url=https://hub-ci.huggingface.co",
	"canonical_purl": "pkg:huggingface/microsoft/deberta-v3-base@559062ad13d311b87b2c455e67dcd5f1c8f65111?repository_url=https://hub-ci.huggingface.co",
	"type": "huggingface",
	"namespace": "microsoft",
	"name": "deberta-v3-base",
	"version": "559062ad13d311b87b2c455e67dcd5f1c8f65111",
	"qualifiers": {"repository_url": "https://hub-ci.huggingface.co"},
	"subpath": null,
	"is_invalid": false
	},

The colon in the qualifier is not encoded.

purl-spec/test-suite-data.json

Lines 590 to 601 in 7f7e82f

	{
	"description": "cpan module name are case sensitive",
	"purl": "pkg:cpan/URI::[email protected]",
	"canonical_purl": "pkg:cpan/URI::[email protected]",
	"type": "cpan",
	"namespace": null,
	"name": "URI::PackageURL",
	"version": "2.11",
	"qualifiers": null,
	"subpath": null,
	"is_invalid": false
	},

The colon in the name is not encoded.

[matt-phylum]

Multiple implementations contain clearly intentional code to ensure that colons are unencoded as in the original test:

https://github.com/package-url/packageurl-dotnet/blob/a42c0b8b7f29a9944b76cca2c6785b408295bd6d/src/PackageUrl.cs#L155

https://github.com/package-url/packageurl-js/blob/bb4185a4fd8f3599c8d8365fe1574e83318c75ac/src/encode.js#L62

https://github.com/package-url/packageurl-php/blob/4ef3df484b9df65b1892e27a121a457bca661e70/src/PackageUrlBuilder.php#L266

https://github.com/package-url/packageurl-python/blame/6f38e3e2f90be99d634cf72d0c0755c8c9e62084/src/packageurl/__init__.py#L64

https://github.com/sonatype/package-url-java/blob/6af9e57a6cd711693ddc5d85fa781c3e850d7395/src/main/java/org/sonatype/goodies/packageurl/PercentEncoding.java#L59-L60

[pombredanne]

@matt-phylum @johnmhoran Thanks for all the research and details!

[pombredanne]

@matt-phylum re:

9/14 implementations agree that this colon is not to be encoded.

So my intent, albeit looking weird to me in hindsight, was indeed to ensure that we would NOT encode the colon anywhere indeed. Let's update the spec accordingly and will can merge these tests afterwards.

Thank you++

[jeremylong]

LGTM - but I agree with @pombredanne, the wording in the spec should be updated

[pombredanne]

@matt-phylum re:

So my intent, albeit looking weird to me in hindsight, was indeed to ensure that we would NOT encode the colon anywhere indeed. Let's update the spec accordingly and will can merge these tests afterwards.

We are getting close to merge state in #398 for this

[johnmhoran]

Given the changes we made yesterday to the "purl separators" section -- "When percent-encoding is required, all characters MUST be encoded except for the colon ':'." -- the changes proposed for the test (leaving the ':' unencoded in the version) look good.

[pombredanne]

Agree. Especially now that #439 is merged!
Thanks.