From 6e1aef014f9b3d815857f8f2bdc330dd61f2f4a1 Mon Sep 17 00:00:00 2001 From: Andrei <122784628+andrei-near@users.noreply.github.com> Date: Mon, 3 Jun 2024 14:28:07 +0400 Subject: [PATCH] Handle correctly failed downloads (#11456) 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 https://github.com/near/nearcore/issues/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 ``` https://github.com/near/nearcore/issues/7122 --- nearcore/src/download_file.rs | 43 ++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/nearcore/src/download_file.rs b/nearcore/src/download_file.rs index 27e4fcdc4d1..ae2dc794066 100644 --- a/nearcore/src/download_file.rs +++ b/nearcore/src/download_file.rs @@ -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")] @@ -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( @@ -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| 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,