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.

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

HTTP: delete Content-Encoding and Content-Length headers after decompression #5212

Merged
merged 1 commit into from
Nov 26, 2020
Merged

HTTP: delete Content-Encoding and Content-Length headers after decompression #5212

merged 1 commit into from
Nov 26, 2020
+28 −4

Conversation

maiha
Copy link
Contributor

@maiha maiha commented Oct 29, 2017

We got following conclusions from #4957.

  1. keep consistency by deleting some headers after decompression
  2. FYI: Go lang sets Uncompressed flag when the body is decompressed.

This PR provides the former and solves consistency problem I'm facing.

# pseudo code
io  = some_compressed_http
res = HTTP::Client::Response.from_io(io)
io  = res.to_io
res = HTTP::Client::Response.from_io(io)
io  = res.to_io
res = HTTP::Client::Response.from_io(io)
... # This works well infinitely

Since some discussion is necessary and the amount of change is further increased,
I'd like to divide the latter issue into other PR.

Thank you,

@RX14
Copy link
Member

RX14 commented Oct 29, 2017

We need to yield an extra uncompressed flag from this method, and use it in HTTP::Response, etc.

@jkthorne
Copy link
Contributor

jkthorne commented Jun 12, 2018 •
edited
Loading

@maiha What is the status on this?

@bararchy
Copy link
Contributor

@RX14 can this PR be closed? it seems abandoned

@RX14
Copy link
Member

RX14 commented Oct 21, 2019

This is, as far as I can tell, the only record of an unsolved issue. So unless someone wants to make an issue to replace this PR it shouldn't be closed.

@straight-shoota straight-shoota added help wanted This issue is generally accepted and needs someone to pick it up kind:feature pr:needs-work A PR requires modifications by the author. topic:stdlib:networking labels Oct 21, 2019
@jhass jhass force-pushed the http-consist-headers-body branch from a7fefa7 to 5adc8fc Compare November 25, 2020 22:04
jhass
jhass approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@jhass jhass left a comment

Choose a reason for hiding this comment

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

This looks great to me. I took the liberty to rebase this and fix the specs :)

@straight-shoota straight-shoota added this to the 1.0.0 milestone Nov 25, 2020
@jhass jhass removed pr:needs-work A PR requires modifications by the author. help wanted This issue is generally accepted and needs someone to pick it up labels Nov 25, 2020
@jhass jhass merged commit f0391d7 into crystal-lang:master Nov 26, 2020
@bcardiff bcardiff mentioned this pull request Dec 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhass jhass

@bcardiff bcardiff

@straight-shoota straight-shoota

Assignees
No one assigned
Labels
kind:feature topic:stdlib:networking
Projects
None yet
Milestone
0.36.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@maiha @RX14 @jkthorne @bararchy @jhass @bcardiff @straight-shoota