git-upload-pack strips leading / only when single-quoted #5848

New Issue

GiteaMirror · 2025-11-02T06:38:10-06:00

GiteaMirror commented

2025-11-02 06:38:10 -06:00

Originally created by @ninewise on GitHub (Aug 11, 2020).

Gitea version (or commit ref): v1.12.2
Git version: 2.26.2
Operating system: Alpine Linux
Database (use [x]):
- PostgreSQL
- MySQL
- MSSQL
- SQLite

Description

Creating this issue for a friend without github.

This line replaces first ocurrence of '/ with ' from parameter. This works for cloning the repository with git over SSH, as the remote command gets executed as git-upload-pack '/user/repo' (strace shows this clearly). In that case, said function rewrites it to git-upload-pack 'user/repo' and later on it gets correctly split in user and repo. Now, there are other implementations of Git which doesn't send the commands in that precise way, but as command git-upload-pack with parameter /user/repo (note no single-quotes). Got is one of such implementations, which can't clone the repo as it fails here.

I think the intended behaviour is to remove any leading / from the parameter, quoted or not. Most correct solution would be using a shell quoting library to normalize the input and then remove the leading /. Alternatively, for a quick fix, removing the replace in parseCmd and changing line 144 to also trim / should do the trick.

Originally created by @ninewise on GitHub (Aug 11, 2020). - Gitea version (or commit ref): v1.12.2 - Git version: 2.26.2 - Operating system: Alpine Linux - Database (use `[x]`): - [x] PostgreSQL - [ ] MySQL - [ ] MSSQL - [ ] SQLite ## Description Creating this issue for a friend without github. [This line] replaces first ocurrence of `'/` with `'` from parameter. This works for cloning the repository with `git` over SSH, as the remote command gets executed as `git-upload-pack '/user/repo'` (`strace` shows this clearly). In that case, said function rewrites it to `git-upload-pack 'user/repo'` and later on it gets correctly split in user and repo. Now, there are other implementations of Git which doesn't send the commands in that precise way, but as command `git-upload-pack` with parameter `/user/repo` (note no single-quotes). [Got] is one of such implementations, which can't clone the repo as it fails [here]. I think the intended behaviour is to remove any leading `/` from the parameter, quoted or not. Most correct solution would be using a shell quoting library to normalize the input and then remove the leading `/`. Alternatively, for a quick fix, removing the replace in `parseCmd` and changing [line 144] to also trim `/` should do the trick. [This line]: https://github.com/go-gitea/gitea/blob/02e990a89bcb1e55ea4b71f51cc5c24bfe11f885/cmd/serv.go#L62 [here]: https://github.com/go-gitea/gitea/blob/02e990a89bcb1e55ea4b71f51cc5c24bfe11f885/cmd/serv.go#L153 [Got]: http://www.gameoftrees.org/ [line 144]: https://github.com/go-gitea/gitea/blob/02e990a89bcb1e55ea4b71f51cc5c24bfe11f885/cmd/serv.go#L144

GiteaMirror closed this issue

2025-11-02 06:38:10 -06:00

GiteaMirror commented

2025-11-02 06:38:11 -06:00

@zeripath commented on GitHub (Aug 21, 2020):

The approach in #12554 would probably help fix this.

@zeripath commented on GitHub (Aug 21, 2020): The approach in #12554 would probably help fix this.

GiteaMirror commented

2025-11-02 06:38:12 -06:00

@ninewise commented on GitHub (Aug 24, 2020):

I don't think that will work. That code only deals with ", which I guess is well-defined in unified diff format. However, this case has to deal with shell quoting. I still believe that it'll be best to do proper shell parsing. If that's too invasive, I think the second-best approach will be teaching Gitea to with both single, double and unquoted parameters.

@ninewise commented on GitHub (Aug 24, 2020): I don't think that will work. That code only deals with `"`, which I guess is well-defined in unified diff format. However, this case has to deal with shell quoting. I still believe that it'll be best to do proper shell parsing. If that's too invasive, I think the second-best approach will be teaching Gitea to with both single, double and unquoted parameters.

GiteaMirror commented

2025-11-02 06:38:12 -06:00

@CL-Jeremy commented on GitHub (Aug 31, 2020):

@zeripath I think this PR broke clone via SSH completely... I actually found myself unable to clone anything via SSH on my own server the same day the PR was merged (which coincidentally had master merged in right on this commit), but thought it was my own mistake in configuration. I've just tried on try.gitea.io and got this:

02:11:41.343266 run-command.c:663       trace: run_command: unset GIT_DIR; ssh git@try.gitea.io 'git-upload-pack '\''CL-Jeremy/test-mirror.git'\'''
fatal: 'CL-Jeremy/test-mirror.git' does not appear to be a git repository
Gitea: Internal error
Failed to execute git command: exit status 128

@CL-Jeremy commented on GitHub (Aug 31, 2020): @zeripath I think this PR broke clone via SSH completely... I actually found myself unable to clone anything via SSH on my own server the same day the PR was merged (which coincidentally had master merged in right on this commit), but thought it was my own mistake in configuration. I've just tried on [try.gitea.io](try.gitea.io) and got this: ``` 02:11:41.343266 run-command.c:663 trace: run_command: unset GIT_DIR; ssh git@try.gitea.io 'git-upload-pack '\''CL-Jeremy/test-mirror.git'\''' fatal: 'CL-Jeremy/test-mirror.git' does not appear to be a git repository Gitea: Internal error Failed to execute git command: exit status 128 ```

GiteaMirror commented

2025-11-02 06:38:12 -06:00

@CL-Jeremy commented on GitHub (Sep 5, 2020):

🆗 fixed in https://github.com/go-gitea/gitea/pull/12668

@CL-Jeremy commented on GitHub (Sep 5, 2020): 🆗 fixed in https://github.com/go-gitea/gitea/pull/12668

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#5848

git-upload-pack strips leading / only when single-quoted #5848

Description

`git-upload-pack` strips leading `/` only when single-quoted #5848