Skip to content

fix: decode PURL percent-encoding in versions and package paths - #244

Merged
andrew merged 2 commits into
git-pkgs:mainfrom
wickedOne:purl-version-encoding
Aug 14, 2026
Merged

andrew merged 2 commits into
git-pkgs:mainfrom
wickedOne:purl-version-encoding

Conversation

@wickedOne

Copy link
Copy Markdown
Contributor

another claude generated patch to fix the following behaviour:
image

Problem

Package versions containing + were unusable in the UI. Reported against nmap_7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1_amd64.deb from the Ubuntu archive (PROXY_UPSTREAM_DEBIAN=http://archive.ubuntu.com/ubuntu), which displayed as @7.91%2Bdfsg1%2Breally7.80%2Bdfsg1-2ubuntu0.1.

The artifact download itself was always fine — the bug was in reading the version back out of the PURL. PURL percent-encodes + in path components, so the version is stored as pkg:deb/nmap@7.91%2Bdfsg1%2B…. Two places treated the raw substring after @ as the version:

  1. Display — Version.Version() (strings.LastIndex(purl, "@")) and the SQL SUBSTR(v.purl, INSTR(v.purl,'@') + 1) in GetRecentlyCachedPackages never decoded, so %2B leaked into the dashboard and package pages.
  2. Round-trip — that encoded string went into the version link. chi routes on r.URL.RawPath whenever the URL contains an escape, so the handler received 7.91%2B… back and re-encoded it into pkg:deb/nmap@7.91%252B…. The version page 404'd.

This affects any ecosystem with non-alphanumeric version characters, not just Debian: npm scoped packages had the same problem via %40.

Changes

  • internal/database/types.go — add VersionFromPURL (strips qualifiers/subpath, then percent-decodes); Version.Version() uses it. Add Version.DisplayPURL() for a readable PURL in the UI, decoding all path components.
  • internal/database/queries.go — GetRecentlyCachedPackages selects v.purl and derives the version in Go. This also removes the duplicated Postgres-specific SUBSTRING/POSITION variant of the query.
  • internal/server/resolve.go — splitWildcardPath percent-decodes each segment after splitting, so an encoded %2F inside a name is not mistaken for a separator.
  • Templates use DisplayPURL for version headings and labels.

Storage is unchanged. Stored PURLs keep their canonical encoding (%2B, %40), which is what the API, cache keys and all lookups use. This is display and inbound routing only, so no cache migration is needed. Old encoded URLs still resolve, so existing bookmarks keep working.

Security

validatePackagePath now validates the decoded segments and rejects .. path elements. This closes a traversal vector: package names are rejoined and interpolated straight into upstream URLs by several registries (e.g.
fmt.Sprintf("%s/web/packages/%s/DESCRIPTION", baseURL, name) for CRAN), and Go sends dot-segments verbatim for the upstream to resolve. GET /api/package/cran/pkg%2F..%2F..%2Fadmin now returns 400 instead of reaching upstream as /web/packages/pkg/../../admin/DESCRIPTION.

Note this is stricter than before the change: the original code had no .. check at all, so a literal ../.. also passed validation.

Testing

  • New: internal/database/version_purl_test.go (VersionFromPURL, DisplayPURL, and GetRecentlyCachedPackages decoding under both sqlite and Postgres).
  • Extended: resolve_test.go (segment decoding, traversal rejection), server_test.go (version page reachable via both encoded and decoded URLs), debian_test.go (pool paths with + and ~).
  • Each new test was confirmed to fail without the fix.
  • Verified end-to-end against the real Ubuntu archive with 12 packages across deb/npm/cargo/pypi, following every version link the UI generates: all 200, no encoding leaks. Version shapes covered include multiple +, + after -, ~dfsg, leading 0~, and npm @scope. ~ is correctly left unencoded (it is PURL-safe).
  • Full suite and golangci-lint pass; the 6 remaining staticcheck warnings are pre-existing in untouched test files.

@andrew andrew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Version.Version() now returns decoded text, but the templates insert it directly into URL path segments. html/template preserves reserved characters and valid escapes, so versions such as release%2F1, v1%3Fbuild, or a literal %2B generate links that route to the wrong version or return 404. Please keep decoded text for display but use a separately path-escaped value for package, browse, and compare URLs, with tests for /, ?, and literal %xx sequences.

@wickedOne

Copy link
Copy Markdown
Contributor Author

follow-up:

What changed

Version.Version() still returns decoded text, but nothing builds a URL from it any
more. database.Version.EscapedVersion() (url.PathEscape over the decoded version)
is now the value every link uses:

Template Change
package_show.html version href, and the compare checkboxes (data-version → data-version-path)
version_show.html browse link
browse_source.html breadcrumb href, and the JS constant the browse API URL is built from (version → versionPath)
compare_versions.html the JS constants the compare API URL is built from (fromVersion/toVersion → …Path)

The last two are fed by new EscapedVersion and Escaped{From,To}Version fields on
BrowseSourceData and ComparePageData.

Escaping on the way out and decoding per path segment on the way in round-trips
exactly, so the rebuilt PURL is the one that was stored. + stays literal (it is
legal in a path segment), so Debian versions still read naturally in the URL bar.

The bug this uncovered

Writing the requested literal-%xx test turned up a routing bug, not just a
link-generation one: /ui/package/deb/nmap/1.0%252B — the correctly escaped link for
a version whose text is 1.0%2B — returned 404.

chi routes on r.URL.RawPath when it is set and on r.URL.Path otherwise, and
net/url only sets RawPath when the request's escaping differs from the canonical
encoding of the decoded path:

  • 1.0%252B is the canonical encoding of 1.0%2B, so RawPath is empty and the
    wildcard arrives already decoded once. The original unconditional decode in
    splitWildcardPath decoded it a second time, yielding 1.0+ and a lookup for a
    version that does not exist.
  • release%2F1 and 7.91%2Bdfsg1 are non-canonical, set RawPath, and do need the
    decode.

So the same handler sees both forms and only one of them may be decoded. The decode is
now conditional on wildcardIsEncoded(r) (r.URL.RawPath != "").

While threading that through, the five identical chi.URLParam + validate + split
blocks in server.go, api.go and browse.go collapsed into one
packagePathSegments(r). validatePackagePath now checks each segment both as the
handlers see it and decoded once more, so .. and control-character rejection no
longer depends on which form arrived — an escape that reaches a handler undecoded is
decoded by the upstream registry instead.

Tests

Covering /, ?, literal %xx and +:

  • TestVersionEscapedVersion — escape/decode round trip.
  • TestPackagePathSegments — drives a real chi router, so both the raw and the
    pre-decoded arrival are exercised.
  • TestVersionURLEscaping — follows the links the pages actually render (package →
    version → browse) and rebuilds the compare URL from the checkbox attributes.

Reverting the template escaping fails the last one, so it is not vacuous.

@wickedOne
wickedOne requested a review from andrew August 13, 2026 13:51
@andrew
andrew merged commit 849500d into git-pkgs:main Aug 14, 2026
5 checks passed
@wickedOne
wickedOne deleted the purl-version-encoding branch August 14, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants