ninenines / gun

HTTP/1.1, HTTP/2, Websocket client (and more) for Erlang/OTP.
ISC License
898 stars 231 forks source link

Support for HTTP over Unix domain sockets #135

Closed sanmiguel closed 6 years ago

sanmiguel commented 6 years ago

As mentioned in #134 it's not that easy to use gun to make HTTP requests over a unix domain socket, but this branch aims to add that support. Please let me know if this approach is not one you would take and we can rework it.

I'd also like to find out how you would like to see this tested.

TODO:

essen commented 6 years ago

Sounds good.

For the test I suppose creating a socket, connecting to it/sending a request and being able to receive something on the server side is enough. No need to parse or anything, just need to be able to connect and send stuff. Should be in gun_SUITE.

I'm guessing the test will need to be disabled in Windows.

For nitpicks I prefer do_open rather than open_, took me a little time to figure it out. open_unix should be a one liner call to do_open rather than creating variables.

sanmiguel commented 6 years ago

@essen I've added a test case unix_socket_connect/1, but there's one detail I want to change: it uses a hard-coded path to the socket of /tmp/gun.sock which is obviously not ideal.

I'd like to use something from within CT's structure, but there's not already a test/gun_SUITE_data dir. Any thoughts on how you'd like it resolved?

I could do something like:

DataDir = config(data_dir, Config),
SocketPath = filename:join(DataDir, "gun.sock"),
filelib:ensure_dir(SocketPath),
_ = file:delete(SocketPath),

What do you reckon?

essen commented 6 years ago

The data_dir is for files that are supposed to be committed to the repo, probably not what you want. I believe the one you want is priv_dir, which should be generated automatically per test run (the log_private/ directory deep in the CT logs).

sanmiguel commented 6 years ago

Ah, this is going to be fun... Was getting {error, einval} when using priv_dir instead...

From man unix on my mac:

UNIX-domain addresses are variable-length filesystem pathnames of at most 104 characters.

Current length of path to priv_dir including socket name: 163

How strongly would you object to using e.g. test_server:temp_name("/tmp/gun") to get the directory name?

Also, how do we go about disabling this test for Windows?

essen commented 6 years ago

Oh then no problem just go with /tmp.

Just do something like https://github.com/ninenines/cowboy/blob/master/test/req_SUITE.erl#L163 except over case os:type() of {win32, _} -> ...; _ -> ... end.

sanmiguel commented 6 years ago

Cool thanks, I'll get it done in the morning. :+1:

On Thu, 7 Dec 2017, 22:38 Loïc Hoguin, notifications@github.com wrote:

Oh then no problem just go with /tmp.

Just do something like https://github.com/ninenines/cowboy/blob/master/test/req_SUITE.erl#L163 except over case os:type() of {win32, } -> ...; -> ... end.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ninenines/gun/pull/135#issuecomment-350102181, or mute the thread https://github.com/notifications/unsubscribe-auth/ACBkRr-pd9xhqquAPmBS0FhObxTAu_cdks5s-FrpgaJpZM4Q5pG4 .

sanmiguel commented 6 years ago

@essen this is now ready for you to look at! 👍

dcgibbons commented 6 years ago

eagerly awaiting this one! 👍

essen commented 6 years ago

Merged, thanks!

sanmiguel commented 6 years ago

Thanks!