8 Commits
Author SHA1 Message Date
sneak ea77bdfa63 Scale both sides of a fit=cover image by the same amount (part of #282)
check / check (push) Waiting to run
cropCentreAndResize cut the centre to whole source pixels and forced
that to the requested size, so the two sides were scaled by different
amounts: a 16x16 source at 100x60 came out squashed by 4%. It now
widens the centre to whole source pixels and lets thumbnail with a
centre crop scale that by the one amount next uses and cut the output
from it. thumbnail copies the scaled image into memory, which is larger
than the output by less than one scaled source pixel on each side.
TODO.md says so.

Model: opus-5-5
2026-10-15 23:39:38 +00:00
sneak eaac8fd7cf Test that fit=cover keeps the source's proportions (part of #282)
A grey square in the centre of the source must come out square: a
16x16 source at 100x60, a 10x10 one at 100x35 and a 64x64 one at
300x100, and each turned on its side. The square's width and height
are measured where its blurred edges cross the grey halfway between
the square's and the background's, read to a fraction of a pixel.

Model: opus-5-5
2026-10-15 23:39:24 +00:00
sneak 70631def34 Drop the fit=cover test cases that fill memory without the fix (part of #282)
Without the fix, the 8192x64 and 64x8192 cases each made libvips ask for
about 24 GiB, so a return of the bug would take the build host's memory
instead of failing the test. The cases left fail at once without the fix.

Model: opus-5-5
2026-10-15 23:39:24 +00:00
sneak 4d59886082 Test that fit=cover serves a source whose centre is under a pixel wide (part of #282)
An 8192x1 source at 1x8192, or the same turned on its side, has a centre
with the requested proportions 1/8192 of a pixel wide. The crop must keep
at least one pixel of it, or the request fails. The test's cases are now
named by source and requested size.

Model: opus-5-5
2026-10-15 23:39:24 +00:00
sneak 96d5ccf5cc Crop the centre first for fit=cover (closes #282)
libvips' thumbnail with a centre crop scales the whole source to cover
the requested size and copies that into memory before it crops: 8192x64
at 8192x8192 made it allocate about 24 GiB, and 8192x1 failed with 500.
resize now cuts the centre of the source to the requested size's
proportions first and resizes that, so no image larger than the source
or the output is made. The crop is the same centre as before. TODO.md
says so.

Model: opus-5-5
2026-10-15 23:39:24 +00:00
sneak fb33a3bb34 Test that fit=cover serves a thin source and keeps its centre (part of #282)
An 8192x64 or 8192x1 source, or one turned on its side, requested at
8192x8192 with fit=cover must be served at that size, and fit=cover must
keep the centre of an ordinary source.

Model: opus-5-5
2026-10-15 23:39:24 +00:00
clawbot 1fd5cf8bd0 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
2026-10-16 01:36:56 +02:00
clawbot 62b4f4791e Refuse an image URL whose host is not a host name or IP address (closes #275)
check / check (push) Canceled after 0s
A ? written into the host segment as %3F reached the parser decoded,
so cdn.example.com?x=1 passed the allowlist as cdn.example.com, fetched
the host's root path with that query, and was cached under its own keys:
another unsigned spelling of one upstream request. The host segment is
now accepted only as a host name, an IPv4 address or a bracketed IPv6
address, with an optional port, and anything else is refused with 400.
An IPv6 address without brackets is now refused instead of needing a
signature; a host name outside ASCII is refused, its punycode accepted.

Model: opus-5-5
2026-10-16 01:14:51 +02:00
4 changed files with 179 additions and 7 deletions
+8 -3
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
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
itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. For
example,
itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. Neither
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
800x600 WebP is
`/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp`,
@@ -575,7 +576,11 @@ pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
`CDN.Example.com` is matched, fetched, cached and counted against
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
even when the host is allowlisted. A host holding `@` is refused with 400.
even when the host is allowlisted. The host must be a host name or an IPv4
address (letters, digits, hyphens, underscores and dots) or an IPv6 address in
brackets, either one optionally followed by `:` and a port. Any other host, such
as one holding `@`, or a `?`, `#`, `%`, `\` or space written percent-encoded, is
refused with 400.
### Configuration
+16
View File
@@ -42,6 +42,22 @@ P2: security: per-origin rate limiting
resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for
the 8192x1 one. The output can sit a fraction of an output pixel from where it
was.
- 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
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
is refused with 400 (closes #275): only `@` was refused, so a `?` written as
`%3F` gave the host `cdn.example.com?x=1`, which Go's URL parser reads as
`cdn.example.com` with a query. Such a URL passed the allowlist, and each text
after the `?` fetched the host's root path and cached the answer under keys of
its own. A `#`, `%`, `\` or space written percent-encoded is refused the same
way, and so is an IPv6 address without brackets, which needed a signature
before. Underscores are kept because `allowlist_hosts` accepts host names
holding them.
- 2026-10-15 the README says how a source URL with a query is written into an
image URL, and the image route accepts it (closes #255): the query follows the
source path after `%3F`. pixa now reads the image URL's path decoded once, so
@@ -264,3 +264,127 @@ func TestHandleImage_AllowlistOrSignature(t *testing.T) {
})
}
}
// TestHandleImage_InvalidHost_Returns400 sends image URLs whose host no URL
// host can be, and checks that each is refused with 400 naming the host as
// decoded: allowlistedHost with a character after it that a URL host cannot
// hold, percent-encoded, an IPv6 address without brackets, and brackets
// around something that is not an IPv6 address. A "?" or "#" after
// allowlistedHost would otherwise end the host when the source URL is read,
// so the request would pass the allowlist and be fetched and cached under a
// host that is not the one fetched.
func TestHandleImage_InvalidHost_Returns400(t *testing.T) {
t.Parallel()
photos := newPhotoFetcher(t, allowlistedHost)
tests := []struct {
name string
writtenHost string
decodedHost string
}{
{"? written as %3F", allowlistedHost + "%3Fx=1", allowlistedHost + "?x=1"},
{"# written as %23", allowlistedHost + "%23x", allowlistedHost + "#x"},
{"% written as %25", allowlistedHost + "%253F", allowlistedHost + "%3F"},
{`\ written as %5C`, allowlistedHost + "%5Cx", allowlistedHost + `\x`},
{"space written as %20", allowlistedHost + "%20", allowlistedHost + " "},
{"IPv6 address without brackets", "2001:db8::1", "2001:db8::1"},
{"IPv4 address in brackets", "[203.0.113.5]", "[203.0.113.5]"},
{"no address in brackets", "[abc]", "[abc]"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rec := sendGet(t, newImageRoute(t, photos), photoURL(tt.writtenHost))
checkErrorBody(t, rec, http.StatusBadRequest,
fmt.Sprintf("invalid image URL: invalid source host: %q",
tt.decodedHost))
})
}
}
// TestHandleImage_HostForms_Served checks that the image route serves, with
// a valid signature, the image at photoPath on a host name, an IPv4 address,
// an IPv6 address in brackets, and a host followed by ":" with or without a
// port.
func TestHandleImage_HostForms_Served(t *testing.T) {
t.Parallel()
hosts := []string{
allowlistedHost,
"img_1.example.com",
"203.0.113.5",
"[2001:db8::1]",
allowlistedHost + ":8443",
"[2001:db8::1]:8443",
allowlistedHost + ":",
}
photos := newPhotoFetcher(t, hosts...)
exp := time.Now().Add(time.Hour)
for _, host := range hosts {
t.Run(host, func(t *testing.T) {
t.Parallel()
sig := photoSignature(testSigningKey, host, exp)
rec := sendGet(t, newImageRoute(t, photos),
photoURLWithSig(host, sig, exp))
if rec.Code != http.StatusOK {
t.Errorf("status = %d, want %d: %s",
rec.Code, http.StatusOK, rec.Body)
}
})
}
}
// 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 "#"`)
})
}
}
+31 -4
View File
@@ -3,6 +3,7 @@ package imgcache
import (
"errors"
"fmt"
"net/netip"
"net/url"
"regexp"
"strconv"
@@ -50,6 +51,14 @@ var sizeFormatRegex = regexp.MustCompile(
// percentEscapeRegex matches one percent-escape, such as "%2e"
var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
// hostRegex matches the forms the host of an image URL may take: a host name
// or an IPv4 address (letters, digits, "-", "_" and "."), or an IPv6 address
// in brackets (hex digits, ":" and "."), either one optionally followed by ":"
// and a port, which may be empty. Its one group holds the text in brackets,
// which parseImageComponents also checks is an IPv6 address.
var hostRegex = regexp.MustCompile(
`^(?:[A-Za-z0-9_.-]+|\[([0-9A-Fa-f:.]+)\])(?::[0-9]*)?$`)
// ParsedURL contains the parsed components of an image proxy URL.
type ParsedURL struct {
// Host is the source origin host (e.g., "cdn.example.com")
@@ -141,10 +150,20 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
return nil, ErrMissingHost
}
// A URL reads what comes before "@" as a user name and password, and the
// rest as the host
if strings.Contains(host, "@") {
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
// Only the forms hostRegex describes are accepted. A "?", "#" or "@" in
// the host would make Go's URL parser, which the allowlist check and the
// fetcher use, read a different host from the one allowlisted and cached
hostMatch := hostRegex.FindStringSubmatch(host)
if hostMatch == nil {
return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
}
// Go's URL parser refuses brackets around anything but an IPv6 address
if inBrackets := hostMatch[1]; inBrackets != "" {
addr, err := netip.ParseAddr(inBrackets)
if err != nil || !addr.Is6() {
return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
}
}
// Host names do not depend on letter case, so the allowlist, the upstream
@@ -152,6 +171,14 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
// 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
if qIndex := strings.Index(path, "?"); qIndex != -1 {
query = path[qIndex+1:]