Repository: seanmonstar/reqwest (language: rust)

Issue:
Display Error without url
I would like to show error (Display) but to remove or change an url, because it contains sensitive info (API key). Is there any easy way to do this?


Discussion / hints:
Hi!

I'm having the same problem.

For now I'm using this dirty trick:

```rust
let url = String::from("...");
let res = reqwest::get(url.as_str())
    .and_then(|r| r.error_for_status())
    .map_err(|e| display_http_error(e, url))

fn display_http_error(e: reqwest::Error, url: String) -> String {
    e.to_string().replace(format!("{}: ", url).as_str(), "")
}
```

It works because I still have access to the URL string, but it is not a nice solution...
Seems like a fair thing to want to do. What seems like a good way to do this? Things that come to mind:

- Add some `without_url` method that returns some `impl Display` reference, kind of like how the `time` crate provides formatters.

  This would help directly when wanting to format, and could be done with references.
- Add `without_url` that consumes the error and drops the URL field completely.

  This would be useful to remove the URL once and prevent forgetting about it in other places. It couldn't be done with a reference, though.

There's probably other ways too, if you have an idea.
I'm really new to Rust so I don't know yet all the consequences of these options...

Maybe another option is to expose (in different namespaces) 2 implementations of the `Display` trait for the `Error` struct.
I believe this would be possible in Haskell and users could choose to import one or another into the current scope, but I don't know if it possible in Rust or if it is a nice pattern or not...

Anyway, even a simple `without_url(&self) -> String` method on the `Error` struct would be nice. I didn't manage to make one from outside the lib, but maybe there is way (?)
@dsferruzza what you describe is the same as my first suggestion: have a secondary `Display` returned by a method on `Error`. It'd be usable like this:

```rust
let error = got_some_reqwest_error();

// Output: "http://httpbin.org/redirect/11: Too many redirects"
println!("{}", error);


// Output: "Too many redirects"
println!("{}", error.without_url());
```
`fn without_url(&self) -> String` should have similar implementation as `Display for reqwest::Error` has.

What about `fn without_url(&self) -> Cow<Self>`? A method of `Error` which removes url.
> What about fn without_url(&self) -> Cow<Self>? A method of Error which removes url.

I'm assuming the method would return `Cow::Owned` with a clone of the error if the URL was removed? If it is indeed possible to clone `reqwest::Error`, I think it's better to implement the Clone trait for Error and make `without_url` take self by value.

As far as I can tell, these are all viable options (I added the last one).
```rust
/// Remove URL information from error and return modified error
pub fn without_url(self) -> Self;
/// Generate a printable object which shows the error without URL information
pub fn without_url(&self) -> impl Display;
/// Remove URL information from this error
pub fn delete_url(&mut self);
```
Hi all!

I'm coming up against this same issue (with an API key in the query). @seanmonstar /@kangalioo, is there any plan to merge #1299? Otherwise, I'd propose a PR with the following API which probably covers the gamut of the above:
```rust

/// Mutative methods

/// Get a mutable reference to the url, if present (for example, if you wanted to modify the url to remove the sensitive information
pub fn url_mut(&mut self) -> Option<&mut Url>;
/// Remove the url entirely
pub fn clear_url(&mut self) -> &mut Self;
/// Strip the url and return self (similar to `clear_url`, but possibly more ergonomic in some cases)
pub fn without_url(mut self) -> Self;

/// Format Structs (where each return wrapper implements Display)

/// Return a wrapper struct (borrowing self) that masks the url in the display impl (but still shows that it exists
pub fn as_url_masked(&self) -> UrlMaskedError<'_>;
/// Return a wrapper struct (borrowing self) that ignores the url field
pub fn as_url_hidden(&self) -> UrlHiddenError<'_>;
/// Return a wrapper struct (borrowing self) that applies a custom formatting function to the url on display.
pub fn with_url_formatter<F: Fn(&Url, f: &mut std::fmt::Formatter) -> std::fmt::Result>(&self, f: F) -> UrlFormattedError<'_, F>;

```

And, since we're allowing mutation of the `url` field, it may make sense to make `Error::with_url`, which already exists, public.

Fix patch:
diff --git a/src/error.rs b/src/error.rs
index fb73de97f..45a5c063c 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -9,6 +9,10 @@ use crate::{StatusCode, Url};
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// The Errors that may occur when processing a `Request`.
+///
+/// Note: Errors may include the full URL used to make the `Request`. If the URL
+/// contains sensitive information (e.g. an API key as a query parameter), be
+/// sure to remove it ([`without_url`](Error::without_url))
 pub struct Error {
     inner: Box<Inner>,
 }
@@ -56,6 +60,28 @@ impl Error {
         self.inner.url.as_ref()
     }
 
+    /// Returns a mutable referene to the URL related to this error
+    ///
+    /// This is useful if you need to remove sensitive information from the URL
+    /// (e.g. an API key in the query), but do not want to remove the URL
+    /// entirely.
+    pub fn url_mut(&mut self) -> Option<&mut Url> {
+        self.inner.url.as_mut()
+    }
+
+    /// Add a url related to this error (overwriting any existing)
+    pub fn with_url(mut self, url: Url) -> Self {
+        self.inner.url = Some(url);
+        self
+    }
+
+    /// Strip the related url from this error (if, for example, it contains
+    /// sensitive information)
+    pub fn without_url(mut self) -> Self {
+        self.inner.url = None;
+        self
+    }
+
     /// Returns true if the error is from a type Builder.
     pub fn is_builder(&self) -> bool {
         matches!(self.inner.kind, Kind::Builder)
@@ -128,11 +154,6 @@ impl Error {
 
     // private
 
-    pub(crate) fn with_url(mut self, url: Url) -> Error {
-        self.inner.url = Some(url);
-        self
-    }
-
     #[allow(unused)]
     pub(crate) fn into_io(self) -> io::Error {
         io::Error::new(io::ErrorKind::Other, self)
@@ -158,18 +179,6 @@ impl fmt::Debug for Error {
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        struct ForUrl<'a>(Option<&'a Url>);
-
-        impl fmt::Display for ForUrl<'_> {
-            fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-                if let Some(url) = self.0 {
-                    write!(f, " for url ({})", url.as_str())
-                } else {
-                    Ok(())
-                }
-            }
-        }
-
         match self.inner.kind {
             Kind::Builder => f.write_str("builder error")?,
             Kind::Request => f.write_str("error sending request")?,
@@ -187,9 +196,11 @@ impl fmt::Display for Error {
             }
         };
 
-        ForUrl(self.inner.url.as_ref()).fmt(f)?;
+        if let Some(url) = &self.inner.url {
+            write!(f, " for url ({})", url.as_str())?;
+        }
 
-        if let Some(ref e) = self.inner.source {
+        if let Some(e) = &self.inner.source {
             write!(f, ": {}", e)?;
         }
 


Test patch: