Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: Disable compression in default transport #393

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

doanac
Copy link
Member

@doanac doanac commented Apr 8, 2024

The default Golang client will advertise to the HTTP server that it can handle gzip compressed response encoding. This is true, but it breaks our artifacts.go code for doing download progress.

This is because the default behavior of the transport is to:

  • provide the proper reader to decompress the response
  • update the content-length header to be -1

The -1 breaks our logic. This fix gets us back to how things used to work before CloudFlare started compressing the responses.

@doanac doanac requested a review from vkhoroz April 8, 2024 16:23
@doanac
Copy link
Member Author

doanac commented Apr 8, 2024

similar bug was fix in jobserv UI - foundriesio/gavel-ci@215574a

Copy link
Member

@vkhoroz vkhoroz left a comment

Choose a reason for hiding this comment

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

Wow, I'm sorry for your nerves spent on this.

LGTM

client/foundries.go Show resolved Hide resolved
Copy link
Contributor

@mike-sul mike-sul left a comment

Choose a reason for hiding this comment

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

LGTM. I wonder if there is easy option to make it configurable, so we don't turn it off for all types of requests/clients.

The default Golang client will advertise to the HTTP server that
it can handle gzip compressed response encoding. This is true,
but it breaks our artifacts.go code for doing download progress.

This is because the default behavior of the transport is to:

 * provide the proper reader to decompress the response
 * update the content-length header to be -1

The -1 breaks our logic. This fix gets us back to how things used
to work before CloudFlare started compressing the responses.

Signed-off-by: Andy Doan <[email protected]>
@doanac doanac merged commit bc0a9a5 into foundriesio:main Apr 11, 2024
8 checks passed
@doanac doanac deleted the http-fix branch April 11, 2024 19:55
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.

3 participants