E53A Decompress responses by heaths · Pull Request #11569 · cli/cli · GitHub
[go: up one dir, main page]
Skip to content

Decompress responses #11569

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft

Decompress responses #11569

wants to merge 1 commit into from

Conversation

heaths
Copy link
Contributor
@heaths heaths commented Aug 22, 2025

Fixes #11559. Go's http.Transport will automatically decompress responses if the request doesn't send the accept-encoding header, but if a user requests it - since content-encoding is supposed to be dictated by the accept-encoding header (see RFC 9110 S8.4) - we'd need to decompress anyway.

@heaths
Decompress responses
e7a8e45 
Fixes cli#11559. Go's `http.Transport` will automatically decompress responses if the request doesn't send the `accept-encoding` header, but if a user requests it - since `content-encoding` is supposed to be dictated by the `accept-encoding` header (see RFC 9110 S8.4) - we'd need to decompress anyway.
@heaths heaths temporarily deployed to cli-automation August 22, 2025 17:37 — with GitHub Actions Inactive
@heaths
Copy link
Contributor Author
heaths commented Aug 22, 2025 •
edited
Loading

As commented on #11559, the best place to do this would be in github.com/cli/go-gh/pkg/api.NewHTTPClient before the sanitizerRoundTripper wraps the response and destroys the binary data. That would also work ubiquitiously for any REST client at least. GraphQL clients might be more of a challenge since that uses a third-party library.

As is, this PR alone won't work because the sanitizerRoundTripper does destroy the gzip header (binary) resulting in an invalid UTF-8 string error when gzip.NewReader tries to read the header. But I wanted to upload this as a draft PR anyway that shows we can make a minimal change to gh api if desired, as long as we have an option to disable sanitization of non-ASCII characters in go-gh; though, I don't think that's a good option either since it's there for a reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gzipped responses are not decoded so cannot be unmarshaled as JSON
1 participant
@heaths
0