Repository navigation
fix: decode PURL percent-encoding in versions and package paths - #244
Conversation
andrew
left a comment
There was a problem hiding this comment.
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.
follow-up:What changed
The last two are fed by new Escaping on the way out and decoding per path segment on the way in round-trips The bug this uncoveredWriting the requested literal- chi routes on
So the same handler sees both forms and only one of them may be decoded. The decode is While threading that through, the five identical TestsCovering
Reverting the template escaping fails the last one, so it is not vacuous. |
another claude generated patch to fix the following behaviour:

Problem
Package versions containing
+were unusable in the UI. Reported againstnmap_7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1_amd64.debfrom 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 aspkg:deb/nmap@7.91%2Bdfsg1%2B…. Two places treated the raw substring after@as the version:Version.Version()(strings.LastIndex(purl, "@")) and the SQLSUBSTR(v.purl, INSTR(v.purl,'@') + 1)inGetRecentlyCachedPackagesnever decoded, so%2Bleaked into the dashboard and package pages.r.URL.RawPathwhenever the URL contains an escape, so the handler received7.91%2B…back and re-encoded it intopkg: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— addVersionFromPURL(strips qualifiers/subpath, then percent-decodes);Version.Version()uses it. AddVersion.DisplayPURL()for a readable PURL in the UI, decoding all path components.internal/database/queries.go—GetRecentlyCachedPackagesselectsv.purland derives the version in Go. This also removes the duplicated Postgres-specificSUBSTRING/POSITIONvariant of the query.internal/server/resolve.go—splitWildcardPathpercent-decodes each segment after splitting, so an encoded%2Finside a name is not mistaken for a separator.DisplayPURLfor 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
validatePackagePathnow 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..%2Fadminnow 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
internal/database/version_purl_test.go(VersionFromPURL,DisplayPURL, andGetRecentlyCachedPackagesdecoding under both sqlite and Postgres).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~).+,+after-,~dfsg, leading0~, and npm@scope.~is correctly left unencoded (it is PURL-safe).golangci-lintpass; the 6 remaining staticcheck warnings are pre-existing in untouched test files.