test: unencoded subpath cannot contain . or ..

[jeremylong]

Add test cases to check for invalid . and .. segments in the subpath.

[jkowalleck]

see also: https://github.com/package-url/purl-spec/pull/394/files#r1954552460

[matt-phylum]

I'm wondering if this is actually a good idea. I can agree that the subpath should not contain . or .. segments, and I would prefer if implementations didn't need path normalization logic, but I don't think parsers and formatters should validate that the subpath does not contain . or .. segments. The problem is that if you observe a PURL pkg:golang/google.golang.org/genproto@abcdedf#/googleapis/../api/annotations/ it cannot be understood at all if the parser rejects the .., and it cannot be canonicalized or otherwise parsed and then written back out if the formatter rejects the ... It's syntactically valid and at least the type name version have an clear, unambiguous meaning, but because of the .. everything is thrown away.

[jkowalleck]

[...] but I don't think parsers and formatters should validate that the subpath does not contain . or .. segments.

why? i mean, this is already part of the spec:

• spec

  purl-spec/PURL-SPECIFICATION.rst

  Line 212 in 65d98c4

  - must not be any of '..' or '.'

• generate

  purl-spec/PURL-SPECIFICATION.rst

  Line 322 in 65d98c4

  - Discard empty, '.' and '..' segments

• parse

  purl-spec/PURL-SPECIFICATION.rst

  Line 346 in 65d98c4

  - Discard any '.' or '..' segment from that split

The problem is that if you observe a PURL pkg:golang/google.golang.org/genproto@abcdedf#/googleapis/../api/annotations/ it cannot be understood at all if the parser rejects the .., and it cannot be canonicalized or otherwise parsed and then written back out if the formatter rejects the ... It's syntactically valid and at least the type name version have an clear, unambiguous meaning, but because of the .. everything is thrown away.

per spec, the .. and . are to be discarded, rest is to be kept untouched. to attempt of path-resolution or anything. foo/../bar becomes foo/bar. (not bar)

[matt-phylum]

Then these tests are wrong, right? They have "is_invalid": true, and they include the . and .. in the parsed subpaths.

[dwalluck]

pkg:golang/google.golang.org/genproto@abcdedf#/googleapis/../api/annotations/ it cannot be understood at all if the parser rejects the .., and it cannot be canonicalized or otherwise parsed and then written back out if the formatter rejects the ... It's syntactically valid and at least the type name version have an clear, unambiguous meaning, but because of the .. everything is thrown away.

My problem is that a human reading /googleapis/../api/annotations/ would assume that it means /api/annotations/ when in fact it means /googleapis/api/annotations/. So, I'd prefer an error instead of silently dropping.

The same could be said for qualifiers keys that are not lowercase or are empty, too, in that what the parser does, and what a human might assume that they mean are two different things.

[jkowalleck]

pkg:golang/google.golang.org/genproto@abcdedf#/googleapis/../api/annotations/ it cannot be understood at all if the parser rejects the .., and it cannot be canonicalized or otherwise parsed and then written back out if the formatter rejects the ... It's syntactically valid and at least the type name version have an clear, unambiguous meaning, but because of the .. everything is thrown away.

My problem is that a human reading /googleapis/../api/annotations/ would assume that it means /api/annotations/ when in fact it means /googleapis/api/annotations/. So, I'd prefer an error instead of silently dropping.

The same could be said for qualifiers keys that are not lowercase or are empty, too, in that what the parser does, and what a human might assume that they mean are two different things.

this would be a change of the current spec.
please open an extra ticket for that.

[johnmhoran]

LGTM!

[dwalluck]

Sorry to comment on a closed pull request, but the subpath in the test refers to the non-normalized subpath. This doesn't seem right---aren't the components supposed to refer to the components in the canonical purl? For example, like type in this same example refers to "golang" and not "GOLANG".

After the segments are dropped, the resulting subpath component will not contain the dots but the test has them.

[jkowalleck]

Sorry to comment on a closed pull request, but the subpath in the test refers to the non-normalized subpath. This doesn't seem right---aren't the components supposed to refer to the components in the canonical purl? For example, like type in this same example refers to "golang" and not "GOLANG".

After the segments are dropped, the resulting subpath component will not contain the dots but the test has them.

please open a pull request or annotate the code you are referring to.

please be aware of the purpose of the test components:

• purl-spec/PURL-SPECIFICATION.rst

  Lines 473 to 485 in 65d98c4

  	To test ``purl`` parsing and building, a tool can use this test suite and for
  	every listed test object, run these tests:
  	- parsing the test canonical ``purl`` then re-building a ``purl`` from these parsed
  	components should return the test canonical ``purl``
  	- parsing the test ``purl`` should return the components parsed from the test
  	canonical ``purl``
  	- parsing the test ``purl`` then re-building a ``purl`` from these parsed components
  	should return the test canonical ``purl``
  	- building a ``purl`` from the test components should return the test canonical ``purl``

[dwalluck]

Opened #404 with an explanation.