feat: add unix_socket() option to client builder

[seanmonstar]

Adds a new option, ClientBuilder::unix_socket(path), if set, will force all connections to use that instead of TCP. TLS works as expected. Thinking on my own experience, if I need to use unix sockets, I'm not usually using the same configured client for external requests too. This way provides a useful easy option, and we're slowly working on making reqwest more modular.

Closes #39

Background (originally at #39 (comment))

Using a proxy felt right from an implementation point of view, but it's not really a proxy. The more I thought about it, the more it felt kinda-right-but-just-wrong. It would also complicate the work outlined in hyperium/hyper#3850.

The most powerful option would be to just allow users to provide a completely custom connector (some sort of impl Connect) that returns any impl Read + Write. However, it has a couple downsides. It's more complicated for a user, and reqwest aims to be easy for the user. It also raises questions of whether you consider the pool and TLS "part of a connector", so does a custom one replace those, or only the TCP portion (which feels less powerful), and if so, what does the config of those types do then? Most influential to my decision: we can always add this later, for more power. And anyone who needs it can built a custom client stack right now (see https://seanmonstar.com/blog/modular-reqwest/).

Next up I considered an option on the RequestBuilder, which felt quite elegant. Calling RequestBuilder::unix_socket("/tmp/foo.sock") would stick a connect::Uds type in the request extensions. However, the current implementation internals prevent that. That's because the hyper-util legacy Client connectors are only provided a Uri, it cannot check extensions. The new version of hyper-util's pool and Connect types could be changed to accept &Request instead of just Uri (hyperium/hyper#3849).

But that means we either use custom URIs, or configure the connector at ClientBuilder time. I don't like any of the custom URIs, since none of them are standardized. Yes, I've looked around. No, I don't like it.

So, this is the current proposed solution.

[MoreTore]

Perfect timing for me as I was just looking for the solution. Question though, if you create a client like this:

    let client = reqwest::Client::builder()
        .unix_socket(server.path())
        .build()
        .unwrap();
        
   let unix_res = client .get("http://yolo.local/foo")
        .send()
        .await
        .expect("send request");

can i still reuse the client with like this?

  let ip_res = client .get("https://google.com")
        .send()
        .await
        .expect("send request");

[seanmonstar]

No, if set, all connections use the socket. I've updated the top to quote the rationale from the linked issue. We could in the future come up with a design that allows both, but it requires coming up with how to intuitively configure that.

[MoreTore]

No, if set, all connections use the socket. I've updated the top to quote the rationale from the linked issue. We could in the future come up with a design that allows both, but it requires coming up with how to intuitively configure that.

Haven't looked at any code but I would assume this can be determined by looking at the socket address protocol.

Like maybe instead of using http:// you could use ipc:// ?

[dblsaiko]

Like maybe instead of using http:// you could use ipc:// ?

I don't like that, as the higher layer protocol is still HTTP, only TCP gets replaced, so this would be assuming HTTP, which would be annoying for connecting to non-HTTP over a socket (perhaps HTTPS for example, or potential future protocols). http+unix:// could work though, similar to the "git+https" URLs you sometimes see.

[MoreTore]

Like maybe instead of using http:// you could use ipc:// ?

I don't like that, as the higher layer protocol is still HTTP, only TCP gets replaced, so this would be assuming HTTP, which would be annoying for connecting to non-HTTP over a socket (perhaps HTTPS for example, or potential future protocols). http+unix:// could work though, similar to the "git+https" URLs you sometimes see.

yea im not very rehersed with all this stuff. All i know is that i need to be able to switch between unix socket at http at runtime. I can make my own implementation to do this based on this PR but i would be making my own interface which i would rather not.

With zmq, you use a leading '/' to indicate an absolute socket path. like

'ipc:///tmp/zmqtest'

maybe this is a good way to it? so in reqwests case it would be like:

    let client = reqwest::Client::builder()
        .unix_socket(server.path())
        .build()
        .unwrap();
        
   let unix_res = client.get("https:///local/foo.socket")
        .send()
        .await
        .expect("send request");

  let ip_res = client.get("https://google.com")
        .send()
        .await
        .expect("send request");

although you may want to use relative paths for some reason, im not sure if thats a good way to do it.

[dblsaiko]

Actually no, this won't work at all, what I said won't either. The actual request URL still has to be extracted somehow, and ideally the Host header has to be set. If you do something like that, neither will be (easily) possible.

Perhaps get() could take a type that specifies a socket to connect to instead of trying to resolve the address in the request URL. E.g. client.get(reqwest::unix("http://localhost/foo", "/tmp/myservice.sock")). Like how you do it with cURL: curl --unix-socket /tmp/myservice.sock http://localhost/foo.

EDIT: seems like this is actually very close to the second proposed alternative in the OP. :)

[MoreTore]

Actually no, this won't work at all, what I said won't either. The actual request URL still has to be extracted somehow, and ideally the Host header has to be set. If you do something like that, neither will be (easily) possible.

Perhaps get() could take a type that specifies a socket to connect to instead of trying to resolve the address in the request URL. E.g. client.get(reqwest::unix("http://localhost/foo", "/tmp/myservice.sock")). Like how you do it with cURL: curl --unix-socket /tmp/myservice.sock http://localhost/foo.

EDIT: seems like this is actually very close to the second proposed alternative in the OP. :)

Yea that would work for me!

[seanmonstar]

Those methods do take a impl IntoUrl, which is sealed. So we could add another sealed method, and do what you suggested. Could be nice.

The other existing problem right now is that the connection pool implementation keys connections based on Uri, so pooling wouldn't work right no unless a custom scheme was used, which I don't want to do. So the pool would need to be refactored to allow defining a different key type. That likely will happen in hyperium/hyper#3849

[seanmonstar]

Besides that, you could use this implementation currently and just use 2 Clients, one for local and one for external connections.

[orium]

@seanmonstar Hi. Is there any concerns about this PR that you still need to solve, or is this something that is just waiting to land when a new minor reqwest is ready to be released?

[seanmonstar]

I initially started coding on this at request of someone with plans to use it. Before merging, I wanted to see if it ended up serving the needs, or if through usage there were some changes needed. So, currently waiting.

[bim9262]

It works great for the project I'm working on (talking to docker over a unix socket)!

[dgagn]

This fits my usecase !

[kernelmethod]

I'm writing up a test suite for a server listening to HTTP over a Unix socket and this works like a charm. The current behavior of the ClientBuilder API with respect to the new .unix_socket method works fine for my use case.

[rgrandl]

Is it possible to merge this PR? We would love to use this feature as well.

[Erik-Sovereign]

I try to use this branch in my test but I'm confused on how to reach the unix socket. I show you my code:

In the prepare_test function I create the unix socket, put its path in a config and put that in my app. I also return it as part of TestParameters:

let unix_socket_temp_file = NamedTempFile::new().unwrap();

let listen_address = if use_unix_socket {
	ListenAdress::UnixSocket(unix_socket_temp_file.path().to_path_buf())
} else {
	ListenAdress::TcpSocket(url.parse().unwrap())
};

let config = Arc::new(AuthRequestBackendConfig {
listen_address,
...
}

let handler = tokio::spawn(async {
	App::new(config).await.unwrap().serve().await.unwrap();
});

let client = if use_unix_socket {
	ClientBuilder::new()
		.cookie_store(true)
		.unix_socket(unix_socket_temp_file.path())
		.build()
		.unwrap()
} else {
	ClientBuilder::new().cookie_store(true).build().unwrap()
};

TestParameters {
	client,
        unix_socket_temp_file,
        ...
 }

I use that in my app like this:

match &self.config.listen_address {
	crate::config::ListenAdress::UnixSocket(path) => {
		// Create the socket file if it does not exist. We don't want to use `create` here, because this will truncate the file contents if the file already exists.
		match OpenOptions::new().write(true).create_new(true).open(path) {
			Ok(_) => (),
			Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {
				info!("Socket file for incoming TCP requests already exists, proceeding with the existing socket.");
			}
			Err(e) => {
				return Err(Box::new(e));
			}
		}

		let listener = UnixListener::bind(path)?;

		axum::serve(listener, app.into_make_service()).await?;
	}

But I'm now confused on how to use it in my test. Here you see how I have written my tests beforehand with a classical url. I don't know how I can now target the socket-path with my request:

#[tokio::test]
#[traced_test]
async fn e2e_unix_socket_request() {
let test_parameters = prepare_test(None, None, true).await;

let response: Vec<String> = test_parameters
	.client
	.post(format!("{}/login", test_parameters.url_base))
	.form(&LoginCredentials {
		username: test_parameters.name_of_initial_user.clone(),
		password: test_parameters.plain_password.clone(),
		url: Some("/rest/roles".to_string()),
	})
	.send()
	.await
	.unwrap()
	.json()
	.await
	.unwrap();

assert_eq!(response, test_parameters.roles_of_initial_user);
}

[seanmonstar]

I don't know how I can now target the socket-path with my request:

If you constructed a Client with the option, all requests will use that socket path. It all happens at ClientBuilder time.

[Erik-Sovereign]

Thanks. It now works for me. I had to understand, that the url-host you give to reqwest doesn't matter to a unix socket, but you still have to give one. It also only works with files for me, that I didn't create but that are created by tokios UnixListener.
Working code looks like this for me:

let temp_dir = std::env::temp_dir();
let random_file_name = format!("test_{}.sock", rand::random::<u32>());
let path = temp_dir.join(random_file_name);

let listen_address = ListenAdress::UnixSocket(path.clone());

let client = ClientBuilder::new().cookie_store(true).unix_socket(path).build().unwrap();

let listener = UnixListener::bind(path)?;

axum::serve(listener, app.into_make_service()).await?;

let response: Vec<String> = test_parameters
	.client
	.post("http://uri-doesnt-matter.com/login")
	.form(&LoginCredentials {
		username: test_parameters.name_of_initial_user.clone(),
		password: test_parameters.plain_password.clone(),
		url: Some("/rest/roles".to_string()),
	})
	.send()
	.await
	.unwrap()
	.json()
	.await
	.unwrap();

[morrisonlevi]

It would be convenient if there was a unix_socket function for the blocking ClientBuilder as well.

[morrisonlevi]

I did a proof-of-concept of using this in the Datadog PHP profiler. It's awkward to have to construct a URL, but I assume it's because:

1. Reqwest still needs to know if it's http or https.
2. Reqwest still needs the URL path e.g. /profiling/v1/input.

But for now, using http://apm.socket/profiling/v1/input as my URL when UNIX sockets are used works.

[seanmonstar]

@morrisonlevi thanks for the feedback! Yep, those two pieces, and also: it needs to construct a host/:authority header if it's at all possible to do so. And many servers don't care what transport was used to connect to them, they will still check the host.

[danielsn]

Are there any remaining blockers to merging this? Wondering when we can plan to start using this?

[seanmonstar]

Nope, no blockers, thanks for all the feedback!