On /v1/image/, a q that was not a number or was outside 1-100 was dropped and 85 used, so q=500 was served and verified against a signature made for 85. It is now a 400 naming q and the value, read with the URL generator's quality check (parseFormInt with minQuality and maxQuality). An empty q= is refused too; only a q missing from the URL is 85.
Two more ways a given q was ignored are closed for the whole route:
r.URL.Query() drops a pair it cannot decode, so q=80% arrived as no q. The query string is now read with url.ParseQuery, and one that cannot be decoded is a 400 showing it.
Only a parameter's first value was read, so q=80&q=500 was served at 80. Any parameter given more than once (q, fit, sig, exp alike) is now a 400 naming it.
The source URL's own query travels in the path as %3F, so refusing repeated parameters does not affect a source URL that repeats a key.
README.md states the q range and both query-string rules.
Judgement call: an empty q is reported as invalid q: not a number, got "", as the shared check reports other non-numeric values.
Judgement call: when several parameters are repeated, the error names one of them.
An empty fit= is still taken as missing; that predates this change.
An exp that cannot be parsed is still ignored; left to #72.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/pixa/issues/134.
On `/v1/image/`, a `q` that was not a number or was outside 1-100 was dropped and 85 used, so `q=500` was served and verified against a signature made for 85. It is now a 400 naming `q` and the value, read with the URL generator's quality check (`parseFormInt` with `minQuality` and `maxQuality`). An empty `q=` is refused too; only a `q` missing from the URL is 85.
Two more ways a given `q` was ignored are closed for the whole route:
- `r.URL.Query()` drops a pair it cannot decode, so `q=80%` arrived as no `q`. The query string is now read with `url.ParseQuery`, and one that cannot be decoded is a 400 showing it.
- Only a parameter's first value was read, so `q=80&q=500` was served at 80. Any parameter given more than once (`q`, `fit`, `sig`, `exp` alike) is now a 400 naming it.
The source URL's own query travels in the path as `%3F`, so refusing repeated parameters does not affect a source URL that repeats a key.
`README.md` states the `q` range and both query-string rules.
- Judgement call: an empty `q` is reported as `invalid q: not a number, got ""`, as the shared check reports other non-numeric values.
- Judgement call: when several parameters are repeated, the error names one of them.
- An empty `fit=` is still taken as missing; that predates this change.
- An `exp` that cannot be parsed is still ignored; left to https://git.eeqj.de/sneak/pixa/issues/72.
Model: opus-5-5
internal/handlers/image.go, parseImageRequest: an empty q= is served at 85 and passes a signature made for 85. So does q=&q=500. A q that is in the URL with an empty value is set, not absent. The definition of done in #134 makes any q that is not a whole number from 1 to 100 a 400, and pixa's configuration already counts a present but empty value as set. The empty fit= on this route is older than this change and is no reason to extend the same behaviour to q. The documents also disagree with each other: README.md says any other q is refused with 400, but the TODO.md entry and the code comment say an empty q is 85. Acceptable: q= is a 400 naming q and the empty value, with a route test, and the TODO.md entry and the code comment say so.
internal/handlers/auth.go, the comment on errInvalidFormField still says it reports a generator form field, but it now also reports the q parameter of /v1/image/. Acceptable: the comment names both, as the two comments this change updated just below it already do.
Judgement call: the empty fit= on /v1/image/ has the same gap. It is out of scope here.
Model: opus-5-5
FAIL
1. `internal/handlers/image.go`, `parseImageRequest`: an empty `q=` is served at 85 and passes a signature made for 85. So does `q=&q=500`. A `q` that is in the URL with an empty value is set, not absent. The definition of done in https://git.eeqj.de/sneak/pixa/issues/134 makes any `q` that is not a whole number from 1 to 100 a 400, and pixa's configuration already counts a present but empty value as set. The empty `fit=` on this route is older than this change and is no reason to extend the same behaviour to `q`. The documents also disagree with each other: `README.md` says any other `q` is refused with 400, but the `TODO.md` entry and the code comment say an empty `q` is 85. Acceptable: `q=` is a 400 naming `q` and the empty value, with a route test, and the `TODO.md` entry and the code comment say so.
2. `internal/handlers/auth.go`, the comment on `errInvalidFormField` still says it reports a generator form field, but it now also reports the `q` parameter of `/v1/image/`. Acceptable: the comment names both, as the two comments this change updated just below it already do.
Judgement call: the empty `fit=` on `/v1/image/` has the same gap. It is out of scope here.
Model: opus-5-5
Fixed: an empty q in the URL is a 400 naming q and the empty value; route tests for q= and q=&q=500; the TODO.md entry, the code comment and the PR body say only a missing q is 85.
Fixed: the comment names both uses.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/pixa/pulls/138#issuecomment-103789:
1. Fixed: an empty `q` in the URL is a 400 naming `q` and the empty value; route tests for `q=` and `q=&q=500`; the `TODO.md` entry, the code comment and the PR body say only a missing `q` is 85.
2. Fixed: the comment names both uses.
Model: opus-5-5
internal/handlers/image.go, parseImageRequest: a q whose value cannot be decoded from the query string is dropped, so the image is served at 85 and passes a signature made for 85. Examples: q=80% (quality written as a percentage), q=%zz, q=50;fit=contain. r.URL.Query() silently skips any pair it cannot decode, so the route sees no q at all. This misses the definition of done in #134 and pixa's rule that a set but unparseable value is an error. It also makes README.md ("a request whose q is anything else is refused with 400"), the TODO.md entry and the code comment above the check untrue. Acceptable: such a request is a 400 whose error shows the part that could not be decoded, for example by parsing r.URL.RawQuery with url.ParseQuery and refusing on its error, with a route test for q=80%. Refusing a query string that cannot be decoded on the whole route, not only for q, is fine.
Model: opus-5-5
FAIL
1. `internal/handlers/image.go`, `parseImageRequest`: a `q` whose value cannot be decoded from the query string is dropped, so the image is served at 85 and passes a signature made for 85. Examples: `q=80%` (quality written as a percentage), `q=%zz`, `q=50;fit=contain`. `r.URL.Query()` silently skips any pair it cannot decode, so the route sees no `q` at all. This misses the definition of done in https://git.eeqj.de/sneak/pixa/issues/134 and pixa's rule that a set but unparseable value is an error. It also makes `README.md` ("a request whose `q` is anything else is refused with 400"), the `TODO.md` entry and the code comment above the check untrue. Acceptable: such a request is a 400 whose error shows the part that could not be decoded, for example by parsing `r.URL.RawQuery` with `url.ParseQuery` and refusing on its error, with a route test for `q=80%`. Refusing a query string that cannot be decoded on the whole route, not only for `q`, is fine.
Model: opus-5-5
Fixed: the query string is read with url.ParseQuery, and one that cannot be decoded is a 400 on the whole route whose error shows it; route tests for q=80% and q=50;fit=contain; the TODO.md entry and the PR body say so.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/pixa/pulls/138#issuecomment-103810:
1. Fixed: the query string is read with `url.ParseQuery`, and one that cannot be decoded is a 400 on the whole route whose error shows it; route tests for `q=80%` and `q=50;fit=contain`; the `TODO.md` entry and the PR body say so.
Model: opus-5-5
internal/handlers/image.go, parseImageRequest: only the first q in the query string is checked, and any later q is ignored. q=80&q=500, q=80&q=banana and q=80&q= are served at 80 and pass a signature made for 80, while the same values in the other order (q=500&q=80, q=&q=80) are a 400. A q that is set and invalid is silently ignored. That misses the definition of done in #134 and the repo rule against a silent fallback for a parameter the request gives, and it makes the README.md line ("a request whose q is anything else is refused with 400") and the TODO.md entry untrue. Acceptable: a q given more than once is a 400 naming q, or every q value is checked the same way, with a route test such as q=80&q=500.
The PR body (about 270 words) and the body of the commit "Refuse a q outside 1-100 on /v1/image/ with 400" (about 130 words) are over the limits of about 250 and about 120 words. Acceptable: trim both, for example by dropping the parts that retell what the diff shows.
Model: opus-5-5
FAIL
1. `internal/handlers/image.go`, `parseImageRequest`: only the first `q` in the query string is checked, and any later `q` is ignored. `q=80&q=500`, `q=80&q=banana` and `q=80&q=` are served at 80 and pass a signature made for 80, while the same values in the other order (`q=500&q=80`, `q=&q=80`) are a 400. A `q` that is set and invalid is silently ignored. That misses the definition of done in https://git.eeqj.de/sneak/pixa/issues/134 and the repo rule against a silent fallback for a parameter the request gives, and it makes the `README.md` line ("a request whose `q` is anything else is refused with 400") and the `TODO.md` entry untrue. Acceptable: a `q` given more than once is a 400 naming `q`, or every `q` value is checked the same way, with a route test such as `q=80&q=500`.
2. The PR body (about 270 words) and the body of the commit "Refuse a q outside 1-100 on /v1/image/ with 400" (about 130 words) are over the limits of about 250 and about 120 words. Acceptable: trim both, for example by dropping the parts that retell what the diff shows.
Model: opus-5-5
Route tests: a q that is not a whole number from 1 to 100, an empty one
included, is a 400 naming q and the value; a parameter given more than
once is a 400 naming it; a query string that cannot be decoded is a 400
showing it. On next each is served, at 85 or at the first value given.
Model: opus-5-5
A q that was not a number or was outside 1-100 was dropped and 85 used,
so q=500 was served and verified against a signature made for 85. It is
now a 400 naming q and the value, read with the generator's quality
check; only a q missing from the URL is 85.
The route also refuses with 400 a query string that cannot be decoded
(r.URL.Query() drops such a pair, so q=80% arrived as no q) and any
parameter given more than once, which was read from its first value only
(q=80&q=500 was served at 80).
Model: opus-5-5
Fixed: any query parameter given more than once on /v1/image/ is a 400 naming it; route tests for q=80&q=500, q=80&q= and fit=cover&fit=contain; README.md, TODO.md and the code comment state the rule.
Fixed: the PR body and both commit bodies are trimmed.
Judgement call: the q=&q=500 test case is removed, as it is now one more repeated q, and the linter refuses a third copy of the same expected error.
The test is renamed TestHandleImage_InvalidQuery_Returns400, since it now covers fit too.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/pixa/pulls/138#issuecomment-103825:
1. Fixed: any query parameter given more than once on `/v1/image/` is a 400 naming it; route tests for `q=80&q=500`, `q=80&q=` and `fit=cover&fit=contain`; `README.md`, `TODO.md` and the code comment state the rule.
2. Fixed: the PR body and both commit bodies are trimmed.
- Judgement call: the `q=&q=500` test case is removed, as it is now one more repeated `q`, and the linter refuses a third copy of the same expected error.
- The test is renamed `TestHandleImage_InvalidQuery_Returns400`, since it now covers `fit` too.
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.
Closes #134.
On
/v1/image/, aqthat was not a number or was outside 1-100 was dropped and 85 used, soq=500was served and verified against a signature made for 85. It is now a 400 namingqand the value, read with the URL generator's quality check (parseFormIntwithminQualityandmaxQuality). An emptyq=is refused too; only aqmissing from the URL is 85.Two more ways a given
qwas ignored are closed for the whole route:r.URL.Query()drops a pair it cannot decode, soq=80%arrived as noq. The query string is now read withurl.ParseQuery, and one that cannot be decoded is a 400 showing it.q=80&q=500was served at 80. Any parameter given more than once (q,fit,sig,expalike) is now a 400 naming it.The source URL's own query travels in the path as
%3F, so refusing repeated parameters does not affect a source URL that repeats a key.README.mdstates theqrange and both query-string rules.qis reported asinvalid q: not a number, got "", as the shared check reports other non-numeric values.fit=is still taken as missing; that predates this change.expthat cannot be parsed is still ignored; left to #72.Model: opus-5-5
14bde63d0dtoa0c6412587FAIL
internal/handlers/image.go,parseImageRequest: an emptyq=is served at 85 and passes a signature made for 85. So doesq=&q=500. Aqthat is in the URL with an empty value is set, not absent. The definition of done in #134 makes anyqthat is not a whole number from 1 to 100 a 400, and pixa's configuration already counts a present but empty value as set. The emptyfit=on this route is older than this change and is no reason to extend the same behaviour toq. The documents also disagree with each other:README.mdsays any otherqis refused with 400, but theTODO.mdentry and the code comment say an emptyqis 85. Acceptable:q=is a 400 namingqand the empty value, with a route test, and theTODO.mdentry and the code comment say so.internal/handlers/auth.go, the comment onerrInvalidFormFieldstill says it reports a generator form field, but it now also reports theqparameter of/v1/image/. Acceptable: the comment names both, as the two comments this change updated just below it already do.Judgement call: the empty
fit=on/v1/image/has the same gap. It is out of scope here.Model: opus-5-5
a0c6412587to98d23ad32eRework of #138 (comment):
qin the URL is a 400 namingqand the empty value; route tests forq=andq=&q=500; theTODO.mdentry, the code comment and the PR body say only a missingqis 85.Model: opus-5-5
FAIL
internal/handlers/image.go,parseImageRequest: aqwhose value cannot be decoded from the query string is dropped, so the image is served at 85 and passes a signature made for 85. Examples:q=80%(quality written as a percentage),q=%zz,q=50;fit=contain.r.URL.Query()silently skips any pair it cannot decode, so the route sees noqat all. This misses the definition of done in #134 and pixa's rule that a set but unparseable value is an error. It also makesREADME.md("a request whoseqis anything else is refused with 400"), theTODO.mdentry and the code comment above the check untrue. Acceptable: such a request is a 400 whose error shows the part that could not be decoded, for example by parsingr.URL.RawQuerywithurl.ParseQueryand refusing on its error, with a route test forq=80%. Refusing a query string that cannot be decoded on the whole route, not only forq, is fine.Model: opus-5-5
98d23ad32eto19bb063782Rework of #138 (comment):
url.ParseQuery, and one that cannot be decoded is a 400 on the whole route whose error shows it; route tests forq=80%andq=50;fit=contain; theTODO.mdentry and the PR body say so.Model: opus-5-5
FAIL
internal/handlers/image.go,parseImageRequest: only the firstqin the query string is checked, and any laterqis ignored.q=80&q=500,q=80&q=bananaandq=80&q=are served at 80 and pass a signature made for 80, while the same values in the other order (q=500&q=80,q=&q=80) are a 400. Aqthat is set and invalid is silently ignored. That misses the definition of done in #134 and the repo rule against a silent fallback for a parameter the request gives, and it makes theREADME.mdline ("a request whoseqis anything else is refused with 400") and theTODO.mdentry untrue. Acceptable: aqgiven more than once is a 400 namingq, or everyqvalue is checked the same way, with a route test such asq=80&q=500.Model: opus-5-5
19bb063782to35c576e67735c576e677to881446739eRework of #138 (comment):
/v1/image/is a 400 naming it; route tests forq=80&q=500,q=80&q=andfit=cover&fit=contain;README.md,TODO.mdand the code comment state the rule.q=&q=500test case is removed, as it is now one more repeatedq, and the linter refuses a third copy of the same expected error.TestHandleImage_InvalidQuery_Returns400, since it now coversfittoo.Model: opus-5-5
PASS: every way a request can give
qother than a whole number from 1 to 100 is now a 400, and a missingqis still 85.Model: opus-5-5