Skip to content
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.

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

Improve interoperability with http #2060

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Conversation

shouya
Copy link
Contributor

@shouya shouya commented Dec 8, 2023

I made a few changes to make reqwest types compatible with http types. This will enable a lot of interesting uses (such as a reverse proxy, example code attached in the end).

By interoperability with http I mean the following four components:

  • impl TryFrom from http::Request<http::Body> to reqwest::Request (already implemented prior to this PR)
  • impl TryFrom from reqwest::Response to http::Response<reqwest::Body>
  • impl http::Body for reqwest::Body
  • impl tower::Service<reqwest::Request, Response=reqwest::Response, Error=reqwest::Error> for reqwest::Client (also already implemented prior to this PR)

In fact, all the implementation logic is already there. I only assembled these together.

Now with all these trait implementations and conversion functions, it becomes easy to play with axum and other http-based frameworks. Here's an example reverse proxy written in axum:

use axum::Router;
use tower::{Layer, ServiceExt};

#[tokio::main(flavor = "current_thread")]
async fn main() {
  let client = reqwest::Client::new();
  let example_proxy = client
    .map_request(|r| reqwest::Request::try_from(r).unwrap())
    .map_response(|r| http::Response::try_from(r).unwrap())
    .map_err(|e| panic!("axum requires this to be infallible {:?}", e));
  let rewrite_req_url = axum::middleware::map_request(rewrite_req_url);

  let example_proxy = rewrite_req_url.layer(example_proxy);

  let app = Router::new().nest_service("/example", example_proxy);
  let service = app.into_make_service();

  axum::Server::bind(&"0.0.0.0:3000".parse().unwrap())
    .serve(service)
    .await
    .unwrap();
}

async fn rewrite_req_url<B>(mut req: http::Request<B>) -> http::Request<B> {
  let uri = format!("https://example.com{}", req.uri());
  req.headers_mut().insert(
    http::header::HOST,
    http::header::HeaderValue::from_static("example.com"),
  );
  *req.uri_mut() = uri.parse().unwrap();
  req
}

I noticed that the wrapper ImplStream is not really used anywhere, so I removed it. In my opinion, it is sensible to interpret a reqwest::Body as a Stream or a http::Body on its own. Please let me know if there are considerations where it's still necessary.

@seanmonstar
Copy link
Owner

Thank you for putting this together! I do want to improve the type interoperability with http. I'm also working on a draft PR to upgrade reqwest to hyper v1, which has my current attention. I do hope to include this for the next release, though.

@seanmonstar
Copy link
Owner

Would you like to try to rebase this on to the 0.12-dev branch? I'm hope to prep a release this week. :)

@shouya shouya changed the base branch from master to 0.12-dev March 18, 2024 23:58
@shouya shouya force-pushed the master branch 2 times, most recently from 601a507 to 0699394 Compare March 19, 2024 12:27
@shouya
Copy link
Contributor Author

shouya commented Mar 19, 2024

@seanmonstar: Updated. I found that of the conversions and trait implementations I initially proposed, only the conversion from reqwest::Response to http::Response<reqwest::Body> is still needed.

As for the part for treating reqwest::Body as Stream<Item=Bytes>, I think it may be unnecessary. Given that it's easy enough to get a stream out of a body with a http_body_util::BodyStream, so I removed the relevant code. I can add it back if you think it's good to have.

Please have a look.

@seanmonstar seanmonstar merged commit c4c96cf into seanmonstar:0.12-dev Mar 19, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-breaking-change Blocked: breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants