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

Fix downloading huge files (broken since hyperup) #22144

Merged
merged 3 commits into from Nov 8, 2018

Conversation

Eijebong
Copy link
Contributor

@Eijebong Eijebong commented Nov 8, 2018

This change is Reviewable

Sometimes hyper sends data that can't completely decompressed, resetting
the buffer means we're losing some data and thus breaking the body
…e body

If hyper reads compressed enough data, we were decompressing 32k by 32k
but we were throwing away the end of the body because we would end up
having lots of backed up data in the cursor when hyper was done.
@highfive
Copy link

highfive commented Nov 8, 2018

Heads up! This PR modifies the following files:

@highfive
Copy link

highfive commented Nov 8, 2018

warning Warning warning

  • These commits modify net code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 8, 2018
@Eijebong
Copy link
Contributor Author

Eijebong commented Nov 8, 2018

cc @Manishearth

@jdm
Copy link
Member

jdm commented Nov 8, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit dcbe7d3 has been approved by jdm

@highfive highfive assigned jdm and unassigned Manishearth Nov 8, 2018
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 8, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit dcbe7d3 with merge 8ebac59...

bors-servo pushed a commit that referenced this pull request Nov 8, 2018
Fix downloading huge files (broken since hyperup)

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22144)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 8, 2018
@Eijebong
Copy link
Contributor Author

Eijebong commented Nov 8, 2018

[Errno 12] Cannot allocate memory

components/net/connector.rs Outdated Show resolved Hide resolved
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 8, 2018
@jdm
Copy link
Member

jdm commented Nov 8, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 85c6eff has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 8, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 85c6eff with merge 5e3b67f...

bors-servo pushed a commit that referenced this pull request Nov 8, 2018
Fix downloading huge files (broken since hyperup)

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22144)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-taskcluster

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 8, 2018
@jdm
Copy link
Member

jdm commented Nov 8, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 85c6eff with merge dd5e5e9...

bors-servo pushed a commit that referenced this pull request Nov 8, 2018
Fix downloading huge files (broken since hyperup)

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22144)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 8, 2018
@bors-servo
Copy link
Contributor

💔 Test failed - mac-rel-wpt1

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 8, 2018
@jdm
Copy link
Member

jdm commented Nov 8, 2018

@bors-servo
Copy link
Contributor

@bors-servo
Copy link
Contributor

@bors-servo bors-servo merged commit 85c6eff into servo:master Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants