-
Notifications
You must be signed in to change notification settings - Fork 983
Add Unix domain socket support #637
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
Conversation
@rschmitt It is a big one. Please allow me a few days to get around to it. |
I suggest reading the diff from the bottom up. It just flows more logically that way: example code, connection changes, routing changes, integration test changes. |
.../src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java
Outdated
Show resolved
Hide resolved
...esting/src/test/java/org/apache/hc/client5/testing/extension/sync/UnixDomainProxyServer.java
Show resolved
Hide resolved
.../src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/socket/UnixDomainSocketFactory.java
Outdated
Show resolved
Hide resolved
httpclient5/src/main/java/org/apache/hc/client5/http/socket/UnixDomainSocketFactory.java
Outdated
Show resolved
Hide resolved
httpclient5/src/test/java/org/apache/hc/client5/http/TestHttpRoute.java
Outdated
Show resolved
Hide resolved
httpclient5/src/test/java/org/apache/hc/client5/http/TestHttpRoute.java
Outdated
Show resolved
Hide resolved
httpclient5/src/test/java/org/apache/hc/client5/http/examples/UnixDomainSocket.java
Show resolved
Hide resolved
httpclient5/src/test/java/org/apache/hc/client5/http/examples/UnixDomainSocketAsync.java
Show resolved
Hide resolved
This change adds Unix domain socket support. The sync client uses JUnixSocket, which provides synchronous UDS support through the legacy `java.net.Socket` API; the async client uses the JEP 380 implementation of UDS through the `java.nio.channels.SocketChannel` API (requires JDK16 or later). Since the synchronous client is tightly coupled to the `Socket` API, we can't trivially use JEP 380 UDS support. We would first have to write an adapter to implement the `Socket` API, backed by a JEP 380 UDS `SocketChannel`. This would require us to implement features like socket option configuration, connection timeouts, and socket timeouts; we would also have to implement APIs like `getInetAddress()` which don't actually make sense in a UDS context. This is probably doable (JUnixSocket does it, albeit with a different implementation strategy based on native code), but it's not trivial. The asynchronous client is the other way around: it supports JEP 380, but not JUnixSocket. The issue here is more subtle: JDK and JUnixDomain channels cannot be mixed in the same selector, and since JUnixDomain does not provide an implementation of TCP/IP channels, supporting JUnixSocket in the async client would require substantial rework in the IO reactor. Since JDK8 is end-of-life next year, I doubt this is worth doing unless we can find some clever way of integrating the new channel type with minimal churn. Unix domain socket support is exposed through the `RequestConfig` API. A path to a Unix domain socket can be provided as a client-wide default through `setDefaultRequestConfig`, or on a per-request basis through `setConfig`. Currently, proxies and TLS are not supported through UDS. The former feature seems unnecessary, but the latter is likely worth adding at some point, since contacting an HTTPS endpoint over UDS (sometimes denoted by the URI scheme `https+unix`) is not unheard of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final word has @ok2c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rschmitt Looks good to me
This change adds Unix domain socket support. The sync client uses JUnixSocket, which provides synchronous UDS support through the legacy
java.net.Socket
API; the async client uses the JEP 380 implementation of UDS through thejava.nio.channels.SocketChannel
API (requires JDK16 or later).Since the synchronous client is tightly coupled to the
Socket
API, we can't trivially use JEP 380 UDS support. We would first have to write an adapter to implement theSocket
API, backed by a JEP 380 UDSSocketChannel
. This would require us to implement features like socket option configuration, connection timeouts, and socket timeouts; we would also have to implement APIs likegetInetAddress()
which don't actually make sense in a UDS context. This is probably doable (JUnixSocket does it, albeit with a different implementation strategy based on native code), but it's not trivial.The asynchronous client is the other way around: it supports JEP 380, but not JUnixSocket. The issue here is more subtle: JDK and JUnixDomain channels cannot be mixed in the same selector, and since JUnixDomain does not provide an implementation of TCP/IP channels, supporting JUnixSocket in the async client would require substantial rework in the IO reactor. Since JDK8 is end-of-life next year, I doubt this is worth doing unless we can find some clever way of integrating the new channel type with minimal churn.
Unix domain socket support is exposed through the
RequestConfig
API. A path to a Unix domain socket can be provided as a client-wide default throughsetDefaultRequestConfig
, or on a per-request basis throughsetConfig
. Currently, proxies and TLS are not supported through UDS. The former feature seems unnecessary, but the latter is likely worth adding at some point, since contacting an HTTPS endpoint over UDS (sometimes denoted by the URI schemehttps+unix
) is not unheard of.