Fixes #246. With cdn.example.com allowlisted, CDN.Example.com and cdn.example.com:8443 passed the allowlist too. Each spelling was fetched and cached on its own and had its own connections under upstream_connections_per_host, and a port let an unsigned URL reach any port of an allowlisted host.
The image URL parser lower-cases the host, so the allowlist, the fetcher, the per-host connection limit and the cache keys see one spelling. A host holding @, which a URL reads as a user name and password, is refused with 400.
A host followed by :, with or without a port, is never treated as allowlisted, so its URL needs a signature. A signed URL may still name a port.
What the diff does not show:
A URL is now signed over the lower-cased host, so a signature made over CDN.Example.com is refused with 401. README.md says so under Signature Specification.
The port is read as Go's URL parser reads it, as the fetcher does: a host ending in : has an empty port, and in an IPv6 address written without brackets the port follows the last :. So with 2001:db8::1 allowlisted, 2001:db8::1:8443 and 2001:db8::1: need a signature.
Encrypted URLs are unchanged; their host comes from the token.
Disclosures:
Deviation from the plan: the port check is in imgcache.Service.ValidateRequest, not allowlist.IsAllowed, which also matches referer_blocklist, where a Referer with a port must still be blocked. internal/allowlist is unchanged.
Judgement call: the @ refusal uses a new imgcache.ErrInvalidHost ("invalid source host"), next to ErrMissingHost.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/pixa/issues/246. With `cdn.example.com` allowlisted, `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too. Each spelling was fetched and cached on its own and had its own connections under `upstream_connections_per_host`, and a port let an unsigned URL reach any port of an allowlisted host.
- The image URL parser lower-cases the host, so the allowlist, the fetcher, the per-host connection limit and the cache keys see one spelling. A host holding `@`, which a URL reads as a user name and password, is refused with 400.
- A host followed by `:`, with or without a port, is never treated as allowlisted, so its URL needs a signature. A signed URL may still name a port.
What the diff does not show:
- A URL is now signed over the lower-cased host, so a signature made over `CDN.Example.com` is refused with 401. `README.md` says so under Signature Specification.
- The port is read as Go's URL parser reads it, as the fetcher does: a host ending in `:` has an empty port, and in an IPv6 address written without brackets the port follows the last `:`. So with `2001:db8::1` allowlisted, `2001:db8::1:8443` and `2001:db8::1:` need a signature.
- Encrypted URLs are unchanged; their host comes from the token.
Disclosures:
- Deviation from the plan: the port check is in `imgcache.Service.ValidateRequest`, not `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a `Referer` with a port must still be blocked. `internal/allowlist` is unchanged.
- Judgement call: the `@` refusal uses a new `imgcache.ErrInvalidHost` ("invalid source host"), next to `ErrMissingHost`.
Model: opus-5-5
internal/imgcache/service.go, ValidateRequest: the port check uses net.SplitHostPort, which fails on an IPv6 address written without brackets, so such a host never counts as having a port. Go's URL parser, at the go 1.25.4 in go.mod, reads 2001:db8::1:8443 as host 2001:db8::1 with port 8443. With 2001:db8::1 allowlisted, an unsigned /v1/image/2001:db8::1:8443/... is therefore served, and pixa connects to port 8443. 2001:db8::1: and 2001:db8::1:443 are further unsigned spellings, each with its own cache keys and its own count against upstream_connections_per_host. Until this is fixed, the new README.md paragraph, the TODO.md entry and the code comment saying a host with a port or a : needs a signature are untrue. Acceptable: decide whether there is a port the way the URL parser and the fetcher do (for example parsedURL.Port() is set, or the host ends in :), or refuse with 400 in parseImageComponents a host with more than one : outside brackets. Either way, add a test for 2001:db8::1:8443 and 2001:db8::1: against an allowlisted 2001:db8::1.
Model: opus-5-5
Review failed: `needs-rework`.
- `internal/imgcache/service.go`, `ValidateRequest`: the port check uses `net.SplitHostPort`, which fails on an IPv6 address written without brackets, so such a host never counts as having a port. Go's URL parser, at the `go 1.25.4` in `go.mod`, reads `2001:db8::1:8443` as host `2001:db8::1` with port `8443`. With `2001:db8::1` allowlisted, an unsigned `/v1/image/2001:db8::1:8443/...` is therefore served, and pixa connects to port 8443. `2001:db8::1:` and `2001:db8::1:443` are further unsigned spellings, each with its own cache keys and its own count against `upstream_connections_per_host`. Until this is fixed, the new `README.md` paragraph, the `TODO.md` entry and the code comment saying a host with a port or a `:` needs a signature are untrue. Acceptable: decide whether there is a port the way the URL parser and the fetcher do (for example `parsedURL.Port()` is set, or the host ends in `:`), or refuse with 400 in `parseImageComponents` a host with more than one `:` outside brackets. Either way, add a test for `2001:db8::1:8443` and `2001:db8::1:` against an allowlisted `2001:db8::1`.
Model: opus-5-5
Decision for the finding, taking the reviewer's first way: decide whether the host has a port the way Go's URL parser and the fetcher do (the parsed source URL's Port() is set, or the host ends in :), not with net.SplitHostPort. Add the tests the reviewer names, for 2001:db8::1:8443 and 2001:db8::1: against an allowlisted 2001:db8::1, and keep the README paragraph, the TODO.md entry and the code comment true.
Model: opus-5-5
Decision for the finding, taking the reviewer's first way: decide whether the host has a port the way Go's URL parser and the fetcher do (the parsed source URL's `Port()` is set, or the host ends in `:`), not with `net.SplitHostPort`. Add the tests the reviewer names, for `2001:db8::1:8443` and `2001:db8::1:` against an allowlisted `2001:db8::1`, and keep the README paragraph, the `TODO.md` entry and the code comment true.
Model: opus-5-5
Port check: ValidateRequest now takes a host to have a port when the parsed source URL's Port() is set or the host ends in :, as the fetcher reads it, in place of net.SplitHostPort. The new TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature covers 2001:db8::1:8443 and 2001:db8::1: against an allowlisted 2001:db8::1. The README.md paragraph and the TODO.md entry are true of the fix as written and are unchanged; the code comment and the PR body now name the unbracketed IPv6 case.
Model: opus-5-5
- Port check: `ValidateRequest` now takes a host to have a port when the parsed source URL's `Port()` is set or the host ends in `:`, as the fetcher reads it, in place of `net.SplitHostPort`. The new `TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature` covers `2001:db8::1:8443` and `2001:db8::1:` against an allowlisted `2001:db8::1`. The `README.md` paragraph and the `TODO.md` entry are true of the fix as written and are unchanged; the code comment and the PR body now name the unbracketed IPv6 case.
Model: opus-5-5
The image URL parser must lower-case the host and refuse one holding
"@", which a URL reads as a user name and password before the host;
ErrInvalidHost is added for that refusal. A host with a port, or a ":"
with no port, must match no allowlist entry, so its request needs a
signature. All three tests fail until the fix.
Model: opus-5-5
With cdn.example.com allowlisted, CDN.Example.com and
cdn.example.com:8443 passed the allowlist too, and each spelling had its
own cache entries and its own connections under
upstream_connections_per_host. The image URL parser now lower-cases the
host and refuses one holding "@" with ErrInvalidHost (400).
ValidateRequest counts a host followed by ":", with or without a port,
as not allowlisted, so it needs a signature. That check is not in
allowlist.IsAllowed, which also matches referer_blocklist. A URL is now
signed over the lower-cased host; README.md says so.
Model: opus-5-5
With 2001:db8::1 allowlisted, Go's URL parser, which the fetcher uses,
reads 2001:db8::1:8443 as 2001:db8::1 with port 8443, and
2001:db8::1: as 2001:db8::1 with an empty port. Both must need a
signature. The test fails until the port check reads the port as the
parsed URL does.
Model: opus-5-5
net.SplitHostPort fails on an IPv6 address written without brackets,
so with 2001:db8::1 allowlisted, 2001:db8::1:8443 and 2001:db8::1:
counted as having no port and were served without a signature, though
the fetcher connects to port 8443. ValidateRequest now takes a host to
have a port when the parsed URL's Port() is set or the host ends in
":", as Go's URL parser and so the fetcher read it.
Model: opus-5-5
Rebased onto next after #269. In Signature Specification the host line is this PR's (lower-cased, followed by the port when the URL has one) and the path and query lines are #269's (% written as %25, : as %3A). This PR's TODO.md entry is back at the top of Completed Steps, where the union merge had put it below three newer entries. Nothing else changed.
Model: opus-5-5
Rebased onto `next` after https://git.eeqj.de/sneak/pixa/pulls/269. In Signature Specification the `host` line is this PR's (lower-cased, followed by the port when the URL has one) and the `path` and `query` lines are https://git.eeqj.de/sneak/pixa/pulls/269's (`%` written as `%25`, `:` as `%3A`). This PR's `TODO.md` entry is back at the top of Completed Steps, where the union merge had put it below three newer entries. Nothing else changed.
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 #246. With
cdn.example.comallowlisted,CDN.Example.comandcdn.example.com:8443passed the allowlist too. Each spelling was fetched and cached on its own and had its own connections underupstream_connections_per_host, and a port let an unsigned URL reach any port of an allowlisted host.@, which a URL reads as a user name and password, is refused with 400.:, with or without a port, is never treated as allowlisted, so its URL needs a signature. A signed URL may still name a port.What the diff does not show:
CDN.Example.comis refused with 401.README.mdsays so under Signature Specification.:has an empty port, and in an IPv6 address written without brackets the port follows the last:. So with2001:db8::1allowlisted,2001:db8::1:8443and2001:db8::1:need a signature.Disclosures:
imgcache.Service.ValidateRequest, notallowlist.IsAllowed, which also matchesreferer_blocklist, where aRefererwith a port must still be blocked.internal/allowlistis unchanged.@refusal uses a newimgcache.ErrInvalidHost("invalid source host"), next toErrMissingHost.Model: opus-5-5
Review failed:
needs-rework.internal/imgcache/service.go,ValidateRequest: the port check usesnet.SplitHostPort, which fails on an IPv6 address written without brackets, so such a host never counts as having a port. Go's URL parser, at thego 1.25.4ingo.mod, reads2001:db8::1:8443as host2001:db8::1with port8443. With2001:db8::1allowlisted, an unsigned/v1/image/2001:db8::1:8443/...is therefore served, and pixa connects to port 8443.2001:db8::1:and2001:db8::1:443are further unsigned spellings, each with its own cache keys and its own count againstupstream_connections_per_host. Until this is fixed, the newREADME.mdparagraph, theTODO.mdentry and the code comment saying a host with a port or a:needs a signature are untrue. Acceptable: decide whether there is a port the way the URL parser and the fetcher do (for exampleparsedURL.Port()is set, or the host ends in:), or refuse with 400 inparseImageComponentsa host with more than one:outside brackets. Either way, add a test for2001:db8::1:8443and2001:db8::1:against an allowlisted2001:db8::1.Model: opus-5-5
Decision for the finding, taking the reviewer's first way: decide whether the host has a port the way Go's URL parser and the fetcher do (the parsed source URL's
Port()is set, or the host ends in:), not withnet.SplitHostPort. Add the tests the reviewer names, for2001:db8::1:8443and2001:db8::1:against an allowlisted2001:db8::1, and keep the README paragraph, theTODO.mdentry and the code comment true.Model: opus-5-5
bcf216b61bto95cf7c9430ValidateRequestnow takes a host to have a port when the parsed source URL'sPort()is set or the host ends in:, as the fetcher reads it, in place ofnet.SplitHostPort. The newTestService_ValidateRequest_UnbracketedIPv6PortNeedsSignaturecovers2001:db8::1:8443and2001:db8::1:against an allowlisted2001:db8::1. TheREADME.mdparagraph and theTODO.mdentry are true of the fix as written and are unchanged; the code comment and the PR body now name the unbracketed IPv6 case.Model: opus-5-5
Review passed.
Model: opus-5-5
74f62ddac9to589318db97Rebased onto
nextafter #269. In Signature Specification thehostline is this PR's (lower-cased, followed by the port when the URL has one) and thepathandquerylines are #269's (%written as%25,:as%3A). This PR'sTODO.mdentry is back at the top of Completed Steps, where the union merge had put it below three newer entries. Nothing else changed.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.