Skip to content

Conversation

@keegancsmith
Copy link
Member

Ensure that if the Content-Encoding of a servegit request is gzip then we read it as a compressed input. git-upload-pack does not accept gzipped input and fails. This is only sometimes gzipped and I'm not sure of the conditions so we can't rely on it being always or never encoded.

This is a port of https://github.com/sourcegraph/sourcegraph/pull/21067

Co-authored-by: @efritz

keegancsmith and others added 2 commits May 26, 2021 22:02
Previously we did http.Error. However, we have already set the HTTP
response code. Additionally we may have already written other data. So
we instead just write out the error message on a new line.

This error can often be swallowed by the git client. So we additionally
log the error to the debug log.
Ensure that if the Content-Encoding of a servegit request is gzip then
we read it as a compressed input. git-upload-pack does not accept
gzipped input and fails. This is only sometimes gzipped and I'm not sure
of the conditions so we can't rely on it being always or never encoded.

This is a port of sourcegraph/sourcegraph 8e1e58c.

Co-authored-by: Eric Fritz <eric@sourcegraph.com>
@keegancsmith keegancsmith requested a review from efritz May 26, 2021 20:12
@efritz
Copy link
Contributor

efritz commented May 26, 2021

Is there any other places that we need to update? (a la https://github.com/sourcegraph/sourcegraph/pull/21067#pullrequestreview-663133145)

Copy link
Contributor

@efritz efritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some common core here that's extractable/worthy of a util function in github.com/sourcegraph/sourcegraph/lib?

@keegancsmith keegancsmith merged commit 08d62c7 into main May 26, 2021
@keegancsmith keegancsmith deleted the k/serve-git-gzip branch May 26, 2021 21:21
@keegancsmith
Copy link
Member Author

Is there some common core here that's extractable/worthy of a util function in github.com/sourcegraph/sourcegraph/lib?

Yes this could make sense to share. We have a lot more instrumentation in gitserver version (which was a copy of this version originally I think :) ). I've added to my personal backlog to explore next month.

Is there any other places that we need to update? (a la sourcegraph/sourcegraph#21067 (review))

I believe this is everything. A quick search for something specific to this indicates that. https://sourcegraph.com/search?q=context:%40keegan+x-git-upload-pack-advertisement&patternType=regexp

scjohns pushed a commit that referenced this pull request Apr 24, 2023
Ensure that if the Content-Encoding of a servegit request is gzip then
we read it as a compressed input. git-upload-pack does not accept
gzipped input and fails. This is only sometimes gzipped and I'm not sure
of the conditions so we can't rely on it being always or never encoded.

This is a port of sourcegraph/sourcegraph 8e1e58c.

Co-authored-by: Eric Fritz <eric@sourcegraph.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants