Refuse an image URL whose source path or query holds a # (closes #286) #287
@@ -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`,
|
||||||
|
|||||||
@@ -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 "#"`)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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:]
|
||||||
|
|||||||
Reference in New Issue
Block a user