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

Add Gzip decoding automatically to Response #3

Closed
seanmonstar opened this issue Oct 16, 2016 · 7 comments
Closed

Add Gzip decoding automatically to Response #3

seanmonstar opened this issue Oct 16, 2016 · 7 comments

Comments

@seanmonstar
Copy link
Owner

No description provided.

@echochamber
Copy link
Contributor

Would using https://github.com/alexcrichton/flate2-rs work here?

@seanmonstar
Copy link
Owner Author

Yes, that would probably be a good crate to use.

Internally, we'd adjust the Response struct to be an enum, with variants
for plain text and gzip, and automatically pick which one to use after
inspecting headers.

On Tue, Nov 15, 2016, 10:07 PM Jason Schein notifications@github.com
wrote:

Would using https://github.com/alexcrichton/flate2-rs work here?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADJF7k25RH2-vZdW1dTqO11tgq3EMeVks5q-p26gaJpZM4KYB2F
.

@echochamber
Copy link
Contributor

Cool, I'll give this one a go over the weekend.

@echochamber
Copy link
Contributor

Its been a while since I've written any Rust and I'm pretty novice when it comes low(er) level stuff, so I want to thoroughly test this to actually make sure it works before submitting a PR, but if you want to see my progress and provide any feedback, here is my local fork/branch.

@seanmonstar
Copy link
Owner Author

@echochamber feel free to submit a PR, it is actually far easier to comment and review it when it is a PR. PRs don't have to be perfect from the beginning :)

Regarding echochamber@fb399a2:

I realize I mispoke, I didn't meant to change the Response struct to an enum, but instead to change the inner field to hold an enum. By keeping it a struct and the internals a private field, we can add more helpers or refactor later and not break anyone. For example:

pub struct Response {
    inner: Decoder
}

enum Decoder {
    Plaintext(::hyper::client::Response),
    Gzip(GzipDecoder<::hyper::client::Response>),
} 

Not I haven't checked that that is exactly how you type a GzipDecoder.

Separately, it seems that we should be checking the Transfer-Encoding, not the Content-Encoding. See http://stackoverflow.com/questions/11641923/transfer-encoding-gzip-vs-content-encoding-gzip

@echochamber
Copy link
Contributor

@echochamber feel free to submit a PR, it is actually far easier to comment and review it when it is a PR. PRs don't have to be perfect from the beginning :)

Opened a PR here #25

Thanks for the feedback and suggestions thus far. I'll try to have my PR reflect your suggestions this evening.

@seanmonstar
Copy link
Owner Author

Closed by #61

repi pushed a commit to EmbarkStudios/reqwest that referenced this issue Dec 20, 2019
repi pushed a commit to EmbarkStudios/reqwest that referenced this issue Dec 20, 2019
gsollazzo added a commit to AIDEM-Technologies/reqwest that referenced this issue Jul 18, 2022
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

No branches or pull requests

2 participants