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 debug panic on download with redirect body. #10048

Merged
merged 1 commit into from
Nov 8, 2021

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 5, 2021

With a debug build of cargo, downloading crates can panic if the download host issues a redirect with a body. From what I can see, the curl progress function gets called with the original size of the redirect body (such as total=154 cur=154, indicating that it has read 154 bytes of the redirect message). Then it calls the progress function again with cur=0 to start again from the beginning. The next line in this patch, cur - dl.current.get() would panic since it is a u64 and a 0 value of cur is less than the old current.

This was never really an issue with crates.io because it emits a redirect body of 0 bytes.

I think it is fine to skip this block in that situation, as it is only for resetting the timeout counter. Though, I guess it could use saturating_sub instead.

@rust-highfive
Copy link

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2021
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 8, 2021

📌 Commit 3d20973 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2021
@bors
Copy link
Contributor

bors commented Nov 8, 2021

⌛ Testing commit 3d20973 with merge 2e2a16e...

@bors
Copy link
Contributor

bors commented Nov 8, 2021

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 2e2a16e to master...

@bors bors merged commit 2e2a16e into rust-lang:master Nov 8, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 9, 2021
Update cargo

4 commits in 94ca096afbf25f670e76e07dca754fcfe27134be..2e2a16e983f597da62bc132eb191bc3276d4b1bb
2021-10-29 14:45:06 +0000 to 2021-11-08 15:13:38 +0000
- Fix debug panic on download with redirect body. (rust-lang/cargo#10048)
- no need to clone (rust-lang/cargo#10051)
- Update curl. (rust-lang/cargo#10040)
- Fix --scrape-examples-target-crate using package name (with dashes) instead of crate name (with underscores) (rust-lang/cargo#10037)
@ehuss ehuss added this to the 1.58.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants