Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

gitserver: use -z for ListFiles#50023

Merged
keegancsmith merged 1 commit into
mainfrom
k/list-files
Mar 27, 2023
Merged

gitserver: use -z for ListFiles#50023
keegancsmith merged 1 commit into
mainfrom
k/list-files

Conversation

@keegancsmith

Copy link
Copy Markdown
Member

If you do not pass -z to git ls-tree, it will quote filenames which contain special characters. This API isn't used much, so we have never noticed this issue before (most client code uses LsFiles). However, we Cody is using this API, and that is how we noticed it. Autoindexing uses it, and likely didn't encounter issues because the pattern passed in was likely specific enough to not include filenames with special characters.

In the future we should look into unifying ListFiles and LsFiles, but they are different enough right now. Right now we copy-paste how LsFiles parses the output.

Test Plan: added a test case

If you do not pass -z to git ls-tree, it will quote filenames which
contain special characters. This API isn't used much, so we have never
noticed this issue before (most client code uses LsFiles). However, we
Cody is using this API, and that is how we noticed it. Autoindexing uses
it, and likely didn't encounter issues because the pattern passed in was
likely specific enough to not include filenames with special characters.

In the future we should look into unifying ListFiles and LsFiles, but
they are different enough right now. Right now we copy-paste how LsFiles
parses the output.

Test Plan: added a test case
@keegancsmith keegancsmith requested review from a team and novoselrok March 27, 2023 14:25
@cla-bot cla-bot Bot added the cla-signed label Mar 27, 2023
@sourcegraph-bot

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 00ec5ff...efcaba3.

Notify File(s)
@indradhanush internal/gitserver/commands.go
internal/gitserver/commands_test.go
@sashaostrikov internal/gitserver/commands.go
internal/gitserver/commands_test.go

@keegancsmith keegancsmith merged commit 9e4963f into main Mar 27, 2023
@keegancsmith keegancsmith deleted the k/list-files branch March 27, 2023 15:35
github-actions Bot pushed a commit that referenced this pull request Mar 28, 2023
If you do not pass -z to git ls-tree, it will quote filenames which
contain special characters. This API isn't used much, so we have never
noticed this issue before (most client code uses LsFiles). However, we
Cody is using this API, and that is how we noticed it. Autoindexing uses
it, and likely didn't encounter issues because the pattern passed in was
likely specific enough to not include filenames with special characters.

In the future we should look into unifying ListFiles and LsFiles, but
they are different enough right now. Right now we copy-paste how LsFiles
parses the output.

Test Plan: added a test case

(cherry picked from commit 9e4963f)
keegancsmith added a commit that referenced this pull request Mar 28, 2023
If you do not pass -z to git ls-tree, it will quote filenames which
contain special characters. This API isn't used much, so we have
never noticed this issue before (most client code uses LsFiles).
However, we Cody is using this API, and that is how we noticed it.
Autoindexing uses it, and likely didn't encounter issues because the
pattern passed in was likely specific enough to not include filenames
with special characters.

In the future we should look into unifying ListFiles and LsFiles, but
they are different enough right now. Right now we copy-paste how LsFiles
parses the output.

Test Plan: added a test case

Backport
9e4963f from #50023

Co-authored-by: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants