Make one image URL per source on the generator page for host case and an empty path (closes #293)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
The generator page passed the source host and path into the encrypted URL as entered, so a host spelled in another letter case, or a source with no path against the same source with /, made separate cache keys and fetched one upstream image twice. The page now lower-cases the source host, as /v1/image/ does, and writes an empty source path as /, before it builds the encrypted URL. The checks the page gained for #288 still hold. Model: opus-5-5
This commit was merged in pull request #294.
This commit is contained in:
@@ -473,9 +473,11 @@ nor change what it asks for.
|
||||
`jxl` for a form with no format.
|
||||
|
||||
The token holds the source's host, path and query and the size, format, quality,
|
||||
fit and expiry, encrypted with a key derived from `signing_key`. The source
|
||||
URL's scheme is not kept: the image is fetched like any other (see Routes), and
|
||||
the blocked networks still apply.
|
||||
fit and expiry, encrypted with a key derived from `signing_key`. The host is
|
||||
kept in lower case and an empty path as `/`, as for `/v1/image/`, so
|
||||
`https://CDN.Example.com` and `https://cdn.example.com/` are one source image,
|
||||
fetched and cached once. The source URL's scheme is not kept: the image is
|
||||
fetched like any other (see Routes), and the blocked networks still apply.
|
||||
|
||||
How long the URL lasts is chosen on the page, from 1 minute to 1 year, or never.
|
||||
The expiry is fixed in the token when the URL is made and cannot be changed or
|
||||
|
||||
@@ -30,6 +30,11 @@ P2: security: per-origin rate limiting
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-16 the generator page lower-cases the source host and writes an empty
|
||||
source path as `/` before it makes the encrypted URL, as `/v1/image/` does
|
||||
(closes #293). A host written in upper and in lower case, or a source URL with
|
||||
no path and with the path `/`, made two cache keys and two upstream requests
|
||||
for one image.
|
||||
- 2026-10-16 the source URL a request fetches is built with `net/url` from the
|
||||
source host, path and query, not joined as text (closes #288). The source
|
||||
path, decoded, is fetched percent-encoded again, so `100%25.png` fetches
|
||||
|
||||
@@ -127,7 +127,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
||||
sourceURL := r.FormValue("url")
|
||||
|
||||
// Validate source URL
|
||||
parsed, err := url.Parse(sourceURL)
|
||||
parsed, err := parseSourceURL(sourceURL)
|
||||
if err != nil || parsed.Host == "" {
|
||||
s.renderGeneratorWithForm(w, r, "Invalid source URL", r.Form,
|
||||
http.StatusBadRequest)
|
||||
@@ -198,6 +198,25 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// parseSourceURL parses the source URL entered on the generator page with its
|
||||
// host lower-cased and an empty path written as "/", as /v1/image/ reads a
|
||||
// source URL, so that each spelling of one source URL makes one cache key and
|
||||
// one upstream request.
|
||||
func parseSourceURL(sourceURL string) (*url.URL, error) {
|
||||
parsed, err := url.Parse(sourceURL)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
parsed.Host = strings.ToLower(parsed.Host)
|
||||
|
||||
if parsed.Path == "" {
|
||||
parsed.Path = "/"
|
||||
}
|
||||
|
||||
return parsed, nil
|
||||
}
|
||||
|
||||
// fetchedAsWritten reports whether fetchURL, the URL pixa fetches for an
|
||||
// image URL made from the source URL source, is source as it was written,
|
||||
// apart from the scheme, which the token does not keep. It is not when source
|
||||
|
||||
@@ -308,6 +308,70 @@ func TestGeneratedURLFetchesTheSourceURLAsEntered(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestGeneratorMakesOneImageURLForTwoSpellingsOfASourceURL enters two
|
||||
// spellings of one source URL on the generator page, requests the encrypted
|
||||
// URL made for each, and checks that the upstream receives one request and
|
||||
// that the second encrypted URL is served from the cache. One pair writes the
|
||||
// host in upper and in lower case, which /v1/image/ lower-cases. The host is
|
||||
// an IPv6 address, whose hex digits have letter case, because the fetcher
|
||||
// would look a host name up in DNS. It has a port, as the fetcher refuses an
|
||||
// IPv6 address written without one. The other pair is a source URL with no
|
||||
// path and with the path "/", both of which fetch "/".
|
||||
func TestGeneratorMakesOneImageURLForTwoSpellingsOfASourceURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
// sourceURLs are the two spellings, after "https://"
|
||||
sourceURLs []string
|
||||
// wantRequest is the path the upstream must receive
|
||||
wantRequest string
|
||||
}{
|
||||
{"host in upper and in lower case",
|
||||
[]string{"[2001:DB8::A]:8443/a.png", "[2001:db8::a]:8443/a.png"},
|
||||
"/a.png"},
|
||||
{"no path and the path written as a slash",
|
||||
[]string{upstreamHost, upstreamHost + "/"}, "/"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
upstream := startRecordingUpstream(t)
|
||||
s, _, _ := startImageProxy(t, upstream.Server)
|
||||
|
||||
var imageURLs []string
|
||||
|
||||
for _, sourceURL := range tt.sourceURLs {
|
||||
rec := postGenerate(t, s, "https://"+sourceURL)
|
||||
|
||||
match := generatedURLPattern.FindStringSubmatch(rec.Body.String())
|
||||
if rec.Code != http.StatusOK || match == nil {
|
||||
t.Fatalf("POST /generate: %d with no encrypted URL; body %s",
|
||||
rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
imageURLs = append(imageURLs, match[1])
|
||||
}
|
||||
|
||||
getImage(t, s, imageURLs[0])
|
||||
|
||||
second := getImage(t, s, imageURLs[1])
|
||||
if got := second.Header().Get("X-Pixa-Cache"); got != string(imgcache.CacheHit) {
|
||||
t.Errorf("second X-Pixa-Cache = %q, want HIT", got)
|
||||
}
|
||||
|
||||
got := upstream.requestTargets()
|
||||
t.Logf("upstream received %q", got)
|
||||
|
||||
if !slices.Equal(got, []string{tt.wantRequest}) {
|
||||
t.Errorf("upstream received %q, want [%q]", got, tt.wantRequest)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestGeneratorRefusesSourceURLItCannotFetchAsWritten submits source URLs the
|
||||
// generator page must make no encrypted URL for, and checks that it answers
|
||||
// 400 and shows why. pixa keeps the source path decoded and fetches it
|
||||
|
||||
Reference in New Issue
Block a user