Skip to content

Commit

Permalink
Handle correctly failed downloads (#11456)
Browse files Browse the repository at this point in the history
Hyper is not opinionated enough to care about connection StatusCode.
In case of errors such as AccessDenied, return code 403, it will still
download the return body and not raise any errors (see
#7121).
status_code.is_success checks if status is within 200-299.

```
nearcore/target/debug/neard --home ".near" init --chain-id inexistent_network --download-genesis --download-config
2024-06-01T18:23:34.118646Z  INFO neard: version="trunk" build="1.36.1-671-g99a05c482" latest_protocol=67
2024-06-01T18:23:34.121032Z  INFO near: Downloading config file from: https://s3-us-west-1.amazonaws.com/build.nearprotocol.com/nearcore-deploy/inexistent_network/config.json ...
Error: Failed to initialize configs

Caused by:
    0: Failed to download the config file from https://s3-us-west-1.amazonaws.com/build.nearprotocol.com/nearcore-deploy/inexistent_network/config.json
    1: Unsuccessful HTTP connection. Return code: 403 Forbidden
```

Unit test to make sure non successful codes are handled:
```
cargo test --package nearcore --lib download_file::tests::test_file_download_bad_http_code
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.33s
warning: the following packages contain code that will be rejected by a future version of Rust: fs_extra v1.2.0, wasmparser v0.78.2
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 52`
     Running unittests src/lib.rs (target/debug/deps/nearcore-9131a198eca62608)

running 1 test
test download_file::tests::test_file_download_bad_http_code ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 24 filtered out; finished in 0.00s
```
#7122
  • Loading branch information
andrei-near authored Jun 3, 2024
1 parent 33f9ca3 commit 6e1aef0
Showing 1 changed file with 42 additions and 1 deletion.
43 changes: 42 additions & 1 deletion nearcore/src/download_file.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use hyper::body::HttpBody;
use hyper::{body::HttpBody, StatusCode};
use indicatif::{ProgressBar, ProgressStyle};
use std::path::{Path, PathBuf};
use tokio::io::AsyncWriteExt;

#[derive(thiserror::Error, Debug)]
pub enum FileDownloadError {
#[error("Unsuccessful HTTP connection. Return code: {0}")]
HttpResponseCode(StatusCode),
#[error("{0}")]
HttpError(hyper::Error),
#[error("Failed to open temporary file")]
Expand Down Expand Up @@ -45,6 +47,10 @@ async fn download_file_impl(
let https_connector = hyper_tls::HttpsConnector::new();
let client = hyper::Client::builder().build::<_, hyper::Body>(https_connector);
let mut resp = client.get(uri).await.map_err(FileDownloadError::HttpError)?;
let status_code = resp.status();
if !status_code.is_success() {
return Err(FileDownloadError::HttpResponseCode(status_code));
}
let bar = if let Some(file_size) = resp.size_hint().upper() {
let bar = ProgressBar::new(file_size);
bar.set_style(
Expand Down Expand Up @@ -321,6 +327,41 @@ mod tests {
check_file_download(payload, Err("Failed to decompress XZ stream: lzma data error")).await;
}

#[tokio::test]
async fn test_file_download_bad_http_code() {
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let port = listener.local_addr().unwrap().port();
let tmp_file = tempfile::NamedTempFile::new().unwrap();

tokio::task::spawn(async move {
let make_svc = make_service_fn(move |_conn| {
let handle_request = move |_: Request<Body>| async move {
Ok::<_, Infallible>(
Response::builder()
.status(StatusCode::NOT_FOUND)
.body(Body::from(""))
.unwrap(),
)
};
async move { Ok::<_, Infallible>(service_fn(handle_request)) }
});
let server = Server::from_tcp(listener).unwrap().serve(make_svc);
if let Err(e) = server.await {
eprintln!("server error: {}", e);
}
});

let res = download_file(&format!("http://localhost:{}", port), tmp_file.path())
.await
.map(|()| std::fs::read(tmp_file.path()).unwrap());

assert!(
matches!(res, Err(FileDownloadError::HttpResponseCode(StatusCode::NOT_FOUND))),
"got {:?}",
res
);
}

fn auto_xz_test_write_file(
buffer: &[u8],
chunk_size: usize,
Expand Down

0 comments on commit 6e1aef0

Please sign in to comment.