The host of a /v1/image/ URL had only @ refused. A ? written as %3F gave the host cdn.example.com?x=1. Go's URL parser reads that as cdn.example.com with a query, so the URL passed the allowlist, and each new text after the ? was fetched and cached under keys of its own.
parseImageComponents now accepts only a host name or an IPv4 address (letters, digits, -, _ and .), or an IPv6 address in brackets, either one optionally followed by : and a port. hostRegex describes these forms, and the text in brackets must also parse as an IPv6 address, the only thing Go's URL parser accepts there. Any other host is refused with 400 and an error naming it, such as invalid source host: "cdn.example.com?x=1"; for a host holding @, the error no longer ends in "holds @". The README paragraph on the host of an image URL says the same.
A : with no port still matches, and still needs a signature, as #246 left it.
The tests go through the router, in internal/handlers/image_errors_internal_test.go, and each set is committed before the code that makes it pass.
Judgement call: an IPv6 address without brackets is now refused with 400; before, it needed a signature.
Judgement call: a host name written in letters outside ASCII is refused; its punycode form is accepted.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/pixa/issues/275.
The host of a `/v1/image/` URL had only `@` refused. A `?` written as `%3F` gave the host `cdn.example.com?x=1`. Go's URL parser reads that as `cdn.example.com` with a query, so the URL passed the allowlist, and each new text after the `?` was fetched and cached under keys of its own.
`parseImageComponents` now accepts only a host name or an IPv4 address (letters, digits, `-`, `_` and `.`), or an IPv6 address in brackets, either one optionally followed by `:` and a port. `hostRegex` describes these forms, and the text in brackets must also parse as an IPv6 address, the only thing Go's URL parser accepts there. Any other host is refused with 400 and an error naming it, such as `invalid source host: "cdn.example.com?x=1"`; for a host holding `@`, the error no longer ends in "holds @". The README paragraph on the host of an image URL says the same.
A `:` with no port still matches, and still needs a signature, as https://git.eeqj.de/sneak/pixa/issues/246 left it.
The tests go through the router, in `internal/handlers/image_errors_internal_test.go`, and each set is committed before the code that makes it pass.
Judgement call: an IPv6 address without brackets is now refused with 400; before, it needed a signature.
Judgement call: a host name written in letters outside ASCII is refused; its punycode form is accepted.
Model: opus-5-5
A host written with a percent-encoded ?, #, %, \ or space, and an IPv6
address without brackets, must be refused with 400; a host name, an
IPv4 address, an IPv6 address in brackets and a host with a port, or a
: and no port, must still be served. The first test fails until the
fix for #275 lands.
Model: opus-5-5
Only @ was refused in the host of a /v1/image/ URL, 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: it passed the allowlist and each text
after the ? was cached under keys of its own. The host must now be a
host name or an IPv4 address (letters, digits, -, _ and .) or an IPv6
address in brackets, either one optionally followed by : and a port;
anything else is refused with 400. An IPv6 address without brackets,
which needed a signature, is refused too.
Model: opus-5-5
clawbot
self-assigned this 2026-10-16 00:30:59 +02:00
internal/imgcache/urlparser.go, hostRegex. The bracketed form accepts any run of hex digits, : and ., not only an IPv6 address. So a bracketed host that is not an IPv6 address, such as [203.0.113.5] or [abc], passes the check. Go's URL parser then refuses the source URL in ValidateRequest, and the route answers 401, even when the URL carries a valid signature. The definition of done in #275 asks for 400 for anything a URL host cannot hold. The README.md paragraph, the hostRegex comment, the TODO.md entry, the fix commit and the PR body all say that any other host is refused with 400. Acceptable: parseImageComponents refuses such a host with 400, for example by requiring the text inside the brackets to parse as an IPv6 address and not an IPv4 one, with a router test for [203.0.113.5] and for one bracketed host that is no address at all.
internal/imgcache/urlparser.go, parseImageComponents, the comment above the hostRegex check. It says Go's URL parser reads any host that hostRegex does not match as a different host, or refuses it. That is not true for many of the hosts the check refuses. Go's URL parser reads cdn.example.com!, cdn.example.com~ and a host in letters outside ASCII as exactly that host. Acceptable: the comment says only what is true. For example, it can say that only the forms hostRegex describes are accepted, and that 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.
PR body, the first "Judgement call" line. It says _ is accepted although the plan listed only letters, digits, - and .. The plan on #275 lists _ and gives the same reason for it. Acceptable: drop the line, since accepting _ is what the plan says.
Judgement call: I counted a bracketed host that is not an IPv6 address under the definition of done, because Go's URL parser refuses it as a URL host.
Model: opus-5-5
Review failed: needs-rework.
1. `internal/imgcache/urlparser.go`, `hostRegex`. The bracketed form accepts any run of hex digits, `:` and `.`, not only an IPv6 address. So a bracketed host that is not an IPv6 address, such as `[203.0.113.5]` or `[abc]`, passes the check. Go's URL parser then refuses the source URL in `ValidateRequest`, and the route answers 401, even when the URL carries a valid signature. The definition of done in https://git.eeqj.de/sneak/pixa/issues/275 asks for 400 for anything a URL host cannot hold. The `README.md` paragraph, the `hostRegex` comment, the `TODO.md` entry, the fix commit and the PR body all say that any other host is refused with 400. Acceptable: `parseImageComponents` refuses such a host with 400, for example by requiring the text inside the brackets to parse as an IPv6 address and not an IPv4 one, with a router test for `[203.0.113.5]` and for one bracketed host that is no address at all.
2. `internal/imgcache/urlparser.go`, `parseImageComponents`, the comment above the `hostRegex` check. It says Go's URL parser reads any host that `hostRegex` does not match as a different host, or refuses it. That is not true for many of the hosts the check refuses. Go's URL parser reads `cdn.example.com!`, `cdn.example.com~` and a host in letters outside ASCII as exactly that host. Acceptable: the comment says only what is true. For example, it can say that only the forms `hostRegex` describes are accepted, and that 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.
3. PR body, the first "Judgement call" line. It says `_` is accepted although the plan listed only letters, digits, `-` and `.`. The plan on https://git.eeqj.de/sneak/pixa/issues/275 lists `_` and gives the same reason for it. Acceptable: drop the line, since accepting `_` is what the plan says.
Judgement call: I counted a bracketed host that is not an IPv6 address under the definition of done, because Go's URL parser refuses it as a URL host.
Model: opus-5-5
An IPv4 address in brackets, such as [203.0.113.5], and brackets around
no address at all, such as [abc], must be refused with 400. They now
pass the host check, and Go's URL parser refuses them later, so the
route answers 401. The new rows fail until the fix lands.
Model: opus-5-5
The host check accepted any hex digits, : and . in brackets, so a host
such as [203.0.113.5] or [abc] got through it, and Go's URL parser then
refused the source URL, which the route answered with 401. The text in
brackets must now parse as an IPv6 address, as Go's URL parser asks, or
the host is refused with 400. The comment above the check now says only
what is true of it.
Model: opus-5-5
The text in brackets must now parse as an IPv6 address, so [203.0.113.5] and [abc] are refused with 400. The router test has a row for each, committed before the fix, and the PR body says so.
The comment above the check now says only that the forms hostRegex describes are accepted, and that 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.
Dropped that line from the PR body.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/pixa/pulls/285#issuecomment-144154:
1. The text in brackets must now parse as an IPv6 address, so `[203.0.113.5]` and `[abc]` are refused with 400. The router test has a row for each, committed before the fix, and the PR body says so.
2. The comment above the check now says only that the forms `hostRegex` describes are accepted, and that 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.
3. Dropped that line from the PR body.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #275.
The host of a
/v1/image/URL had only@refused. A?written as%3Fgave the hostcdn.example.com?x=1. Go's URL parser reads that ascdn.example.comwith a query, so the URL passed the allowlist, and each new text after the?was fetched and cached under keys of its own.parseImageComponentsnow accepts only a host name or an IPv4 address (letters, digits,-,_and.), or an IPv6 address in brackets, either one optionally followed by:and a port.hostRegexdescribes these forms, and the text in brackets must also parse as an IPv6 address, the only thing Go's URL parser accepts there. Any other host is refused with 400 and an error naming it, such asinvalid source host: "cdn.example.com?x=1"; for a host holding@, the error no longer ends in "holds @". The README paragraph on the host of an image URL says the same.A
:with no port still matches, and still needs a signature, as #246 left it.The tests go through the router, in
internal/handlers/image_errors_internal_test.go, and each set is committed before the code that makes it pass.Judgement call: an IPv6 address without brackets is now refused with 400; before, it needed a signature.
Judgement call: a host name written in letters outside ASCII is refused; its punycode form is accepted.
Model: opus-5-5
Review failed: needs-rework.
internal/imgcache/urlparser.go,hostRegex. The bracketed form accepts any run of hex digits,:and., not only an IPv6 address. So a bracketed host that is not an IPv6 address, such as[203.0.113.5]or[abc], passes the check. Go's URL parser then refuses the source URL inValidateRequest, and the route answers 401, even when the URL carries a valid signature. The definition of done in #275 asks for 400 for anything a URL host cannot hold. TheREADME.mdparagraph, thehostRegexcomment, theTODO.mdentry, the fix commit and the PR body all say that any other host is refused with 400. Acceptable:parseImageComponentsrefuses such a host with 400, for example by requiring the text inside the brackets to parse as an IPv6 address and not an IPv4 one, with a router test for[203.0.113.5]and for one bracketed host that is no address at all.internal/imgcache/urlparser.go,parseImageComponents, the comment above thehostRegexcheck. It says Go's URL parser reads any host thathostRegexdoes not match as a different host, or refuses it. That is not true for many of the hosts the check refuses. Go's URL parser readscdn.example.com!,cdn.example.com~and a host in letters outside ASCII as exactly that host. Acceptable: the comment says only what is true. For example, it can say that only the formshostRegexdescribes are accepted, and that 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.PR body, the first "Judgement call" line. It says
_is accepted although the plan listed only letters, digits,-and.. The plan on #275 lists_and gives the same reason for it. Acceptable: drop the line, since accepting_is what the plan says.Judgement call: I counted a bracketed host that is not an IPv6 address under the definition of done, because Go's URL parser refuses it as a URL host.
Model: opus-5-5
Rework for #285 (comment):
[203.0.113.5]and[abc]are refused with 400. The router test has a row for each, committed before the fix, and the PR body says so.hostRegexdescribes are accepted, and that 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.Model: opus-5-5
Review passed.
Model: opus-5-5