From facc60f3a84a8220655f45acf8f60aa3c8f17268 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 17 Feb 2024 09:37:06 -0500 Subject: [PATCH] Add graceful fallback for Artifactory indexes (#1574) ## Summary There are more details in https://github.com/astral-sh/uv/issues/1370, but it looks like Artifactory servers have incorrect behavior when it comes to HTTP range requests, in that they return `Accept-Ranges: bytes`, but then incorrectly return 200 requests when you actually ask for a given range. This PR ensures that we fallback gracefully in this case. It's built on https://github.com/prefix-dev/async_http_range_reader/pull/5. Assuming that gets merged upstream, we can then remove the Git dependency. Closes https://github.com/astral-sh/uv/issues/1370. ## Test Plan `cargo run pip install requests -i https://killjoyuvbug.jfrog.io/artifactory/api/pypi/pypi/simple --verbose` --- Cargo.lock | 5 ++-- Cargo.toml | 2 +- crates/uv-client/src/error.rs | 31 +++++++++++++++++++++++++ crates/uv-client/src/registry_client.rs | 27 +++++++++++---------- 4 files changed, 48 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2aaf52edb..8f6835073 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -203,9 +203,9 @@ dependencies = [ [[package]] name = "async_http_range_reader" -version = "0.5.0" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea8c52f8b749ec4e8665041001a31208afdae9ef88916d2edf1610deb8b3616e" +checksum = "5143aaae4ec035a5d7cfda666eab896fe5428a2a8ab09ca651a2dce3a8f06912" dependencies = [ "bisection", "futures", @@ -213,6 +213,7 @@ dependencies = [ "itertools 0.12.1", "memmap2 0.9.4", "reqwest", + "reqwest-middleware", "thiserror", "tokio", "tokio-stream", diff --git a/Cargo.toml b/Cargo.toml index a3ad53a02..e815ddac8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ anstream = { version = "0.6.5" } anyhow = { version = "1.0.79" } async-compression = { version = "0.4.6" } async-trait = { version = "0.1.77" } -async_http_range_reader = { version = "0.5.0" } +async_http_range_reader = { version = "0.6.1" } async_zip = { git = "https://github.com/charliermarsh/rs-async-zip", rev = "d76801da0943de985254fc6255c0e476b57c5836", features = ["deflate"] } base64 = { version = "0.21.7" } cachedir = { version = "0.3.1" } diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index 92a61a21b..70bffaf53 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -155,4 +155,35 @@ impl ErrorKind { ErrorKind::RequestMiddlewareError(err) } + + /// Returns `true` if the error is due to the server not supporting HTTP range requests. + pub(crate) fn is_http_range_requests_unsupported(&self) -> bool { + match self { + // The server doesn't support range requests (as reported by the `HEAD` check). + ErrorKind::AsyncHttpRangeReader( + AsyncHttpRangeReaderError::HttpRangeRequestUnsupported, + ) => { + return true; + } + + // The server doesn't support range requests, but we only discovered this while + // unzipping due to erroneous server behavior. + ErrorKind::Zip(_, ZipError::UpstreamReadError(err)) => { + if let Some(inner) = err.get_ref() { + if let Some(inner) = inner.downcast_ref::() { + if matches!( + inner, + AsyncHttpRangeReaderError::HttpRangeRequestUnsupported + ) { + return true; + } + } + } + } + + _ => {} + } + + false + } } diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index fb6596e90..68318a68f 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -3,7 +3,7 @@ use std::fmt::Debug; use std::path::Path; use std::str::FromStr; -use async_http_range_reader::{AsyncHttpRangeReader, AsyncHttpRangeReaderError}; +use async_http_range_reader::AsyncHttpRangeReader; use async_zip::tokio::read::seek::ZipFileReader; use futures::{FutureExt, TryStreamExt}; use reqwest::{Client, ClientBuilder, Response, StatusCode}; @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; use tempfile::tempfile_in; use tokio::io::BufWriter; use tokio_util::compat::FuturesAsyncReadCompatExt; -use tracing::{debug, info_span, instrument, trace, warn, Instrument}; +use tracing::{info_span, instrument, trace, warn, Instrument}; use url::Url; use distribution_filename::{DistFilename, SourceDistFilename, WheelFilename}; @@ -454,19 +454,17 @@ impl RegistryClient { match result { Ok(metadata) => return Ok(metadata), - Err(err) => match err.into_kind() { - ErrorKind::AsyncHttpRangeReader( - AsyncHttpRangeReaderError::HttpRangeRequestUnsupported, - ) => {} - kind => return Err(kind.into()), - }, + Err(err) => { + if err.kind().is_http_range_requests_unsupported() { + // The range request version failed. Fall back to downloading the entire file + // and the reading the file from the zip the regular way. + warn!("Range requests not supported for {filename}; downloading wheel"); + } else { + return Err(err); + } + } } - // The range request version failed (this is bad, the webserver should support this), fall - // back to downloading the entire file and the reading the file from the zip the regular - // way. - - debug!("Range requests not supported for {filename}; downloading wheel"); // TODO(konstin): Download the wheel into a cache shared with the installer instead // Note that this branch is only hit when you're not using and the server where // you host your wheels for some reasons doesn't support range requests @@ -708,8 +706,9 @@ pub enum Connectivity { mod tests { use std::str::FromStr; - use pypi_types::{JoinRelativeError, SimpleJson}; use url::Url; + + use pypi_types::{JoinRelativeError, SimpleJson}; use uv_normalize::PackageName; use crate::{html::SimpleHtml, SimpleMetadata, SimpleMetadatum};