Refuse an image URL whose source path or query holds a # (closes #286)
check / check (push) Canceled after 0s

A # written as %23 in the source path or query reached the parser
decoded and stayed in the source URL, where Go reads it as the start of
a fragment and drops it before fetching. So each text after the # was
another unsigned spelling of one upstream request, cached under its own
keys. An image URL whose decoded source path or query holds a # is now
refused with 400, signed or not, and README.md's rule for writing a
source path and query says a # cannot be part of either.

Model: opus-5-5
This commit was merged in pull request #287.
This commit is contained in:
2026-10-16 01:36:56 +02:00
parent 62b4f4791e
commit 1fd5cf8bd0
4 changed files with 64 additions and 2 deletions
+3 -2
View File
@@ -323,8 +323,9 @@ first `/`, the size and format after the last `/`, and between them the source
path up to the first `?` and the source query after it. So the `?` before the path up to the first `?` and the source query after it. So the `?` before the
query, any `?` in the query, and every `%` in the source path or query must be query, any `?` in the query, and every `%` in the source path or query must be
percent-encoded, as `%3F` and `%25`. Any other character may be written as percent-encoded, as `%3F` and `%25`. Any other character may be written as
itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. For itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. Neither
example, the source path nor the source query may hold a `#`, and an image URL that
writes one as `%23` is refused with 400. For example,
`https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg` at `https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg` at
800x600 WebP is 800x600 WebP is
`/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp`, `/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp`,
+5
View File
@@ -30,6 +30,11 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-15 a `/v1/image/` URL whose source path or source query holds a `#`,
written as `%23`, is refused with 400 (closes #286): the source URL built from
it ended at the `#` when fetched, so each text after it fetched the same
upstream image, without a signature for an allowlisted host, and cached it
under keys of its own. `README.md` says neither may hold a `#`.
- 2026-10-15 the host of a `/v1/image/` URL must be a host name or an IPv4 - 2026-10-15 the host of a `/v1/image/` URL must be a host name or an IPv4
address (letters, digits, hyphens, underscores and dots) or an IPv6 address in address (letters, digits, hyphens, underscores and dots) or an IPv6 address in
brackets, either one optionally followed by `:` and a port, and any other host brackets, either one optionally followed by `:` and a port, and any other host
@@ -340,3 +340,51 @@ func TestHandleImage_HostForms_Served(t *testing.T) {
}) })
} }
} }
// TestHandleImage_HashInSourcePathOrQuery_Returns400 sends image URLs whose
// source path or source query holds a "#", written as %23, and checks that
// each is refused with 400, the one signed for that source path included. The
// source URL would otherwise end at the "#" when it is fetched, so each text
// after it would fetch the image at photoPath and cache it under keys of its
// own.
func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
t.Parallel()
photos := newPhotoFetcher(t, allowlistedHost, signedHost)
exp := time.Now().Add(time.Hour)
sig := signature.New(testSigningKey).Sign(&signature.Request{
SourceHost: signedHost,
SourcePath: photoPath + "#x",
Width: 50,
Height: 50,
Format: string(imgcache.FormatJPEG),
Quality: 85,
FitMode: string(imgcache.FitCover),
Expires: exp,
})
tests := []struct {
name string
target string
}{
{"# in the source path",
"/v1/image/" + allowlistedHost + photoPath + "%23x/50x50.jpeg"},
{"# in the source query",
"/v1/image/" + allowlistedHost + photoPath + "%3Fv=1%23x/50x50.jpeg"},
{"# in the source path, signed",
fmt.Sprintf("/v1/image/%s%s%%23x/50x50.jpeg?sig=%s&exp=%d",
signedHost, photoPath, sig, exp.Unix())},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rec := sendGet(t, newImageRoute(t, photos), tt.target)
checkErrorBody(t, rec, http.StatusBadRequest,
`invalid image URL: invalid image path: `+
`the source path or query holds a "#"`)
})
}
}
+8
View File
@@ -171,6 +171,14 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
// host // host
host = strings.ToLower(host) host = strings.ToLower(host)
// Go's URL parser, which the fetcher uses, reads a "#" as the start of a
// fragment and drops the rest, so each text after it would be another
// spelling, cached under its own keys, of one upstream request
if strings.Contains(path, "#") {
return nil, fmt.Errorf(
`%w: the source path or query holds a "#"`, ErrInvalidPath)
}
// Extract query string if present in path // Extract query string if present in path
if qIndex := strings.Index(path, "?"); qIndex != -1 { if qIndex := strings.Index(path, "?"); qIndex != -1 {
query = path[qIndex+1:] query = path[qIndex+1:]