Skip to content

Commit 4fd77e9

Browse files
committed
fix(download): verify etags for resumed requests
Signed-off-by: Marcel Guzik <[email protected]>
1 parent ce69008 commit 4fd77e9

File tree

1 file changed

+39
-2
lines changed

1 file changed

+39
-2
lines changed

crates/common/download/src/download.rs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use anyhow::anyhow;
55
use backoff::future::retry_notify;
66
use backoff::ExponentialBackoff;
77
use certificate::CloudHttpConfig;
8+
use hyper::header;
89
use log::debug;
910
use log::info;
1011
use log::warn;
@@ -199,6 +200,12 @@ impl Downloader {
199200
loop {
200201
let offset = next_request_offset(&prev_response, file);
201202
let mut response = self.request_range_from(url, offset).await?;
203+
// etags in current and previous request must match to resume
204+
if !etags_matching(&prev_response, &response) {
205+
file.seek(SeekFrom::Start(0)).unwrap();
206+
prev_response = response;
207+
continue;
208+
}
202209
let offset = partial_response::response_range_start(&response)?;
203210

204211
if offset != 0 {
@@ -211,9 +218,7 @@ impl Downloader {
211218
Ok(()) => break,
212219

213220
Err(SaveChunksError::Network(err)) => {
214-
prev_response = response;
215221
warn!("Error while downloading response: {err}.\nRetrying...");
216-
continue;
217222
}
218223

219224
Err(SaveChunksError::Io(err)) => {
@@ -223,6 +228,7 @@ impl Downloader {
223228
})
224229
}
225230
}
231+
prev_response = response;
226232
}
227233

228234
Ok(())
@@ -352,6 +358,37 @@ impl Downloader {
352358
}
353359
}
354360

361+
fn etags_matching(prev_response: &Response, response: &Response) -> bool {
362+
if let Some(etag) = response
363+
.headers()
364+
.get(header::ETAG)
365+
.and_then(|h| h.to_str().ok())
366+
{
367+
// TODO: handle optional backslashes
368+
if etag.starts_with("W/") {
369+
// validator is weak, but in range requests tags must match using strong comparison
370+
// https://www.rfc-editor.org/rfc/rfc9110#section-13.1.5-12.1
371+
return false;
372+
}
373+
374+
let Some(prev_etag) = prev_response
375+
.headers()
376+
.get(header::ETAG)
377+
.and_then(|h| h.to_str().ok())
378+
else {
379+
// previous request didn't have etag and this does or vice versa, abort
380+
return false;
381+
};
382+
383+
if etag != prev_etag {
384+
// etags don't match, abort
385+
return false;
386+
}
387+
}
388+
// etags match
389+
true
390+
}
391+
355392
/// Decides whether HTTP request error is retryable.
356393
fn reqwest_err_to_backoff(err: reqwest::Error) -> backoff::Error<reqwest::Error> {
357394
if err.is_timeout() || err.is_connect() {

0 commit comments

Comments
 (0)