Repository navigation
fix: list repos for a q or prefix of any length, and refuse NUL and invalid UTF-8 - #783
Conversation
SQLite refuses a LIKE pattern over 50,000 bytes (SQLITE_MAX_LIKE_PATTERN_LENGTH), so GET /repos answered 500 for a q or prefix of 50,000 bytes or 25,000 percent signs on the local backend. SQLite's ListRepos now tests the start of lower(url) with substr and q with instr, as the S3-WAL backend does in Go, so the filters have no length limit and need no escaping. PostgreSQL refuses a NUL byte or invalid UTF-8 in a text parameter, and answered 500 for one in q or prefix, while SQLite matched a q with a NUL byte as its text before the NUL. GET /repos now answers 400 for either in q or prefix, and every backend asserts the filters are UTF-8 without NUL. Change-Id: Iace746814c0c2af656a1f75c4e747f46467cb0e1 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: I37772e9c3af38d3e2e396d3ffe3d0eb7fff7a2de Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
TestS3WALMetaConformance now runs testListReposLongFilters, so the S3-WAL backend's Go matcher gets the same long q and prefix cases as PostgreSQL and SQLite. testListReposPrefixFilter was already in the list. Change-Id: I80831bff27a7bfa9ea9843fbffa103a86b45d6f1 Signed-off-by: Thomas Kosiewski <tk@coder.com>
q=GAMMA must list github.com/acme/gamma on every backend. A q that is matched case-sensitively now fails TestListReposQFilter too, not only TestListReposLongFilters. Change-Id: I30e8bf5ce4e803d8e8c4d11189793ebc2f9d709f Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Why
GET /api/v1/repos?q=<200 KiB>answers 500 on main (found by #780's verifier). The cause is SQLite: it refuses aLIKEpattern over 50,000 bytes (SQLITE_MAX_LIKE_PATTERN_LENGTH, "LIKE or GLOB pattern too complex"). The local backend matchedqandprefixwithlower(url) LIKE lower(?), and the escaping doubles each%,_and\, so aqorprefixof 50,000 bytes, or of 25,000 percent signs, answered 500. PostgreSQL answers 200 there.The same filters had a second gap. PostgreSQL refuses a NUL byte and invalid UTF-8 in a text parameter (SQLSTATE 22021), so a
qorprefixwith either answered 500 there. SQLite read aqonly up to a NUL byte:q=a%00blistedacme/alpha.What changes
ListRepostests the prefix withsubstr(lower(url), 1, length(?)) = lower(?)andqwithinstr(substr(lower(url), length(<prefix>) + 1), lower(?)) > 0, as the S3-WAL backend does in Go. The filters have no length limit there and need no escaping. PostgreSQL keepsILIKE.GET /reposanswers 400 for aqorprefixwith a NUL byte or invalid UTF-8: "invalid q (want UTF-8 text without NUL)".meta.ListParamsstates that contract, and every backend asserts it (assertListFilters).docs/reference/http-api.mdsays so.Verification
TestListReposLongFilters(meta, on PostgreSQL and SQLite, and on an in-memory S3-WAL store throughTestS3WALMetaConformance): aqand aprefixof 200 KiB list nothing. Aqof 30,000 percent signs lists the one URL that holds them, and one more percent sign lists nothing. Aprefixofacme/and 30,000 percent signs withq=ENDlists that URL, andq=%inside that prefix lists nothing. On main every SQLite case fails with "LIKE or GLOB pattern too complex", and PostgreSQL passes. S3-WAL passes at this head.TestListFilterValues(api, PostgreSQL and SQLite): aqof 200 KiB, aprefixof 200 KiB and aqof 25,001 percent signs answer 200 with an empty last page. Aqorprefixwith a NUL byte or invalid UTF-8 answers 400. On main, SQLite answers 500 for the three long values and 200 for the four NUL and UTF-8 cases, and PostgreSQL answers 500 for the four NUL and UTF-8 cases.TestListReposPrefixFiltergainsacme/pierre/inner, which holds the prefixpierre/but not at its start. It runs on PostgreSQL, SQLite and S3-WAL (TestS3WALMetaConformancealready listed it).TestListReposQFiltergainsq=GAMMA, which must listgithub.com/acme/gamma: aqin another case than the URL.qmatched from the prefix's last character,qmatched over the whole URL, the prefix matched anywhere, and a case-sensitiveq. In the S3-WAL matcher, aqmatched over the whole URL failsListReposPrefixFilterandListReposLongFilters. A case-sensitiveqfails bothTestListReposQFilterandTestListReposLongFilterson each backend:qnot lowercased on SQLite and S3-WAL, andLIKEforILIKEon PostgreSQL.-race: build, vet, golangci-lint (0 issues), the doc-link check and both--fingerprints-onlychecks pass.internal/metapasses. Its sharedListRepostests run on PostgreSQL and SQLite (runBackends), andTestS3WALMetaConformanceruns them on an in-memory S3-WAL store.internal/apipasses with and without-tags=openstorage_viewcheck.internal/apiruns each integration test on PostgreSQL and SQLite only, so no lane runsGET /reposon S3-WAL. The handler refuses a NUL byte or invalid UTF-8 before it calls the store, so that 400 does not depend on the backend.internal/apirun failedTestReadCoreFilesMetadataOrder/pg/v1with 503 "request exceeded the server's git time budget". That run took 792 s, against 402 s for the passing tagged rerun, and the host load average read 56 to 71 right after it. This round changes nointernal/apicode.q=GAMMArow, and merges origin/main 4a505d0, which changes no file of this PR), with-race: build, vet, golangci-lint (0 issues), the doc-link check and both--fingerprints-onlychecks pass.internal/metapasses on PostgreSQL, SQLite and in-memory S3-WAL, andinternal/apipasses with-tags=openstorage_viewcheckon PostgreSQL and SQLite.Size
141 changed lines: 51 of production Go, 88 of tests and 2 of docs.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh