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

Can't use relative URLs for making requests #988

Open
thomaseizinger opened this issue Aug 5, 2020 · 20 comments · May be fixed by #1620
Open

Can't use relative URLs for making requests #988

thomaseizinger opened this issue Aug 5, 2020 · 20 comments · May be fixed by #1620

Comments

@thomaseizinger
Copy link

When trying to pass a relative URL to any of the functions like .get, it errors with RelativeUrlWithoutBase. From my understanding, this is triggered by the url crate because Url::parse only accepts absolute URLs.

My expectation would have been that I can set the Host header in the default header map on the client instance and later on use relative URLs to actually make requests.

Is this something that is planned to be supported?

@seanmonstar
Copy link
Owner

That's an interesting idea! Are there other clients that support that? Also, the full URL gives the scheme as well, so we know whether to use HTTP or HTTPS, and the Host header doesn't typically have that information.

@thomaseizinger
Copy link
Author

thomaseizinger commented Aug 5, 2020

Are there other clients that support that?

I haven't done any research on that.

[...] the Host header doesn't typically have that information.

To be clear, I don't mind actually where we get the base URL and all the other required information from :)
The Host header is just the first thing I could think of that might work within the current design of the lib.

For my usecase (a client-agnostic JSON-RPC library but with optional support for several clients), I want to make requests with a client that is passed in by the user like this: https://github.com/thomaseizinger/rust-jsonrpc-client/blob/custom-derive-send-request/lib/examples/reqwest_blocking.rs

Given that the user already controls the client instance, it would be nice if there would be some way of how the user can control the endpoint that the requests should be sent to without me having to worry about that.

@seanmonstar
Copy link
Owner

As a possible option that doesn't require changing reqwest, you could have a base URL the user can configure, and then join on it, perhaps?

@thomaseizinger
Copy link
Author

As a possible option that doesn't require changing reqwest, you could have a base URL the user can configure, and then join on it, perhaps?

SendRequest would ideally be derived by the user and forward to the impl I linked above. With reqwest's current design, this impl is the only place where I can pass the full URL.

I guess I could make the user provide a 2nd field in the struct they are deriving SendRequest on that contains the base URL 🤔

#[derive(jsonrpc_client::SendRequest)]
struct Client {
    inner: reqwest::blocking::Client,
	base: Url
}

I'll explore this direction and see what I can do. Thanks :)

@707090
Copy link
Contributor

707090 commented Aug 14, 2020

Another independent use case for this is for the WASM browser target. It would be convenient for clients to be able to specify relative URLs from the host they are on

@707090
Copy link
Contributor

707090 commented Aug 17, 2020

I dont know a super bunch about how the crate configures TLS and stuff, but I do see that the client has a local_address field which (presumably) defines the address of the local server. Would it make sense to reuse that as the base of any relative URLs?

If the goal is to specify a non-local address as the base I disagree with the premise. As I mentioned in #1004, in my opinion, I think this crate already blurs the line between what a request is responsible for and what a client is responsible for (specifically relevant I mentioned in there I dont think Client should have default_headers at all). I think specifying where this client is (if that is the correct interpretation of the local_address config) should live on a client, and it can interpret relative URLs from that. This also has a nice parallel with the WASM (browser) implementation being the current webpage host.

If you want specify relative URLs from an arbitrary base, then that should be done outside of reqwest, and supplied to the RequestBuilder IMO. Pending a PR for the changes listed in #1004, the solution for this would be to (in pseudocode, not exact):

let base_url = Url::parse("yourbaseurl.com/");
let base_builder = RequestBuilder::get(base_url).header(*put what is currently default_headers*);
let new_url = base_url.join("/some/path");
let my_request = base_builder.clone().url(new_url).send(&Client::new());

Thoughts @seanmonstar?

@seanmonstar
Copy link
Owner

The local_address is to specify what socket address to bind before starting the connect call. This is useful when you have multiple addresses that can refer to the same machine.

@707090
Copy link
Contributor

707090 commented Aug 17, 2020

Ok, so what my thought was the default interpretation of a relative URL is more like the loopback address (in a non-browser context)

@Lesik
Copy link

Lesik commented Jan 16, 2021

Are there other clients that support that?

request, the major node (javascript) HTTP client library, does.

@felipero
Copy link

felipero commented Mar 7, 2021

This would be cool to have.

@dbanty
Copy link

dbanty commented Jan 19, 2022

FWIW I was just searching for a similar feature for integration tests, thought being that instead of creating a client in each test and passing an address, I could just generate a client which points at the locally running server and pass that to each test. Not a big deal, but would reduce a couple of lines of boilerplate per test.

Instead of this:

let address = spawn_server();
let client = reqwest::Client::new();

let response = client.get(format!("{address}/some-path"));

I could have this:

let client = spawn_server();

let response = client.get("/some-path");

Although in reality it'd probably be a reusable rtest fixture so I can make my Rust server tests as easy to write as FastAPI tests. And that brings me to my answer to this:

Are there other clients that support that?

Coming from the Python world, httpx has this feature. It's the most popular of the "modern" (i.e., async capable) request libraries.

In the Rust world, surf supports this syntax as well. I don't think any of the others I've tried do.

@guccialex
Copy link

guccialex commented Feb 22, 2022

on wasm, for yew or other, you can get the base url (https://youraddress.com) with

let baseurl = web_sys::window().unwrap().origin();

@maxcountryman
Copy link

reqwasm also supports relative urls.

@mellowagain
Copy link

Are there other clients that support that?

surf allows one to specify a base_url in the client config builder and then allows relative urls in requests made using that client.

@TravisWhitehead
Copy link

TravisWhitehead commented Oct 7, 2022

Axum's TestClient helper (for integration testing) might be a useful example for anyone looking for a simple wrapper around some of Client's methods to allow for relative URLs, but this is opinionated towards Axum integration testing:

https://github.com/tokio-rs/axum/blob/main/axum/src/test_helpers/test_client.rs

And if you happen to be specifically looking for a solution for Axum integration testing, it's also available in a crate: https://crates.io/crates/axum-test-helper

@melgish
Copy link

melgish commented Dec 17, 2022

New to rust and was surprised this didn't already exist. Axios, a JavaScript library, supports this when creating a custom instance. You pass the base to the constructor and any relative paths for that instance will be qualified against it. It also does this for default headers.

This way if you have an API that requires a key or needs special headers, you set this once when creating your client, and never have to deal with it again.

@dfaust dfaust linked a pull request Dec 20, 2022 that will close this issue
@WillenOLeal
Copy link

Here is a way around using relative paths

use reqwest::{header, Response, Error};
use serde::{Deserialize, Serialize};

pub struct HttpClient {
    base_url: String,
    headers: header::HeaderMap,
   pub client: reqwest::Client,
}

impl HttpClient {
    pub fn new(base_url: &str, headers: header::HeaderMap) -> Result<Self, Error> {
        let client = reqwest::Client::builder().build()?;
        Ok(Self {
            base_url: base_url.to_owned(),
            headers,
            client,
        })
    }

    pub async fn get<T>(&self, path: &str) -> Result<T, Error>
    where
        T: for<'de> Deserialize<'de>,
    {
        let url = format!("{}{}", self.base_url, path);
        let response = self.client.get(&url).headers(self.headers.clone()).send().await?;
        let json = response.json::<T>().await?;
        Ok(json)
    }

    pub async fn post<T>(&self, path: &str, body: &T) -> Result<Response, Error>
    where
        T: Serialize,
    {
        let url = format!("{}{}", self.base_url, path);
        let resp = self.client
            .post(&url)
            .headers(self.headers.clone())
            .json(body)
            .send()
            .await?;

        Ok(resp)
    }
}

@kontsaki
Copy link

kontsaki commented Apr 9, 2023

Hello,

python httpx supports base_url on the Client builder

https://www.python-httpx.org/advanced/#other-client-only-configuration-options

@JosephLenton
Copy link

Hey, this is also a blocking issue for getting Reqwest support into Axum Test. If there is interest to add this, I'd be happy to put some time aside to look into making a PR for Reqwest.

@JosephLenton
Copy link

Hey @seanmonstar I have opened a PR which implements this with more reliance on Url. Given there are now two PRs open, and lots of chatter on this issue, I would be eager to get one of these merged.

If you can please find time to have a look, if there are issues I am free to help on getting them resolved over the next few weeks. I don't care if it's my, @thomasqueirozb's PR, or a third option that is merged. I am happy to help on any of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.