Skip to content

Commit

Permalink
Show hint in resolution failure on Forbidden (403) or `Unauthoriz…
Browse files Browse the repository at this point in the history
…ed` (`401`) (#8264)

## Summary

Closes #8167.
  • Loading branch information
charliermarsh authored Oct 16, 2024
1 parent 5e05a62 commit 4ca1589
Show file tree
Hide file tree
Showing 12 changed files with 170 additions and 35 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ async_zip = { git = "https://github.com/charliermarsh/rs-async-zip", rev = "011b
axoupdater = { version = "0.7.2", default-features = false }
backoff = { version = "0.4.0" }
base64 = { version = "0.22.1" }
bitflags = { version = "2.6.0" }
boxcar = { version = "0.2.5" }
bytecheck = { version = "0.8.0" }
cachedir = { version = "0.3.1" }
Expand Down
38 changes: 20 additions & 18 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,12 @@ impl RegistryClient {
/// and [PEP 691 – JSON-based Simple API for Python Package Indexes](https://peps.python.org/pep-0691/),
/// which the pypi json api approximately implements.
#[instrument("simple_api", skip_all, fields(package = % package_name))]
pub async fn simple(
&self,
pub async fn simple<'index>(
&'index self,
package_name: &PackageName,
index: Option<&IndexUrl>,
) -> Result<Vec<(IndexUrl, OwnedArchive<SimpleMetadata>)>, Error> {
index: Option<&'index IndexUrl>,
capabilities: &IndexCapabilities,
) -> Result<Vec<(&'index IndexUrl, OwnedArchive<SimpleMetadata>)>, Error> {
let indexes = if let Some(index) = index {
Either::Left(std::iter::once(index))
} else {
Expand All @@ -222,30 +223,31 @@ impl RegistryClient {
for index in it {
match self.simple_single_index(package_name, index).await {
Ok(metadata) => {
results.push((index.clone(), metadata));
results.push((index, metadata));

// If we're only using the first match, we can stop here.
if self.index_strategy == IndexStrategy::FirstIndex {
break;
}
}
Err(err) => match err.into_kind() {
// The package is unavailable due to a lack of connectivity.
ErrorKind::Offline(_) => continue,

// The package could not be found in the remote index.
ErrorKind::WrappedReqwestError(err) => {
if err.status() == Some(StatusCode::NOT_FOUND)
|| err.status() == Some(StatusCode::UNAUTHORIZED)
|| err.status() == Some(StatusCode::FORBIDDEN)
{
continue;
ErrorKind::WrappedReqwestError(err) => match err.status() {
Some(StatusCode::NOT_FOUND) => {}
Some(StatusCode::UNAUTHORIZED) => {
capabilities.set_unauthorized(index.clone());
}
return Err(ErrorKind::from(err).into());
}
Some(StatusCode::FORBIDDEN) => {
capabilities.set_forbidden(index.clone());
}
_ => return Err(ErrorKind::from(err).into()),
},

// The package is unavailable due to a lack of connectivity.
ErrorKind::Offline(_) => {}

// The package could not be found in the local index.
ErrorKind::FileNotFound(_) => continue,
ErrorKind::FileNotFound(_) => {}

other => return Err(other.into()),
},
Expand Down Expand Up @@ -671,7 +673,7 @@ impl RegistryClient {

// Mark the index as not supporting range requests.
if let Some(index) = index {
capabilities.set_supports_range_requests(index.clone(), false);
capabilities.set_no_range_requests(index.clone());
}
} else {
return Err(err);
Expand Down
1 change: 1 addition & 0 deletions crates/uv-distribution-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ uv-platform-tags = { workspace = true }
uv-pypi-types = { workspace = true }

anyhow = { workspace = true }
bitflags = { workspace = true }
fs-err = { workspace = true }
itertools = { workspace = true }
jiff = { workspace = true }
Expand Down
80 changes: 68 additions & 12 deletions crates/uv-distribution-types/src/index_url.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use itertools::Either;
use rustc_hash::FxHashSet;
use rustc_hash::{FxHashMap, FxHashSet};
use std::borrow::Cow;
use std::fmt::{Display, Formatter};
use std::ops::Deref;
Expand Down Expand Up @@ -393,26 +393,82 @@ impl<'a> IndexUrls {
}
}

bitflags::bitflags! {
#[derive(Debug, Copy, Clone)]
struct Flags: u8 {
/// Whether the index supports range requests.
const NO_RANGE_REQUESTS = 1;
/// Whether the index returned a `401 Unauthorized` status code.
const UNAUTHORIZED = 1 << 2;
/// Whether the index returned a `403 Forbidden` status code.
const FORBIDDEN = 1 << 1;
}
}

/// A map of [`IndexUrl`]s to their capabilities.
///
/// For now, we only support a single capability (range requests), and we only store an index if
/// it _doesn't_ support range requests. The benefit is that the map is almost always empty, so
/// validating capabilities is extremely cheap.
/// We only store indexes that lack capabilities (i.e., don't support range requests, aren't
/// authorized). The benefit is that the map is almost always empty, so validating capabilities is
/// extremely cheap.
#[derive(Debug, Default, Clone)]
pub struct IndexCapabilities(Arc<RwLock<FxHashSet<IndexUrl>>>);
pub struct IndexCapabilities(Arc<RwLock<FxHashMap<IndexUrl, Flags>>>);

impl IndexCapabilities {
/// Returns `true` if the given [`IndexUrl`] supports range requests.
pub fn supports_range_requests(&self, index_url: &IndexUrl) -> bool {
!self.0.read().unwrap().contains(index_url)
!self
.0
.read()
.unwrap()
.get(index_url)
.is_some_and(|flags| flags.intersects(Flags::NO_RANGE_REQUESTS))
}

/// Mark an [`IndexUrl`] as not supporting range requests.
pub fn set_supports_range_requests(&self, index_url: IndexUrl, supports: bool) {
if supports {
self.0.write().unwrap().remove(&index_url);
} else {
self.0.write().unwrap().insert(index_url);
}
pub fn set_no_range_requests(&self, index_url: IndexUrl) {
self.0
.write()
.unwrap()
.entry(index_url)
.or_insert(Flags::empty())
.insert(Flags::NO_RANGE_REQUESTS);
}

/// Returns `true` if the given [`IndexUrl`] returns a `401 Unauthorized` status code.
pub fn unauthorized(&self, index_url: &IndexUrl) -> bool {
self.0
.read()
.unwrap()
.get(index_url)
.is_some_and(|flags| flags.intersects(Flags::UNAUTHORIZED))
}

/// Mark an [`IndexUrl`] as returning a `401 Unauthorized` status code.
pub fn set_unauthorized(&self, index_url: IndexUrl) {
self.0
.write()
.unwrap()
.entry(index_url)
.or_insert(Flags::empty())
.insert(Flags::UNAUTHORIZED);
}

/// Returns `true` if the given [`IndexUrl`] returns a `403 Forbidden` status code.
pub fn forbidden(&self, index_url: &IndexUrl) -> bool {
self.0
.read()
.unwrap()
.get(index_url)
.is_some_and(|flags| flags.intersects(Flags::FORBIDDEN))
}

/// Mark an [`IndexUrl`] as returning a `403 Forbidden` status code.
pub fn set_forbidden(&self, index_url: IndexUrl) {
self.0
.write()
.unwrap()
.entry(index_url)
.or_insert(Flags::empty())
.insert(Flags::FORBIDDEN);
}
}
8 changes: 7 additions & 1 deletion crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ use crate::resolution::ConflictingDistributionError;
use crate::resolver::{IncompletePackage, ResolverMarkers, UnavailablePackage, UnavailableReason};
use crate::Options;
use tracing::trace;
use uv_distribution_types::{BuiltDist, IndexLocations, IndexUrl, InstalledDist, SourceDist};
use uv_distribution_types::{
BuiltDist, IndexCapabilities, IndexLocations, IndexUrl, InstalledDist, SourceDist,
};
use uv_normalize::PackageName;
use uv_pep440::Version;
use uv_pep508::MarkerTree;
Expand Down Expand Up @@ -126,6 +128,7 @@ pub struct NoSolutionError {
selector: CandidateSelector,
python_requirement: PythonRequirement,
index_locations: IndexLocations,
index_capabilities: IndexCapabilities,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
fork_urls: ForkUrls,
Expand All @@ -143,6 +146,7 @@ impl NoSolutionError {
selector: CandidateSelector,
python_requirement: PythonRequirement,
index_locations: IndexLocations,
index_capabilities: IndexCapabilities,
unavailable_packages: FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
fork_urls: ForkUrls,
Expand All @@ -157,6 +161,7 @@ impl NoSolutionError {
selector,
python_requirement,
index_locations,
index_capabilities,
unavailable_packages,
incomplete_packages,
fork_urls,
Expand Down Expand Up @@ -261,6 +266,7 @@ impl std::fmt::Display for NoSolutionError {
&tree,
&self.selector,
&self.index_locations,
&self.index_capabilities,
&self.available_indexes,
&self.unavailable_packages,
&self.incomplete_packages,
Expand Down
53 changes: 52 additions & 1 deletion crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use pubgrub::{DerivationTree, Derived, External, Map, Range, ReportFormatter, Te
use rustc_hash::FxHashMap;

use uv_configuration::IndexStrategy;
use uv_distribution_types::{Index, IndexLocations, IndexUrl};
use uv_distribution_types::{Index, IndexCapabilities, IndexLocations, IndexUrl};
use uv_normalize::PackageName;
use uv_pep440::Version;

Expand Down Expand Up @@ -505,6 +505,7 @@ impl PubGrubReportFormatter<'_> {
derivation_tree: &ErrorTree,
selector: &CandidateSelector,
index_locations: &IndexLocations,
index_capabilities: &IndexCapabilities,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
Expand Down Expand Up @@ -540,6 +541,7 @@ impl PubGrubReportFormatter<'_> {
set,
selector,
index_locations,
index_capabilities,
available_indexes,
unavailable_packages,
incomplete_packages,
Expand Down Expand Up @@ -589,6 +591,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause1,
selector,
index_locations,
index_capabilities,
available_indexes,
unavailable_packages,
incomplete_packages,
Expand All @@ -602,6 +605,7 @@ impl PubGrubReportFormatter<'_> {
&derived.cause2,
selector,
index_locations,
index_capabilities,
available_indexes,
unavailable_packages,
incomplete_packages,
Expand All @@ -621,6 +625,7 @@ impl PubGrubReportFormatter<'_> {
set: &Range<Version>,
selector: &CandidateSelector,
index_locations: &IndexLocations,
index_capabilities: &IndexCapabilities,
available_indexes: &FxHashMap<PackageName, BTreeSet<IndexUrl>>,
unavailable_packages: &FxHashMap<PackageName, UnavailablePackage>,
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
Expand Down Expand Up @@ -721,6 +726,20 @@ impl PubGrubReportFormatter<'_> {
}
}
}

// Add hints due to an index returning an unauthorized response.
for index in index_locations.indexes() {
if index_capabilities.unauthorized(&index.url) {
hints.insert(PubGrubHint::UnauthorizedIndex {
index: index.url.clone(),
});
}
if index_capabilities.forbidden(&index.url) {
hints.insert(PubGrubHint::ForbiddenIndex {
index: index.url.clone(),
});
}
}
}

fn prerelease_available_hint(
Expand Down Expand Up @@ -873,6 +892,10 @@ pub(crate) enum PubGrubHint {
// excluded from `PartialEq` and `Hash`
next_index: IndexUrl,
},
/// An index returned an Unauthorized (401) response.
UnauthorizedIndex { index: IndexUrl },
/// An index returned a Forbidden (403) response.
ForbiddenIndex { index: IndexUrl },
}

/// This private enum mirrors [`PubGrubHint`] but only includes fields that should be
Expand Down Expand Up @@ -921,6 +944,12 @@ enum PubGrubHintCore {
UncheckedIndex {
package: PubGrubPackage,
},
UnauthorizedIndex {
index: IndexUrl,
},
ForbiddenIndex {
index: IndexUrl,
},
}

impl From<PubGrubHint> for PubGrubHintCore {
Expand Down Expand Up @@ -974,6 +1003,8 @@ impl From<PubGrubHint> for PubGrubHintCore {
workspace,
},
PubGrubHint::UncheckedIndex { package, .. } => Self::UncheckedIndex { package },
PubGrubHint::UnauthorizedIndex { index } => Self::UnauthorizedIndex { index },
PubGrubHint::ForbiddenIndex { index } => Self::ForbiddenIndex { index },
}
}
}
Expand Down Expand Up @@ -1214,6 +1245,26 @@ impl std::fmt::Display for PubGrubHint {
"--index-strategy unsafe-best-match".green(),
)
}
Self::UnauthorizedIndex { index } => {
write!(
f,
"{}{} An index URL ({}) could not be queried due to a lack of valid authentication credentials ({}).",
"hint".bold().cyan(),
":".bold(),
index.redacted().cyan(),
"401 Unauthorized".bold().red(),
)
}
Self::ForbiddenIndex { index } => {
write!(
f,
"{}{} An index URL ({}) could not be queried due to a lack of valid authentication credentials ({}).",
"hint".bold().cyan(),
":".bold(),
index.redacted().cyan(),
"403 Forbidden".bold().red(),
)
}
}
}
}
Expand Down
Loading

0 comments on commit 4ca1589

Please sign in to comment.