Skip to content

feat(response): preserve URL when converting Response to http::Response #2798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+56 −3
Open

feat(response): preserve URL when converting Response to http::Response #2798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
925da0f
feat(response): preserve URL when converting `Response` to `http::Res…
0x676e67 Aug 14, 2025
ee68589
update
0x676e67 Aug 14, 2025
2a4facb
add test
0x676e67 Aug 14, 2025
194ebbf
fix build
0x676e67 Aug 14, 2025
4c6fc05
fix build warning
0x676e67 Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/async_impl/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,16 +484,20 @@ impl<T: Into<Body>> From<http::Response<T>> for Response {
// It's supposed to be the inverse of the conversion above.
impl From<Response> for http::Response<Body> {
fn from(r: Response) -> http::Response<Body> {
use crate::response::ResponseUrl;

let (parts, body) = r.res.into_parts();
let body = Body::wrap(body);
http::Response::from_parts(parts, body)
let mut response = http::Response::from_parts(parts, body);
response.extensions_mut().insert(ResponseUrl(*r.url));
response
}
}

#[cfg(test)]
mod tests {
use super::Response;
use crate::ResponseBuilderExt;
use crate::{ResponseBuilderExt, ResponseExt};
use http::response::Builder;
use url::Url;

Expand All @@ -510,4 +514,23 @@ mod tests {
assert_eq!(response.status(), 200);
assert_eq!(*response.url(), url);
}

#[test]
fn test_from_http_response_with_url() {
let url = Url::parse("http://example.com").unwrap();
let response = Builder::new()
.status(200)
.url(url.clone())
.body("foo")
.unwrap();
let response = Response::from(response);

assert_eq!(response.status(), 200);
assert_eq!(*response.url(), url);

let http_response = http::Response::from(response);
let resp_url = http_response.url();
assert_eq!(http_response.status(), 200);
assert_eq!(resp_url, Some(&url));
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ mod response;

pub use self::error::{Error, Result};
pub use self::into_url::IntoUrl;
pub use self::response::ResponseBuilderExt;
pub use self::response::{ResponseBuilderExt, ResponseExt};

/// Shortcut method to quickly make a `GET` request.
///
Expand Down
30 changes: 30 additions & 0 deletions src/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use url::Url;

use crate::Body;

#[derive(Debug, Clone, PartialEq)]
pub(crate) struct ResponseUrl(pub Url);

Expand All @@ -12,12 +14,26 @@ pub trait ResponseBuilderExt {
fn url(self, url: Url) -> Self;
}

/// Extension trait for http::Response objects
///
/// Provides methods to extract URL information from HTTP responses
pub trait ResponseExt {
Copy link
Owner

@seanmonstar seanmonstar Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking it might be better to leave this off. Is there a need for it that I'm overlooking?

Copy link
Contributor Author

@0x676e67 0x676e67 Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did run into some scenarios where I needed to convert a reqwest::Response into an http::Response while preserving the original URL. That said, as you pointed out, it's always possible to manually clone the URL using the url() method on reqwest::Response. While that approach is a bit more verbose, the only benefit of this change would be to avoid one additional clone, which probably isn't significant in practice.

So I agree with your suggestion—this functionality isn't strictly necessary at the moment. I'm fine with closing this PR, and if a stronger need arises in the future, we can revisit it then.

Copy link
Owner

@seanmonstar seanmonstar Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I don't mean we need to close the whole thing... I actually have for a while thought we should just store the url in the extensions instead of directly on the struct...

Copy link
Contributor Author

@0x676e67 0x676e67 Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, it's still possible to get the URL with an implementation like this:

impl From<Response> for (Url, http::Response<Body>) {
    fn from(r: Response) -> (Url, http::Response<Body>) {
        let (parts, body) = r.res.into_parts();
        let body = Body::wrap(body);
        let response = http::Response::from_parts(parts, body);
        (*r.url, response)
    }
}

/// Returns a reference to the `Url` associated with this response, if available.
fn url(&self) -> Option<&Url>;
}

impl ResponseBuilderExt for http::response::Builder {
fn url(self, url: Url) -> Self {
self.extension(ResponseUrl(url))
}
}

impl ResponseExt for http::Response<Body> {
fn url(&self) -> Option<&Url> {
self.extensions().get::<ResponseUrl>().map(|r| &r.0)
}
}

#[cfg(test)]
mod tests {
use super::{ResponseBuilderExt, ResponseUrl};
Expand All @@ -38,4 +54,18 @@ mod tests {
Some(&ResponseUrl(url))
);
}

#[cfg(not(target_arch = "wasm32"))]
#[test]
fn test_response_ext() {
use super::ResponseExt;
let url = Url::parse("http://example.com").unwrap();
let response = http::Response::builder()
.status(200)
.extension(ResponseUrl(url.clone()))
.body(crate::Body::empty())
.unwrap();

assert_eq!(response.url(), Some(&url));
}
}
Loading