diff --git a/Cargo.lock b/Cargo.lock index ee6bcaee389..8c0eeafb220 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -585,13 +585,13 @@ dependencies = [ [[package]] name = "crates-io" -version = "0.37.0" +version = "0.38.0" dependencies = [ - "anyhow", "curl", "percent-encoding", "serde", "serde_json", + "thiserror", "url", ] diff --git a/Cargo.toml b/Cargo.toml index 63e06adb53a..8cdb6404f47 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ cargo-util = { version = "0.2.5", path = "crates/cargo-util" } cargo_metadata = "0.14.0" clap = "4.2.0" core-foundation = { version = "0.9.0", features = ["mac_os_10_7_support"] } -crates-io = { version = "0.37.0", path = "crates/crates-io" } +crates-io = { version = "0.38.0", path = "crates/crates-io" } criterion = { version = "0.5.1", features = ["html_reports"] } curl = "0.4.44" curl-sys = "0.4.63" @@ -87,6 +87,7 @@ syn = { version = "2.0.14", features = ["extra-traits", "full"] } tar = { version = "0.4.38", default-features = false } tempfile = "3.1.0" termcolor = "1.1.2" +thiserror = "1.0.40" time = { version = "0.3", features = ["parsing", "formatting"] } toml = "0.7.0" toml_edit = "0.19.0" diff --git a/crates/crates-io/Cargo.toml b/crates/crates-io/Cargo.toml index a82b1d126f7..139b8aa9740 100644 --- a/crates/crates-io/Cargo.toml +++ b/crates/crates-io/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "crates-io" -version = "0.37.0" +version = "0.38.0" edition.workspace = true license.workspace = true repository = "https://github.com/rust-lang/cargo" @@ -13,9 +13,9 @@ name = "crates_io" path = "lib.rs" [dependencies] -anyhow.workspace = true curl.workspace = true percent-encoding.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true +thiserror.workspace = true url.workspace = true diff --git a/crates/crates-io/lib.rs b/crates/crates-io/lib.rs index 243808098c4..6ce39cefd4b 100644 --- a/crates/crates-io/lib.rs +++ b/crates/crates-io/lib.rs @@ -1,18 +1,18 @@ #![allow(clippy::all)] use std::collections::BTreeMap; -use std::fmt; use std::fs::File; use std::io::prelude::*; use std::io::{Cursor, SeekFrom}; use std::time::Instant; -use anyhow::{bail, format_err, Context, Result}; use curl::easy::{Easy, List}; use percent_encoding::{percent_encode, NON_ALPHANUMERIC}; use serde::{Deserialize, Serialize}; use url::Url; +pub type Result = std::result::Result; + pub struct Registry { /// The base URL for issuing API requests. host: String, @@ -125,67 +125,62 @@ struct Crates { meta: TotalCrates, } -#[derive(Debug)] -pub enum ResponseError { - Curl(curl::Error), +/// Error returned when interacting with a registry. +#[derive(Debug, thiserror::Error)] +pub enum Error { + /// Error from libcurl. + #[error(transparent)] + Curl(#[from] curl::Error), + + /// Error from seriailzing the request payload and deserialzing the + /// response body (like response body didn't match expected structure). + #[error(transparent)] + Json(#[from] serde_json::Error), + + /// Error from IO. Mostly from reading the tarball to upload. + #[error("failed to seek tarball")] + Io(#[from] std::io::Error), + + /// Response body was not valid utf8. + #[error("invalid response body from server")] + Utf8(#[from] std::string::FromUtf8Error), + + /// Error from API response containing JSON field `errors.details`. + #[error( + "the remote server responded with an error{}: {}", + status(*code), + errors.join(", "), + )] Api { code: u32, + headers: Vec, errors: Vec, }, + + /// Error from API response which didn't have pre-programmed `errors.details`. + #[error( + "failed to get a 200 OK response, got {code}\nheaders:\n\t{}\nbody:\n{body}", + headers.join("\n\t"), + )] Code { code: u32, headers: Vec, body: String, }, - Other(anyhow::Error), -} - -impl std::error::Error for ResponseError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ResponseError::Curl(..) => None, - ResponseError::Api { .. } => None, - ResponseError::Code { .. } => None, - ResponseError::Other(e) => Some(e.as_ref()), - } - } -} -impl fmt::Display for ResponseError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - ResponseError::Curl(e) => write!(f, "{}", e), - ResponseError::Api { code, errors } => { - f.write_str("the remote server responded with an error")?; - if *code != 200 { - write!(f, " (status {} {})", code, reason(*code))?; - }; - write!(f, ": {}", errors.join(", ")) - } - ResponseError::Code { - code, - headers, - body, - } => write!( - f, - "failed to get a 200 OK response, got {}\n\ - headers:\n\ - \t{}\n\ - body:\n\ - {}", - code, - headers.join("\n\t"), - body - ), - ResponseError::Other(..) => write!(f, "invalid response from server"), - } - } -} - -impl From for ResponseError { - fn from(error: curl::Error) -> Self { - ResponseError::Curl(error) - } + /// Reason why the token was invalid. + #[error("{0}")] + InvalidToken(&'static str), + + /// Server was unavailable and timeouted. Happened when uploading a way + /// too large tarball to crates.io. + #[error( + "Request timed out after 30 seconds. If you're trying to \ + upload a crate it may be too large. If the crate is under \ + 10MB in size, you can email help@crates.io for assistance.\n\ + Total size was {0}." + )] + Timeout(u64), } impl Registry { @@ -221,10 +216,9 @@ impl Registry { } fn token(&self) -> Result<&str> { - let token = match self.token.as_ref() { - Some(s) => s, - None => bail!("no upload token found, please run `cargo login`"), - }; + let token = self.token.as_ref().ok_or_else(|| { + Error::InvalidToken("no upload token found, please run `cargo login`") + })?; check_token(token)?; Ok(token) } @@ -270,12 +264,8 @@ impl Registry { // This checks the length using seeking instead of metadata, because // on some filesystems, getting the metadata will fail because // the file was renamed in ops::package. - let tarball_len = tarball - .seek(SeekFrom::End(0)) - .with_context(|| "failed to seek tarball")?; - tarball - .seek(SeekFrom::Start(0)) - .with_context(|| "failed to seek tarball")?; + let tarball_len = tarball.seek(SeekFrom::End(0))?; + tarball.seek(SeekFrom::Start(0))?; let header = { let mut w = Vec::new(); w.extend(&(json.len() as u32).to_le_bytes()); @@ -300,18 +290,12 @@ impl Registry { let body = self .handle(&mut |buf| body.read(buf).unwrap_or(0)) .map_err(|e| match e { - ResponseError::Code { code, .. } + Error::Code { code, .. } if code == 503 && started.elapsed().as_secs() >= 29 && self.host_is_crates_io() => { - format_err!( - "Request timed out after 30 seconds. If you're trying to \ - upload a crate it may be too large. If the crate is under \ - 10MB in size, you can email help@crates.io for assistance.\n\ - Total size was {}.", - tarball_len - ) + Error::Timeout(tarball_len) } _ => e.into(), })?; @@ -410,10 +394,7 @@ impl Registry { } } - fn handle( - &mut self, - read: &mut dyn FnMut(&mut [u8]) -> usize, - ) -> std::result::Result { + fn handle(&mut self, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result { let mut headers = Vec::new(); let mut body = Vec::new(); { @@ -427,28 +408,29 @@ impl Registry { // Headers contain trailing \r\n, trim them to make it easier // to work with. let s = String::from_utf8_lossy(data).trim().to_string(); + // Don't let server sneak extra lines anywhere. + if s.contains('\n') { + return true; + } headers.push(s); true })?; handle.perform()?; } - let body = match String::from_utf8(body) { - Ok(body) => body, - Err(..) => { - return Err(ResponseError::Other(format_err!( - "response body was not valid utf-8" - ))) - } - }; + let body = String::from_utf8(body)?; let errors = serde_json::from_str::(&body) .ok() .map(|s| s.errors.into_iter().map(|s| s.detail).collect::>()); match (self.handle.response_code()?, errors) { (0, None) | (200, None) => Ok(body), - (code, Some(errors)) => Err(ResponseError::Api { code, errors }), - (code, None) => Err(ResponseError::Code { + (code, Some(errors)) => Err(Error::Api { + code, + headers, + errors, + }), + (code, None) => Err(Error::Code { code, headers, body, @@ -457,6 +439,15 @@ impl Registry { } } +fn status(code: u32) -> String { + if code == 200 { + String::new() + } else { + let reason = reason(code); + format!(" (status {code} {reason})") + } +} + fn reason(code: u32) -> &'static str { // Taken from https://developer.mozilla.org/en-US/docs/Web/HTTP/Status match code { @@ -520,7 +511,7 @@ pub fn is_url_crates_io(url: &str) -> bool { /// registries only create tokens in that format so that is as less restricted as possible. pub fn check_token(token: &str) -> Result<()> { if token.is_empty() { - bail!("please provide a non-empty token"); + return Err(Error::InvalidToken("please provide a non-empty token")); } if token.bytes().all(|b| { // This is essentially the US-ASCII limitation of @@ -531,9 +522,9 @@ pub fn check_token(token: &str) -> Result<()> { }) { Ok(()) } else { - Err(anyhow::anyhow!( + Err(Error::InvalidToken( "token contains invalid characters.\nOnly printable ISO-8859-1 characters \ - are allowed as it is sent in a HTTPS header." + are allowed as it is sent in a HTTPS header.", )) } } diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 45b7c7da5ab..eb749ee206b 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -2023,10 +2023,10 @@ fn api_other_error() { [ERROR] failed to publish to registry at http://127.0.0.1:[..]/ Caused by: - invalid response from server + invalid response body from server Caused by: - response body was not valid utf-8 + invalid utf-8 sequence of [..] ", ) .run();