Repository navigation
Conversation
…and commit-pack wip
|
@codex review |
|
@codex security review |
Codex Review SummaryThis 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. 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. |
Both fields of a commit-pack file operation count as absent when empty, the fuzz target checks the resolved path, and the docs describe rolling upgrades correctly.
|
@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. |
|
@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. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Bravo. 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. |
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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
ThomasK33 asked to fix and build the open issues of this repo and land them on main. For #587 (a path that is not valid UTF-8 comes back changed in JSON) this is the last of three PRs. It is stacked on #727 (its base is #727's branch) and must not merge before #727. #721 is on main. The first two PRs gave every response path an exact
<field>_base64sibling. A client that reads such a path still cannot send it back in a JSON body, because JSON cannot hold the byte. This PR adds the request forms, so a listed path round-trips into grep, archive and commit-pack. With it, #587 is finished.What changes
A request that names a path takes the base64 of the raw bytes in a sibling field:
POST /repos/greppaths_base64(adds prefixes topaths),file_filters.include_globs_base64andfile_filters.exclude_globs_base64(add to the plain globs)POST /repos/archiveinclude_globs_base64andexclude_globs_base64POST /repos/commit-packfiles[].path_base64instead ofpath. A file operation has exactly one of the two, and an empty one counts as absentdecodeRawPathaccepts only canonical padded standard base64. Go's decoder alone also accepts CR and LF inside the text and non-zero padding bits, so two texts could name one path. In grep and archive a bad value is a 400 with aninvalid_paramsentry that names it (paths_base64.1). In commit-pack it is a 400invalidwith a message. A decoded grep or archive value must not be blank or hold a NUL, as for the plain list. A decoded commit-pack path goes through the same checks aspath, with duplicates counted across both forms.?and*match invalid bytes. A[...]class that holds an invalid byte is malformed forpath.Matchand matches nothing. The docs say so.path_base64with 400invalid(empty path). The docs name v0.20.0 as the first release. Release PR chore(main): release 0.20.0 #653 is open for 0.20.0, so this PR must merge before it does.TestEveryPathFieldHasABase64Sibling) also covers*_globsfields, and its exemptions are gone: every path field of the package has its sibling.FuzzDecodeRawPath,FuzzDecodeRawPatternsandFuzzCommitPackFilePathhave seeds, ran 40 s each with no crash, and are infuzz-nightly.yml. The commit-pack target checks the resolved path, and a mutant that drops it fails the first seed.docs/reference/http-api.md"Path encoding" (the input forms, the first release, the endpoint rows),docs/explanation/streaming-commit-protocols.mdanddocs/explanation/wire-compatibility.md.Verification
go build ./...,go vet ./internal/api/,golangci-lint2.15.0 on./internal/api/...: 0 issues. Doc link gate: 0 broken.go test ./internal/api/... ./cmd/openstorage/... -tags=openstorage_viewcheck -race -skip '/^pg$': ok (internal/api330 s under host load,cmd/openstorage32 s). CI runs both backends.grep paths_base64 ignoredfailsTestRawPathInputs/grep_paths_base64_names_one_of_two_colliding_files,decoder lenientfails the bad-input test, andcommit-pack both allowedfails the commit-pack test.serve, real git and curl: thepath_base64values are taken unchanged from thefileslisting. grep withpaths_base64matches onlya<0xff>bwhile its colliding twina<0xfe>bstays out, andpaths: ["a\ufffdb"]matches only the file whose valid name holds U+FFFD. A directory prefix, a union withpaths, include and exclude globs and the bad-input 400 all behave as specified. Archive globs keep and drop the right file. One commit-pack call rewritesa<0xff>b, deletesa<0xfe>band addsadded<0xfd>.txt.file?path=and a fresh clone agree. 13 checks pass.Closes #587
Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh