7 Commits
Author SHA1 Message Date
sneak fb78b881a6 Keep the empty host when the db_url path starts with //
check / check (push) Canceled after 0s
defaultDBURL kept the empty host only when state_dir started with //,
but SQLite reads the path written into the URL, state_dir followed by
/state.sqlite3. For a state_dir of / that path is //state.sqlite3, the
URL started file:// and SQLite read state.sqlite3 as a host, so pixa
failed to start. The decision now looks at that path: a state_dir of
/ gives file:////state.sqlite3. Every other state_dir gives the same
URL as before.

Model: opus-5-5
2026-10-16 01:46:35 +00:00
sneak 58566b78cc Test the db_url derived from a state_dir of /
The path written into the URL is then //state.sqlite3, so the URL
starts file:// and SQLite reads state.sqlite3 as a host. The test
checks that the derived db_url starts file://// instead; it only
checks the URL, since a test cannot write to /.

Model: opus-5-5
2026-10-16 01:45:30 +00:00
sneak 51528c92b4 Keep the empty host for a state_dir starting with //
With OmitHost set, a state_dir starting with // gave a db_url starting
file://, and SQLite read the first directory name as a host, so pixa
failed to start. OmitHost is now left off for such a directory: the URL
starts file://// and SQLite reads an empty host followed by the path.
Every other state_dir gives the same URL as before.

Model: opus-5-5
2026-10-16 01:42:43 +00:00
sneak 1911489815 Test the db_url derived from a state_dir starting with //
The test sets state_dir to a temporary directory with a / put in front,
opens the derived db_url with the SQLite driver, and checks that the
database is state.sqlite3 in that directory, in WAL mode. It fails while
the URL starts file://, where SQLite reads the first directory name as
a host. The open-and-check part of the test before it moves into a
helper both tests call.

Model: opus-5-5
2026-10-16 01:42:43 +00:00
sneak 8c9623f56c Escape state_dir in the db_url derived from it (closes #290)
The derived db_url put state_dir into a file: URL as it was, so SQLite
decoded a %HH in the name and ended the path at a ? or #, and the
journal mode parameter was lost. It is now built with net/url, which
percent-encodes the directory name; SQLite decodes it back and opens
state.sqlite3 in that directory. OmitHost keeps the usual form
file:/var/lib/pixa/state.sqlite3 and a relative state_dir a path.

Model: opus-5-5
2026-10-16 01:42:43 +00:00
sneak 3a06283a94 Test the db_url derived from a state_dir holding %, ? and #
The test sets state_dir to a directory named a%41b?c#d, opens the
derived db_url with the SQLite driver, and checks that the database is
state.sqlite3 in that directory, in WAL mode. It fails until the
directory is escaped in the URL.

Model: opus-5-5
2026-10-16 01:42:43 +00:00
clawbot 4a4bc9758f Fetch the source path and query URL-encoded, not joined as text (closes #288)
check / check (push) Canceled after 0s
The source URL was built by joining the decoded path and query as
text, so a space in the query reached the upstream raw and failed with
502, 100%25.png made a URL Go could not parse and answered 401, and
%20 and %2520 fetched the same path under different cache keys. The
source URL is now built with net/url, so each decoded path and query is
one upstream request matching its signature and cache key; the query
encodes the characters a browser encodes, ' included. The generator page
compares what pixa would fetch with the URL as entered, and answers 400
with the URL to use when they differ, or for a user name and password
or a .. segment.

Model: opus-5-5
2026-10-16 03:31:15 +02:00
8 changed files with 641 additions and 23 deletions

No files matched your search

+36 -11
View File
@@ -331,6 +331,20 @@ writes one as `%23` is refused with 400. For example,
`/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`,
signed as shown under Signature Specification. signed as shown under Signature Specification.
The source path is the path with its percent-escapes decoded, and pixa fetches
it percent-encoded again: every character other than letters, digits, `-`, `.`,
`_`, `~`, `/`, `$`, `&`, `+`, `,`, `:`, `;`, `=` and `@` is sent as a
percent-escape. So `my%20cat.jpg` in the image URL is the source path
`/my cat.jpg`, fetched as `/my%20cat.jpg`; `my%2520cat.jpg` is the source path
`/my%20cat.jpg`, fetched as `/my%2520cat.jpg`; and `100%25.png` is fetched as
`/100%25.png`. The source query is the source URL's query as that URL writes it,
its percent-escapes included, and is sent as it is, except that the characters a
browser percent-encodes in a query (a space, `"`, `'`, `<`, `>`, control
characters and characters outside ASCII) and a `%` that does not begin a
percent-escape are percent-encoded. pixa signs and caches the query in that
form, so `img%3Fname=my%20cat` and `img%3Fname=my%2520cat` in the image URL are
the same source query, `name=my%20cat`.
A size with one side `0` has that side scaled to keep the source's proportions, A size with one side `0` has that side scaled to keep the source's proportions,
and is refused with 400 when that side would come out above 8192, the most and is refused with 400 when that side would come out above 8192, the most
either side may be; so is a size with `fit=outside`, which scales the image to either side may be; so is a size with `fit=outside`, which scales the image to
@@ -445,7 +459,13 @@ nor change what it asks for.
sent with an empty format, or none, also makes a JPEG XL URL. Width and sent with an empty format, or none, also makes a JPEG XL URL. Width and
height both empty or `0` keep the original size; if only one of them is empty height both empty or `0` keep the original size; if only one of them is empty
or `0`, that side is scaled to keep the image's proportions. A URL whose or `0`, that side is scaled to keep the image's proportions. A URL whose
scaled side would come out above 8192 answers 400. scaled side would come out above 8192 answers 400, and so does a source URL
whose path holds a `..` segment. So does a source URL that the image URL
would fetch written another way, and the page then shows the URL pixa would
fetch: one with a user name and password, which pixa does not send, or one
whose path holds `%2F`, fetched as `/`, or `(`, fetched as `%28`, as pixa
keeps the source path decoded and fetches it percent-encoded again (see
Routes).
3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when 3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
it expires. `<host>` is the host the page was opened on, and the URL starts it expires. `<host>` is the host the page was opened on, and the URL starts
with `http` instead while `debug` is on. The name after the token is ignored with `http` instead while `debug` is on. The name after the token is ignored
@@ -507,11 +527,13 @@ Where:
- `host` — source host as the URL writes it up to the first `/`, in lower case, - `host` — source host as the URL writes it up to the first `/`, in lower case,
keeping an IPv6 address's brackets and any `:` and port after it (e.g. keeping an IPv6 address's brackets and any `:` and port after it (e.g.
`cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`) `cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25` - `path` — source path, with its percent-escapes decoded as under Routes (e.g.
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg` `/photos/cat.jpg`, or `/photos/my cat.jpg` for `/photos/my%20cat.jpg`), with
- `query` — source query string, empty string if none, with `%` and `:` written each `%` written as `%25` and each `:` as `%3A`, so `/photos/a:b.jpg` is
as in `path`; neither then holds a `:`, so path `/a:b` with no query and path signed as `/photos/a%3Ab.jpg`
`/a` with query `b:` are signed differently - `query` — source query string as under Routes, empty string if none, with `%`
and `:` written as in `path`; neither then holds a `:`, so path `/a:b` with no
query and path `/a` with query `b:` are signed differently
- `width` — requested width in pixels, `0` for original - `width` — requested width in pixels, `0` for original
- `height` — requested height in pixels, `0` for original - `height` — requested height in pixels, `0` for original
- `format` — output format, one of those listed under Routes, with `original` - `format` — output format, one of those listed under Routes, with `original`
@@ -693,11 +715,14 @@ Key settings in more detail:
- `signing_key` — HMAC secret for URL signatures - `signing_key` — HMAC secret for URL signatures
- `db_url` — the SQLite database to open; omitted, it is - `db_url` — the SQLite database to open; omitted, it is
`file:<state_dir>/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the `file:<state_dir>/state.sqlite3?_pragma=journal_mode(WAL)`, which keeps the
database in WAL mode. pixa opens one connection to it, so its own reads and database in WAL mode. `state_dir` is percent-encoded in it, so a `%`, `?` or
writes run one at a time. pixa adds `_pragma=busy_timeout(5000)` to any `#` in the name is written as `%25`, `%3F` or `%23`. For a `state_dir` of `/`
`db_url`, so a write that finds another program writing to the file waits up or one starting with `//` it starts `file:////`, so SQLite does not read the
to five seconds for it instead of failing. WAL mode comes only from the URL: first name in the path as a host. pixa opens one connection to it, so its own
keep `_pragma=journal_mode(WAL)` in one you set reads and writes run one at a time. pixa adds `_pragma=busy_timeout(5000)` to
any `db_url`, so a write that finds another program writing to the file waits
up to five seconds for it instead of failing. WAL mode comes only from the
URL: keep `_pragma=journal_mode(WAL)` in one you set
- `cache_max_bytes` — disk cache size limit in bytes; `0` disables the disk - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the disk
cache entirely; omitted defaults to 75% of the sum of the free space on the cache entirely; omitted defaults to 75% of the sum of the free space on the
filesystem containing `<state_dir>/cache/` and the bytes of source and filesystem containing `<state_dir>/cache/` and the bytes of source and
+21
View File
@@ -30,6 +30,27 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-16 a `state_dir` whose name holds `%`, `?` or `#` keeps its database
at `state.sqlite3` in that directory (closes #290): the `db_url` derived from
it put the name into a `file:` URL as it was, so SQLite read `%41` as `A`, a
`?` or `#` ended the path early, and the journal mode parameter was lost. The
derived `db_url` is now built with Go's `net/url`, which writes those
characters as `%25`, `%3F` and `%23`. For a `state_dir` of `/` or one starting
with `//` it starts `file:////`, so SQLite no longer reads the first name in
the path as a host.
- 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
`/100%25.png` where it answered 401, and `my%20cat.png` and `my%2520cat.png`
fetch different paths where both fetched `/my%20cat.png` under two cache keys.
The source query stays as the source URL writes it; the characters a browser
percent-encodes in a query, such as `'` or a space, which was sent raw and
answered 502, are percent-encoded when the image URL is read, so the query is
signed and cached as it is sent. The generator page answers 400, showing the
URL pixa would fetch, for a source URL its image URL would fetch written
another way, such as with `%2F` in the path, whatever else the path holds, or
with a user name and password. It also answers 400 for a source path holding a
`..` segment, as `/v1/image/` refuses one.
- 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips - 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips
build a huge image (closes #282): libvips' thumbnail scaled the whole source build a huge image (closes #282): libvips' thumbnail scaled the whole source
to cover the requested size and copied that into memory before it cut out the to cover the requested size and copied that into memory before it cut out the
+25 -2
View File
@@ -338,8 +338,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
settingName(keyDBURL), errValueEmpty) settingName(keyDBURL), errValueEmpty)
} }
// The driver sets the journal mode only through a _pragma parameter. c.DBURL = defaultDBURL(c.StateDir)
c.DBURL = fmt.Sprintf("file:%s/state.sqlite3?_pragma=journal_mode(WAL)", c.StateDir)
} }
if loader.err != nil { if loader.err != nil {
@@ -354,6 +353,30 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
return c, nil return c, nil
} }
// defaultDBURL returns the db_url used when db_url is omitted: the file
// state.sqlite3 in stateDir, in WAL mode, which the driver sets only
// through a _pragma parameter. String() percent-encodes any %, ? or # in
// the directory name, which SQLite decodes back, so the URL names exactly
// this file. OmitHost writes file:/var/lib/pixa/... instead of
// file:///var/lib/pixa/..., and keeps a relative stateDir from being read
// as a host name. The path written into the URL is stateDir followed by
// /state.sqlite3; when it starts with //, as it does for a stateDir of /
// or one starting with //, the empty host is kept, so the URL starts
// file://// and SQLite does not read the first name in the path as a
// host.
func defaultDBURL(stateDir string) string {
dbPath := stateDir + "/state.sqlite3"
dbURL := url.URL{
Scheme: "file",
OmitHost: !strings.HasPrefix(dbPath, "//"),
Path: dbPath,
RawQuery: "_pragma=journal_mode(WAL)",
}
return dbURL.String()
}
// validateKnownKeys rejects configuration files containing keys the // validateKnownKeys rejects configuration files containing keys the
// application does not understand, so typos fail at startup instead of // application does not understand, so typos fail at startup instead of
// being silently ignored, and rejects keys that are explicitly set to // being silently ignored, and rejects keys that are explicitly set to
@@ -135,6 +135,104 @@ func TestDefaultDBURLOpensTheDatabaseInWALMode(t *testing.T) {
} }
} }
// TestDerivedDBURLOpensStateDirHoldingURLCharacters opens the db_url
// derived from a state_dir whose name holds %41, ? and #, and checks that
// the database is state.sqlite3 in that directory, in WAL mode. Put into
// a file: URL as it is, the name would lose its meaning: %41 is read as
// A, and the path ends at the ?, which also loses the journal mode.
func TestDerivedDBURLOpensStateDirHoldingURLCharacters(t *testing.T) {
t.Parallel()
stateDir := filepath.Join(t.TempDir(), "a%41b?c#d")
err := os.Mkdir(stateDir, 0o750)
if err != nil {
t.Fatalf("failed to create %q: %v", stateDir, err)
}
// Quoted so YAML takes the whole name as it is.
c, err := configFromYAML(t, signingKeyLine+"state_dir: '"+stateDir+"'\n")
if err != nil {
t.Fatalf("config with state_dir %q should be valid, got: %v", stateDir, err)
}
t.Logf("state_dir %q gives db_url %q", stateDir, c.DBURL)
checkDBURLOpensFileInWALMode(t, c.DBURL, filepath.Join(stateDir, "state.sqlite3"))
}
// TestDerivedDBURLOpensStateDirStartingWithTwoSlashes opens the db_url
// derived from a state_dir that starts with //, and checks that the
// database is state.sqlite3 in that directory, in WAL mode. Written right
// after file:, such a name gives a URL starting file://, and SQLite reads
// the first directory name as a host and refuses to open it.
func TestDerivedDBURLOpensStateDirStartingWithTwoSlashes(t *testing.T) {
t.Parallel()
dir := t.TempDir()
stateDir := "/" + dir
c, err := configFromYAML(t, signingKeyLine+"state_dir: "+stateDir+"\n")
if err != nil {
t.Fatalf("config with state_dir %q should be valid, got: %v", stateDir, err)
}
t.Logf("state_dir %q gives db_url %q", stateDir, c.DBURL)
checkDBURLOpensFileInWALMode(t, c.DBURL, filepath.Join(dir, "state.sqlite3"))
}
// TestDerivedDBURLForRootStateDir checks the db_url derived from a
// state_dir of /. The path written into the URL is then
// //state.sqlite3, and written right after file: it gives a URL starting
// file://, where SQLite reads state.sqlite3 as a host and refuses to open
// it. The test only checks the URL, since it cannot write to /.
func TestDerivedDBURLForRootStateDir(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine+"state_dir: /\n")
if err != nil {
t.Fatalf("config with state_dir / should be valid, got: %v", err)
}
t.Logf("state_dir / gives db_url %q", c.DBURL)
wantDBURL := "file:////state.sqlite3?_pragma=journal_mode(WAL)"
if c.DBURL != wantDBURL {
t.Errorf("DBURL = %q, want %q", c.DBURL, wantDBURL)
}
}
// checkDBURLOpensFileInWALMode opens dbURL with the SQLite driver pixad
// uses, and checks that the database is in WAL mode and is the file
// dbFile.
func checkDBURLOpensFileInWALMode(t *testing.T, dbURL, dbFile string) {
t.Helper()
db, err := sql.Open("sqlite", dbURL)
if err != nil {
t.Fatalf("failed to open %q: %v", dbURL, err)
}
t.Cleanup(func() { _ = db.Close() })
var journalMode string
err = db.QueryRowContext(t.Context(), "PRAGMA journal_mode").Scan(&journalMode)
if err != nil {
t.Fatalf("failed to read the journal mode of %q: %v", dbURL, err)
}
if journalMode != "wal" {
t.Errorf("journal mode of %q = %q, want wal", dbURL, journalMode)
}
_, err = os.Stat(dbFile)
if err != nil {
t.Errorf("opening %q did not create %q: %v", dbURL, dbFile, err)
}
}
func TestExplicitValidValuesAreUsed(t *testing.T) { func TestExplicitValidValuesAreUsed(t *testing.T) {
t.Parallel() t.Parallel()
+48
View File
@@ -9,7 +9,9 @@ import (
"math" "math"
"net/http" "net/http"
"net/url" "net/url"
"slices"
"strconv" "strconv"
"strings"
"time" "time"
"sneak.berlin/go/pixa/internal/clientip" "sneak.berlin/go/pixa/internal/clientip"
@@ -133,6 +135,16 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
return return
} }
// /v1/image/ refuses a ".." segment in the source path, and so does
// the generator. The path is decoded, so %2e%2e counts too
if slices.Contains(strings.Split(parsed.Path, "/"), "..") {
s.renderGeneratorWithForm(w, r,
"Source URL path cannot hold a .. segment", r.Form,
http.StatusBadRequest)
return
}
payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form) payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form)
if err != nil { if err != nil {
s.renderGeneratorWithForm(w, r, err.Error(), r.Form, s.renderGeneratorWithForm(w, r, err.Error(), r.Form,
@@ -141,6 +153,19 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
return return
} }
// The image URL must fetch the source URL as it was written
sourceRequest := payload.ToImageRequest()
sourceRequest.AllowHTTP = s.config.AllowHTTP
fetchURL := sourceRequest.SourceURL()
if !fetchedAsWritten(parsed, fetchURL) {
s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+
"written: pixa would fetch "+fetchURL,
r.Form, http.StatusBadRequest)
return
}
// Generate encrypted token // Generate encrypted token
token, err := s.encGen.Generate(payload) token, err := s.encGen.Generate(payload)
if err != nil { if err != nil {
@@ -173,6 +198,29 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
} }
} }
// 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
// holds a user name and password, which the token does not keep either, or a
// "?" with no query after it. The token keeps the source path decoded and
// pixa fetches it percent-encoded again, so a path written another way, such
// as with %2F, is fetched as another path. url.URL keeps the path as written
// in RawPath when it is not the decoded path percent-encoded again.
// EscapedPath ignores RawPath when it also holds a character net/url encodes,
// such as a space, so RawPath is compared itself.
func fetchedAsWritten(source *url.URL, fetchURL string) bool {
writtenPath := source.RawPath
if writtenPath == "" {
writtenPath = source.EscapedPath()
}
fetched, err := url.Parse(fetchURL)
return err == nil && source.User == nil && !source.ForceQuery &&
fetched.EscapedPath() == writtenPath &&
fetched.RawQuery == source.RawQuery
}
// buildGeneratePayload parses the numeric form fields and assembles the // buildGeneratePayload parses the numeric form fields and assembles the
// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A // encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A
// non-numeric or out-of-range width, height, quality or ttl, or an // non-numeric or out-of-range width, height, quality or ttl, or an
+16 -8
View File
@@ -5,6 +5,7 @@ import (
"context" "context"
"errors" "errors"
"io" "io"
"net/url"
"time" "time"
) )
@@ -86,9 +87,11 @@ func ValidateImageRequest(req *ImageRequest) error {
type ImageRequest struct { type ImageRequest struct {
// SourceHost is the origin host (e.g., "cdn.example.com") // SourceHost is the origin host (e.g., "cdn.example.com")
SourceHost string SourceHost string
// SourcePath is the path on the origin (e.g., "/photos/cat.jpg") // SourcePath is the path on the origin with its percent-escapes decoded,
// as url.URL's Path holds it (e.g., "/photos/my cat.jpg")
SourcePath string SourcePath string
// SourceQuery is the optional query string for the origin URL // SourceQuery is the optional query string for the origin URL as the URL
// writes it, percent-escapes included, as url.URL's RawQuery holds it
SourceQuery string SourceQuery string
// Size is the requested output dimensions // Size is the requested output dimensions
Size Size Size Size
@@ -107,20 +110,25 @@ type ImageRequest struct {
AllowHTTP bool AllowHTTP bool
} }
// SourceURL returns the full upstream URL to fetch. // SourceURL returns the full upstream URL to fetch: the source path
// Uses http:// scheme when AllowHTTP is true, otherwise https://. // percent-encoded again, as net/url writes a path, so each source path is one
// upstream path, and the source query as it is, with the characters
// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is
// true, otherwise https://.
func (r *ImageRequest) SourceURL() string { func (r *ImageRequest) SourceURL() string {
scheme := "https" scheme := "https"
if r.AllowHTTP { if r.AllowHTTP {
scheme = "http" scheme = "http"
} }
url := scheme + "://" + r.SourceHost + r.SourcePath sourceURL := url.URL{
if r.SourceQuery != "" { Scheme: scheme,
url += "?" + r.SourceQuery Host: r.SourceHost,
Path: r.SourcePath,
RawQuery: escapeQuery(r.SourceQuery),
} }
return url return sourceURL.String()
} }
// ImageResponse represents a processed image ready to serve // ImageResponse represents a processed image ready to serve
+26 -2
View File
@@ -51,6 +51,13 @@ var sizeFormatRegex = regexp.MustCompile(
// percentEscapeRegex matches one percent-escape, such as "%2e" // percentEscapeRegex matches one percent-escape, such as "%2e"
var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`) var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
// queryEscapeRegex matches a percent-escape, or one character that a browser
// percent-encodes in the query of an http or https URL: one outside the
// printable ASCII characters "!" to "~", such as a space, a control character
// or a character outside ASCII, or one of `"#'<>`. A "%" that does not begin a
// percent-escape matches too.
var queryEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}|[^!-~]|["#'<>%]`)
// hostRegex matches the forms the host of an image URL may take: a host name // 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 // or an IPv4 address (letters, digits, "-", "_" and "."), or an IPv6 address
// in brackets (hex digits, ":" and "."), either one optionally followed by ":" // in brackets (hex digits, ":" and "."), either one optionally followed by ":"
@@ -179,9 +186,12 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
`%w: the source path or query holds a "#"`, ErrInvalidPath) `%w: the source path or query holds a "#"`, ErrInvalidPath)
} }
// Extract query string if present in path // Extract query string if present in path. It is the source URL's query
// as that URL writes it. A character a URL's query cannot hold as it is,
// such as a space written %20 in the image URL, is percent-encoded here,
// so the query is signed and cached as it is sent upstream
if qIndex := strings.Index(path, "?"); qIndex != -1 { if qIndex := strings.Index(path, "?"); qIndex != -1 {
query = path[qIndex+1:] query = escapeQuery(path[qIndex+1:])
path = path[:qIndex] path = path[:qIndex]
} }
@@ -270,6 +280,20 @@ func decodePercentEscapes(s string) string {
}) })
} }
// escapeQuery returns a source query as a URL's query holds it: each
// character that queryEscapeRegex names, such as a space, percent-encoded,
// and the rest, the query's percent-escapes included, kept as they are. The
// result is unchanged by a second escapeQuery.
func escapeQuery(query string) string {
return queryEscapeRegex.ReplaceAllStringFunc(query, func(match string) string {
if percentEscapeRegex.MatchString(match) {
return match
}
return url.PathEscape(match)
})
}
// parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600" // parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600"
func parseSizeFormat(s string) (Size, ImageFormat, error) { func parseSizeFormat(s string) (Size, ImageFormat, error) {
matches := sizeFormatRegex.FindStringSubmatch(s) matches := sizeFormatRegex.FindStringSubmatch(s)
+371
View File
@@ -0,0 +1,371 @@
package server
import (
"net/http"
"net/http/httptest"
"net/url"
"regexp"
"slices"
"strings"
"sync"
"testing"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/session"
)
// percentSignPath is a source path holding a "%", which a URL writes %25.
const percentSignPath = "/100%25.png"
// generateFormPattern extracts the CSRF token rendered into the generator
// page's form.
var generateFormPattern = regexp.MustCompile(
`(?s)action="/generate".*?name="gorilla\.csrf\.Token" value="([^"]+)"`)
// generatedURLPattern extracts the path of the encrypted URL the generator
// page shows.
var generatedURLPattern = regexp.MustCompile(`value="[^"]*(/v1/e/[^"]+)"`)
// recordingUpstream is a test upstream that answers every request with a PNG
// and records the request target, the path and query as sent, of each request
// it receives.
type recordingUpstream struct {
*httptest.Server
mu sync.Mutex
received []string
}
// startRecordingUpstream starts a recordingUpstream, which is closed when the
// test ends.
func startRecordingUpstream(t *testing.T) *recordingUpstream {
t.Helper()
source := encodeTestPNG(t, 64, 48)
upstream := &recordingUpstream{}
upstream.Server = httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
upstream.mu.Lock()
upstream.received = append(upstream.received, r.RequestURI)
upstream.mu.Unlock()
w.Header().Set("Content-Type", "image/png")
_, _ = w.Write(source)
}))
t.Cleanup(upstream.Close)
return upstream
}
// requestTargets returns the request targets the upstream has received, in
// the order it received them.
func (u *recordingUpstream) requestTargets() []string {
u.mu.Lock()
defer u.mu.Unlock()
return slices.Clone(u.received)
}
// pageOrigin is the origin of the login and generator pages below, opened
// over HTTPS; httptest gives every request the host example.com.
const pageOrigin = "https://example.com"
// openRoot sends GET / to s over HTTPS, with cookie when it is not nil, and
// returns the page: the login page, or the generator page for a login session.
func openRoot(
t *testing.T, s *Server, cookie *http.Cookie,
) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
pageOrigin+"/", nil)
if cookie != nil {
req.AddCookie(cookie)
}
page := httptest.NewRecorder()
s.ServeHTTP(page, req)
return page
}
// submitForm posts form to path on s as a browser submits it from page over
// HTTPS: with the CSRF token tokenPattern finds in page, the cookies page set,
// cookie when it is not nil, and a Referer naming the page, which pixa needs
// while debug is off, as it is in startImageProxy's config.
func submitForm(
t *testing.T, s *Server, page *httptest.ResponseRecorder,
tokenPattern *regexp.Regexp, path string, form url.Values,
cookie *http.Cookie,
) *httptest.ResponseRecorder {
t.Helper()
match := tokenPattern.FindStringSubmatch(page.Body.String())
if match == nil {
t.Fatalf("the page has no form for %s with a CSRF token", path)
}
form.Set("gorilla.csrf.Token", match[1])
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost,
pageOrigin+path, strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Referer", pageOrigin+"/")
for _, c := range page.Result().Cookies() {
req.AddCookie(c)
}
if cookie != nil {
req.AddCookie(cookie)
}
rec := httptest.NewRecorder()
s.ServeHTTP(rec, req)
return rec
}
// postGenerate logs in to s with the signing key, submits the generator page's
// form with sourceURL, and returns the response.
func postGenerate(
t *testing.T, s *Server, sourceURL string,
) *httptest.ResponseRecorder {
t.Helper()
login := submitForm(t, s, openRoot(t, s, nil), csrfFieldPattern, "/",
url.Values{"key": {testSigningKey}}, nil)
var sessionCookie *http.Cookie
for _, c := range login.Result().Cookies() {
if c.Name == session.CookieName {
sessionCookie = c
}
}
if sessionCookie == nil {
t.Fatalf("logging in answered %d and set no session cookie", login.Code)
}
rec := submitForm(t, s, openRoot(t, s, sessionCookie), generateFormPattern,
"/generate", url.Values{"url": {sourceURL}}, sessionCookie)
t.Logf("POST /generate with url %s: %d", sourceURL, rec.Code)
return rec
}
// TestImageURLFetchesItsSourcePathAndQuery requests image URLs whose source
// query holds a space, written percent-encoded, or a "'", and whose source
// path holds a "%", written percent-encoded, and checks the one request the
// upstream receives: the source path percent-encoded again, and the source
// query with the space written %20 and the "'" written %27, as a browser
// writes them in a query.
func TestImageURLFetchesItsSourcePathAndQuery(t *testing.T) {
t.Parallel()
tests := []struct {
name string
// pathAndQuery is the source path and query as the image URL writes
// them
pathAndQuery string
// wantRequest is the path and query the upstream must receive
wantRequest string
}{
{"space in the query", "/photo.png%3Fname=my%20cat",
"/photo.png?name=my%20cat"},
{"apostrophe in the query", "/photo.png%3Fname=it's",
"/photo.png?name=it%27s"},
{"percent sign in the path", percentSignPath, percentSignPath},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
upstream := startRecordingUpstream(t)
s, _, _ := startImageProxy(t, upstream.Server)
getImage(t, s, "/v1/image/"+upstreamHost+tt.pathAndQuery+"/orig.png")
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)
}
})
}
}
// TestSourcePathsHoldingSpaceAndPercent20AreFetchedApart requests twice each
// of the source paths /my cat.png and /my%20cat.png, which the image URL
// writes my%20cat.png and my%2520cat.png. Each is fetched once, as a request
// of its own, and its second request is served from the cache.
func TestSourcePathsHoldingSpaceAndPercent20AreFetchedApart(t *testing.T) {
t.Parallel()
upstream := startRecordingUpstream(t)
s, _, _ := startImageProxy(t, upstream.Server)
imageURLs := []string{
"/v1/image/" + upstreamHost + "/my%20cat.png/orig.png",
"/v1/image/" + upstreamHost + "/my%2520cat.png/orig.png",
}
for _, imageURL := range imageURLs {
getImage(t, s, imageURL)
}
for _, imageURL := range imageURLs {
cacheStatus := getImage(t, s, imageURL).Header().Get("X-Pixa-Cache")
if cacheStatus != string(imgcache.CacheHit) {
t.Errorf("second GET %s: X-Pixa-Cache = %q, want HIT",
imageURL, cacheStatus)
}
}
got := upstream.requestTargets()
t.Logf("upstream received %q", got)
want := []string{"/my%20cat.png", "/my%2520cat.png"}
if !slices.Equal(got, want) {
t.Errorf("upstream received %q, want %q", got, want)
}
}
// TestSourceQueryWrittenTwoWaysIsOneUpstreamRequest requests the source query
// name=my%20cat written as the README says, name=my%2520cat, and with the
// space written %20, name=my%20cat. Both are one source query: the upstream
// receives one request, and the second image URL is served from the cache.
func TestSourceQueryWrittenTwoWaysIsOneUpstreamRequest(t *testing.T) {
t.Parallel()
upstream := startRecordingUpstream(t)
s, _, _ := startImageProxy(t, upstream.Server)
getImage(t, s, "/v1/image/"+upstreamHost+"/photo.png%3Fname=my%2520cat/orig.png")
second := getImage(t, s,
"/v1/image/"+upstreamHost+"/photo.png%3Fname=my%20cat/orig.png")
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)
want := []string{"/photo.png?name=my%20cat"}
if !slices.Equal(got, want) {
t.Errorf("upstream received %q, want %q", got, want)
}
}
// TestGeneratedURLFetchesTheSourceURLAsEntered makes an encrypted URL on the
// generator page for source URLs holding %20, %25 and %23 in the path or the
// query, requests it, and checks that the upstream receives the path and
// query exactly as entered.
func TestGeneratedURLFetchesTheSourceURLAsEntered(t *testing.T) {
t.Parallel()
// The names hold no "%": each test's state directory is named after it
// and goes into the database URL, which decodes percent-escapes
tests := []struct {
name string
pathAndQuery string
}{
{"space in the path", "/my%20cat.png"},
{"percent sign in the path", percentSignPath},
{"number sign in the path", "/a%23b.png"},
{"all three in the query", "/photo.png?name=my%20cat%25%23"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
upstream := startRecordingUpstream(t)
s, _, _ := startImageProxy(t, upstream.Server)
rec := postGenerate(t, s, "https://"+upstreamHost+tt.pathAndQuery)
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())
}
getImage(t, s, match[1])
got := upstream.requestTargets()
t.Logf("upstream received %q", got)
if !slices.Equal(got, []string{tt.pathAndQuery}) {
t.Errorf("upstream received %q, want [%q]", got, tt.pathAndQuery)
}
})
}
}
// 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
// percent-encoded again, so a path holding %2F would be fetched with a "/"
// there, whatever else the path holds. pixa sends no user name or password,
// and no "?" with no query after it. For these the page shows the URL pixa
// would fetch. A source path holding a ".." segment is refused, as
// /v1/image/ refuses it.
func TestGeneratorRefusesSourceURLItCannotFetchAsWritten(t *testing.T) {
t.Parallel()
// The test config sets allow_http, so pixa would fetch over plain HTTP
const fetchedFrom = "http://" + upstreamHost
tests := []struct {
name string
// sourceURL is the source URL after "https://"
sourceURL string
// wantShown is what the page must show: the URL pixa would fetch, or
// why it makes no encrypted URL
wantShown string
}{
{"encoded slash", upstreamHost + "/a%2Fb.png",
fetchedFrom + "/a/b.png"},
{"encoded slash and a space", upstreamHost + "/a%2Fb c.png",
fetchedFrom + "/a/b%20c.png"},
{"encoded slash and a bar", upstreamHost + "/a%2Fb|c.png",
fetchedFrom + "/a/b%7Cc.png"},
{"encoded slash and a letter outside ASCII", upstreamHost + "/café%2Fx.png",
fetchedFrom + "/caf%C3%A9/x.png"},
{"user name and password", "user:pass@" + upstreamHost + "/a.png",
fetchedFrom + "/a.png"},
{"question mark with no query", upstreamHost + "/a.png?",
fetchedFrom + "/a.png"},
{"two-dot segment", upstreamHost + "/a/../b.png",
"cannot hold a .. segment"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
s, _, _ := startImageProxy(t, startRecordingUpstream(t).Server)
rec := postGenerate(t, s, "https://"+tt.sourceURL)
body := rec.Body.String()
if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
}
if strings.Contains(body, "/v1/e/") {
t.Error("the page shows an encrypted URL")
}
if !strings.Contains(body, tt.wantShown) {
t.Errorf("the page does not show %s; body %s", tt.wantShown, body)
}
})
}
}