Auto merge of #12310 - weihanglo:asymmetric-challenge, r=Eh2406

feat(crates-io): expose HTTP headers and Error type

### What does this PR try to resolve?

This is part of #11521.

[RFC 3231] mentions the authentication process could have an additional **challenge-response mechanism** to prevent potential replay attacks. The challenge usually comes from HTTP `www-authenticate` header as a opaque string. When a client gets a 401/403 response with such a challenge, it may attach the `challenge` to the payload and request again to anwser the challenge.

```
➡️ cargo  requests
⬅️ server responds with `www-authenticate` containing some opaque challenge string
➡️ cargo  automatically requests again without any user perception
⬅️ server responds ok
```

However, `crates-io` crate doesn't expose HTTP headers. There is no access to `www-authenticate` header.

This PR make it expose HTTP headers and the custom `Error` type, so `cargo` can access and do further on the authentication process.

[RFC 3231]: https://rust-lang.github.io/rfcs/3231-cargo-asymmetric-tokens.html#the-authentication-process

Author: bors

Cargo.lock

@@ -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"

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

crates/crates-io/Cargo.toml

@@ -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<T> = std::result::Result<T, Error>;
+
 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<String>,
         errors: Vec<String>,
     },
+
+    /// 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<String>,
         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<curl::Error> 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 [email protected] 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 [email protected] 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<String, ResponseError> {
+    fn handle(&mut self, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result<String> {
         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::<ApiErrorList>(&body)
             .ok()
             .map(|s| s.errors.into_iter().map(|s| s.detail).collect::<Vec<_>>());
 
         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.",
         ))
     }
 }

crates/crates-io/lib.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();