You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/11/29 18:51:41 UTC

[GitHub] [tomcat] minfrin opened a new pull request #382: Add support for unix domain sockets.

minfrin opened a new pull request #382:
URL: https://github.com/apache/tomcat/pull/382


   First implementation added to
   org.apache.coyote.http11.Http11AprProtocol.
   
   Depends on https://github.com/apache/tomcat-native/pull/8.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750148235


   I see no reason why this cannot work which Java UDS and APR UDS.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735880429


   > > > That's correct, it was supposed to be dropped already in 10.0 [it will happen in 10.1]. Instead, it got some defaults changes so that using it requires more deliberate configuration.
   > > 
   > > 
   > > At this stage JEP-380 is too far away for practical use, so having a library able to make native calls gives tomcat a significant edge.
   > > The ability to use normal PEM files in the SSL configuration is also a significant benefit.
   > 
   > I absolutely agree. This is so simple with APR/OpenSSL.
   
   Nobody has cared about UDS for the past 15 years. And PEM files are supported for JSSE and JSSE/OpenSSL. So I don't understand what the problem is.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735859022


   > That's correct, it was supposed to be dropped already in 10.0 [it will happen in 10.1]. Instead, it got some defaults changes so that using it requires more deliberate configuration.
   
   At this stage JEP-380 is too far away for practical use, so having a library able to make native calls gives tomcat a significant edge.
   
   The ability to use normal PEM files in the SSL configuration is also a significant benefit.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-743166815


   Load tested it with Vegeta:
   
   ```
   $ echo "GET http://localhost/testbed/plaintext" | vegeta attack -unix-socket /tmp/tomcat-uds.sock -rate 0 -max-workers 128 -duration 30s | vegeta encode | vegeta report --type json | jq .
   {
     "latencies": {
       "total": 2849931195122,
       "mean": 2189702,
       "50th": 1623066,
       "90th": 4702159,
       "95th": 5418223,
       "99th": 7096980,
       "max": 69859908,
       "min": 56120
     },
     "bytes_in": {
       "total": 15618180,
       "mean": 12
     },
     "bytes_out": {
       "total": 0,
       "mean": 0
     },
     "earliest": "2020-12-11T12:27:16.43339275Z",
     "latest": "2020-12-11T12:27:46.433339493Z",
     "end": "2020-12-11T12:27:46.436590425Z",
     "duration": 29999946743,
     "wait": 3250932,
     "requests": 1301515,
     "rate": 43383.910349897116,
     "throughput": 43379.20957953359,
     "success": 1,
     "status_codes": {
       "200": 1301515
     },
     "errors": []
   }
   ```
   
   Throughput: **43379**. Not bad at all!
   With TCP I was able to get 16654 on the same server, but with Tomcat 9.0.x and vegeta was executed on another machine.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532695851



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -247,7 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
-            APR_HAVE_UNIX             = has(22);
+            APR_HAVE_UNIX             = has(21);

Review comment:
       ...and that would be where the 21 comes from:
   
   ```
           case 21:
   #if defined(APR_POLLSET_WAKEABLE)
               rv = JNI_TRUE;
   #endif
           break;
   ```
   
   Let me revert.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735456090


   > So you have shared machine where everyone can snoop on localhost? Since the socket files will be owned by a Tomcat system user you want to add HTTPd to that group to make it interact with Tomcat?
   
   Yes.
   
   In this particular example it's a mailserver, with a whole host of related daemons running. If any of the those daemons allows anything shady, open ports on localhost are an obvious target. This shuts this all down completely.
   
   You can get away with it if you use passwords, or session cookies, but in this case it's 100% certificates, and that creates a problem.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r533567922



##########
File path: java/org/apache/catalina/core/LocalStrings.properties
##########
@@ -74,7 +74,7 @@ aprListener.aprInitDebug=The Apache Tomcat Native library could not be found usi
 aprListener.aprInitError=The Apache Tomcat Native library failed to load. The error reported was [{0}]
 aprListener.currentFIPSMode=Current FIPS mode: [{0}]
 aprListener.enterAlreadyInFIPSMode=AprLifecycleListener is configured to force entering FIPS mode, but library is already in FIPS mode [{0}]
-aprListener.flags=APR capabilities: IPv6 [{0}], sendfile [{1}], accept filters [{2}], random [{3}].
+aprListener.flags=APR capabilities: IPv6 [{0}], sendfile [{1}], accept filters [{2}], random [{3}], uds [{4}].

Review comment:
       Done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532706511



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Currently there is no neat way (that I can find) to express Windows permissions in a string that can be assigned to a parameter in the connector.
   
   The java implementation to set ACLs on Windows is the following:
   
   https://docs.oracle.com/javase/7/docs/api/java/nio/file/attribute/AclFileAttributeView.html
   
   On Windows there is a concept of permissions for the "owner", (not useful for a unix domain socket) and for "everyone" (useful if placed in a protected path), but there is no such thing as a primary group.
   
   In short, the problem I'm trying to solve is this:
   
   ```
       <Connector path="/tmp/protected/tomcat.socket" pathPermissions="[what-goes-here]" protocol="org.apache.coyote.http11.Http11AprProtocol"
                  maxThreads="150" SSLEnabled="false" >
   ```
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736761640


   > 
   > 
   > > > To be in any way useful the socket must be writable, and to do that it either needs to default to being writable, or needs to explicitly set as writable with at least `pathPermissions="rw-rw----"`.
   > > 
   > > 
   > > So not to undermine the default umask, are we good to take your `pathPermissions="rw-rw----"` proposal?
   > 
   > I'm not following - the umask makes no sense, not even as a default, so we have to override the umask to make it work at all.
   > 
   > I think a sensible approach is "defaults to the same behaviour as localhost, visible to all on the box, while offering posixPermissions to the unix people, and a protected parent directory for the windows people."
   > 
   > That's where we stand now.
   
   OK, my slight counter proposal is not use `rw-rw-rw-` as default, but `rw-rw----` because this would reflect the default umask of 027, i.e, not to create anything world readable. For those who need more permissions, they can supply a custom string.
   
   I also do understand that localhost is open for everyone on that box, but isn't that the whole point of UDS to have more control of the socket?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532913829



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       Absolutely, it is now crystal clear that that is a domain socket.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736746529


   > > To be in any way useful the socket must be writable, and to do that it either needs to default to being writable, or needs to explicitly set as writable with at least `pathPermissions="rw-rw----"`.
   > 
   > So not to undermine the default umask, are we good to take your `pathPermissions="rw-rw----"` proposal?
   
   I'm not following - the umask makes no sense, not even as a default, so we have to override the umask to make it work at all.
   
   I think a sensible approach is "defaults to the same behaviour as localhost, visible to all on the box, while offering posixPermissions to the unix people, and a protected parent directory for the windows people."
   
   That's where we stand now.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532268550



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -347,22 +352,27 @@ public String getName() {
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
         name.append('-');
-        if (getAddress() != null) {
-            name.append(getAddress().getHostAddress());
-            name.append('-');
+        if (getPath() != null) {
+            name.append(getPath().getFileName().toString());

Review comment:
       I need to think about this, ugly...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735780186


   By the way, there were talks at dev@ about dropping/deprecating AprProtocol and recommending the use of NIO(2). Maybe for 10.1.x, not decided yet.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r540877133



##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">

Review comment:
       Having both enabled might lead to confusion as it does now with dual-stack socket. Most don't even know that one can pass sockopt IPV6_ONLY. I would avoid such situation unless the admin does exactly know what is happening here. Now the misfortune here is that we don't have an address field, but rather a host/port which is tied to TCP sockets. Maybe it is worth a discussion on the dev ML to move to `address` instead of `host`/`port` combination. So do all decent socket-based services like sshd, httpd, etc. WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532269203



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();

Review comment:
       OK, go with APR consistency here. Though, I'd rather see `address`, but consistency is more important.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r540877133



##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">

Review comment:
       Having both enabled might lead to confusion as it does now with dual-stack socket. Most don't even know that one can pass sockopt [`IPV6_V6ONLY`](https://www.freebsd.org/cgi/man.cgi?query=ip6&apropos=0&sektion=4&manpath=FreeBSD+12.2-RELEASE+and+Ports&arch=default&format=html). I would avoid such situation unless the admin does exactly know what is happening here. Now the misfortune here is that we don't have an address field, but rather a host/port which is tied to TCP sockets. Maybe it is worth a discussion on the dev ML to move to `address` instead of `host`/`port` combination. So do all decent socket-based services like sshd, httpd, etc. WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-738291588


   @minfrin Do you want to peform anymore changes or do want to me run verifcation on it? Do you think a test would be possible to start up and shut down a UDS?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532990550



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Done - pathPermissions added and documented.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750274850


   It can 100% work with APR, except I personally don't want to add features to that component at this point.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-743160059


   In my test application TLS is configured but it is not used/needed by UDS so HTTP2 does not work:
   
   ```
   $ curl --http2 --unix-socket /tmp/tomcat-uds.sock http://localhost/testbed/plaintext
   curl: (56) Recv failure: Connection reset by peer
   $ ubuntu@martin-arm64 /tmp [56]> curl --unix-socket /tmp/tomcat-uds.sock http://localhost/testbed/plaintext
   curl: (56) Recv failure: Connection reset by peer
   ```
   
   If I use `h2c` then all is fine:
   
   `INFO: The ["http-apr-/tmp/tomcat-uds.sock"] connector has been configured to support HTTP upgrade to [h2c]`
   
   ```
   $ curl --unix-socket /tmp/tomcat-uds.sock http://localhost/testbed/plaintext
   Hello world!⏎    
   ```  
   I think it would be good to document this.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532692219



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       > 
   > 
   > > Why not use https://stackoverflow.com/q/26649751/696632 and this [idea](https://serverfault.com/a/437128)?
   > 
   > It's because this is unix specific. Java lets us set posix permissions, but then this doesn't work on Windows.
   > 
   > I originally considered a pathPermissions parameter with the value set using the fromString() method as per here, but rejected it due to the cross platform nature of tomcat. What do you think?
   > 
   > https://docs.oracle.com/javase/7/docs/api/java/nio/file/attribute/PosixFilePermissions.html#fromString(java.lang.String)
   
   I'd be OK with having settings for POSIX and Windows. It is a Unix technology after all.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532490661



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       While not universal, placing sockets in protected directories is still common.
   
   At this stage, until there is a practical way to express permissions as a string which can then be placed in the connector element in the config, I think this is a good compromise.
   
   I asked the SO community for their thoughts, and this came up: https://stackoverflow.com/a/65064406/4598583
   
   I am thinking ahead for any future JEP-380 implementation, which will have the same issue.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-747467253


   I am not convinced about adding that feature to the APR endpoint ...
   
   Anyway:
   - The changes to IntrospectionUtils are too much given the actual use, strings could be used and the endpoint is the only place that actually deals with the two types so it seems enough
   - I don't get the idea behind the "permissions", since I don't think Tomcat is the party that is supposed to be creating the socket
   
   The UDS feature should already work with NIO and Java 16 EA by using an inherited channel. The limitation is that there is only one endpoint that can use a UDS. I'm ok with adding full UDS support to NIO using the compat package (the amount of reflection needed does not seem too bad so I may try it to see how that would work).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735864234


   > 
   > 
   > > That's correct, it was supposed to be dropped already in 10.0 [it will happen in 10.1]. Instead, it got some defaults changes so that using it requires more deliberate configuration.
   > 
   > At this stage JEP-380 is too far away for practical use, so having a library able to make native calls gives tomcat a significant edge.
   > 
   > The ability to use normal PEM files in the SSL configuration is also a significant benefit.
   
   I absolutely agree. This is so simple with APR/OpenSSL.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736282608


   Even if the Tomcat dev team decides to drop support for the APR connector I don't see a problem the code to be extracted to a new project and be supported by the community.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r533367239



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Specifically, this  is done:
   
   ```
   <Connector path="/tmp/protected/tomcat.socket" pathPermissions="rw-rw----" protocol="org.apache.coyote.http11.Http11AprProtocol"
                  maxThreads="150" SSLEnabled="false" >
   ```
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532815444



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -347,22 +352,27 @@ public String getName() {
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
         name.append('-');
-        if (getAddress() != null) {
-            name.append(getAddress().getHostAddress());
-            name.append('-');
+        if (getPath() != null) {
+            name.append(getPath().getFileName().toString());

Review comment:
       Agreed - fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532587344



##########
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##########
@@ -568,6 +569,14 @@ public final int getLocalPort() {
     protected abstract InetSocketAddress getLocalAddress() throws IOException;
 
 
+    /**
+     * Address for the unix domain socket.
+     */
+    private Path path;

Review comment:
       Since TCP sockets use `hostname`, path is fine. Already discussed, it should be `address` regardless of the family.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532265150



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -347,22 +352,27 @@ public String getName() {
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
         name.append('-');
-        if (getAddress() != null) {
-            name.append(getAddress().getHostAddress());
-            name.append('-');
+        if (getPath() != null) {
+            name.append(getPath().getFileName().toString());

Review comment:
       In theory the name will clash.
   
   We could try the full path separated by dashes? How do we handle windows though, ignore all special characters?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761110354


   > I added the feature for NIO, since it wasn't too difficult using https://openjdk.java.net/jeps/380 .
   
   Thank you for doing this, it is a huge help.
   
   This patch is all about supporting people who are not yet in a position to use Java 16.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532523457



##########
File path: webapps/docs/config/http.xml
##########
@@ -1152,6 +1156,11 @@
   permissions appropriately configured to restrict access as required.
   </p>
 
+  <p>Tomcat will automatically remove the socket on server shutdown. If the
+  socket already exists startup will fail. Care must be taken by the
+  administrator to remove the socket after verifying that the socket isn't
+  already being used by an existing tomcat process.</p>

Review comment:
       Same here




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735453352


   So you have shared machine where everyone can snoop on localhost? Since the socket files will be owned by a Tomcat system user you want to add HTTPd to that group to make it interact with Tomcat?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532665610



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       > Why not use https://stackoverflow.com/q/26649751/696632 and this [idea](https://serverfault.com/a/437128)?
   
   It's because this is unix specific. Java lets us set posix permissions, but then this doesn't work on Windows.
   
   I originally considered a pathPermissions parameter with the value set using the fromString() method as per here, but rejected it due to the cross platform nature of tomcat. What do you think?
   
   https://docs.oracle.com/javase/7/docs/api/java/nio/file/attribute/PosixFilePermissions.html#fromString(java.lang.String)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532274020



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Hmm, I agree on JEP 380. I do think that people will either use `${catalina.base}/work/<name>.sock` or simply `/var/run/<name>.sock`. I would not expect people to create an additional dir for that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532272524



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       We're not, no, as the permissions on the directory containing the socket come into play:
   
   ```
   Little-Net:tomcat-socket10 minfrin$ ls -al /tmp/protected/
   total 0
   drwxr-x---   3 minfrin  wheel   96 Nov 29 23:39 .
   drwxrwxrwt  11 root     wheel  352 Nov 29 23:39 ..
   srw-rw-rw-   1 minfrin  wheel    0 Nov 29 23:39 tomcat.socket
   ```
   
   Only minfrin and members of the wheel group can see the socket above.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735789759


   That's correct, it was supposed to be dropped already in 10.0 [it will happen in 10.1]. Instead, it got some defaults changes so that using it requires more deliberate configuration.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532584495



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -177,6 +177,9 @@ private Library(String libraryName)
     /* Is the O_NONBLOCK flag inherited from listening sockets?
      */
     public static boolean APR_O_NONBLOCK_INHERITED  = false;
+    /* Support for Unix Domain Sockets.
+     */
+    public static boolean APR_HAVE_UNIX           = false;

Review comment:
       I had the same on my mind, but this is the define from APR. It should be consistent. Look into the header files. It is also called `AF_UNIX` = address family Unix domain socket.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761071502


   > @minfrin Do you want to peform anymore changes or do want to me run verifcation on it? Do you think a test would be possible to start up and shut down a UDS?
   
   I've added a test based on the existing APR tests. Can you confirm this is all ok?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761238160


   > > * The permission attribute, is it really useful ?
   > 
   > In the absence of a permission attribute (and without the "everyone" default), the socket is equivalent to a TCP port that has been firewalled off, and thus pointless.
   > 
   > Ignoring special cases like a personal development environment, or a system with no user separation, daemons (like tomcat) are secured with a user tomcat, group tomcat, and a typical umask of 0750 (or some variation). This means that the a) the tomcat user can write, b) the tomcat group can read (typically allowing read access to log files), and c) everyone else get nothing.
   > 
   > In order for any unix domain socket to be of use to anyone, it must be possible to write to it. If you can't write to it, you cannot submit a request. A unix domain socket that only the tomcat user can write to pointless, as you've giving the client control over the tomcat process. A read only unix domain socket for a request/response protocol like HTTP has no practical effect - having written nothing you will read nothing.
   > 
   > For this reason, every daemon out there that I have seen has a mechanism to make the socket writable to a group, and defaulting to being accessible to everyone:
   > 
   > https://github.com/Cisco-Talos/clamav-devel/blob/31824a659dff37ae03e3419395bb68e659c2b165/etc/clamd.conf.sample#L104
   > 
   > https://github.com/trusteddomainproject/OpenDMARC/blob/b0d6408d0859adb336428e3d0bd87749513a9e79/opendmarc/opendmarc.conf.sample#L357
   > 
   > https://github.com/rspamd/rspamd/blob/9c2d72c6eba3fc05fd7459e388ea7c92eb87095f/conf/options.inc#L48
   > 
   > In the absence of an explicit control over permissions, making the permissions world writable by default allows the admin to secure the socket by restricting permissions on the parent directory, such as the following example:
   > 
   > ```
   > [root@localhost clamav-milter]# ls -al
   > total 0
   > drwx--x---.  2 clamilt clamilt   60 Jan 11 13:03 .
   > drwxr-xr-x. 39 root    root    1080 Jan 11 13:06 ..
   > srw-rw-rw-.  1 clamilt clamilt    0 Jan 11 13:03 clamav-milter.socket
   > ```
   > 
   > In the above, the socket itself is world writable, but the parent directory is protected, and therefore the socket is protected.
   > 
   > > * The socket is not deleted on shutdown (although the channel is closed)
   > 
   > If the socket is not deleted on shutdown, the server cannot subsequently be started up. Deleting the socket on shutdown is the most common behaviour. Deleting the socket is startup is not done, as it means that multiple daemons can be started without error.
   > 
   > I think I saw a commit go past fixing this, need to verify.
   
   Ok, I still dislike the permission attribute quite a bit but I can understand things can be annoying without it in some cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532875949



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       While it seems ugly, it makes it perfectly clear that it is a path on the local filesystem, doesn't it?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532267605



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();

Review comment:
       I can try just name if you like?
   
   hostname matches the APR implementation.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736684033


   > The typical umask is 0027, meaning full access for tomcat itself, read access for members of the tomcat group (so that logfiles can be read but not changed), and no access for anyone else.
   >
   > The unix domain socket is useless if you can't write to it. What that means is that only the tomcat user can send requests to tomcat, and members of the tomcat group can't send requests at all, which is completely pointless.
   
   Exactly, that's the whole problem.
   
   > To be in any way useful the socket must be writable, and to do that it either needs to default to being writable, or needs to explicitly set as writable with at least `pathPermissions="rw-rw----"`.
   
   So not to undermine the default umask, are we good to take your `pathPermissions="rw-rw----"` proposal?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin closed pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin closed pull request #382:
URL: https://github.com/apache/tomcat/pull/382


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-743155616


   There is some issue with stopping an embedded Tomcat:
   
   ```
   === Started
   
   ^CDec 11, 2020 11:59:45 AM org.apache.coyote.AbstractProtocol pause
   INFO: Pausing ProtocolHandler ["https-openssl-apr-/tmp/tomcat-uds.sock"]
   Dec 11, 2020 11:59:45 AM org.apache.catalina.core.StandardService stopInternal
   INFO: Stopping service [Tomcat]
   WARNING: An illegal reflective access operation has occurred
   WARNING: Illegal reflective access by org.apache.catalina.loader.WebappClassLoaderBase (file:/home/ubuntu/git/mg.solutions/http2-server-perf-tests/java/tomcat/target/tomcat-embedded-1.0-SNAPSHOT.jar) to field java.io.ObjectStreamClass$Caches.localDescs
   WARNING: Please consider reporting this to the maintainers of org.apache.catalina.loader.WebappClassLoaderBase
   WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
   WARNING: All illegal access operations will be denied in a future release
   Dec 11, 2020 11:59:45 AM org.apache.coyote.AbstractProtocol stop
   INFO: Stopping ProtocolHandler ["https-openssl-apr-/tmp/tomcat-uds.sock"]
   Dec 11, 2020 11:59:55 AM org.apache.tomcat.util.net.Acceptor stop
   WARNING: The acceptor thread [https-openssl-apr-/tmp/tomcat-uds.sock-Acceptor] did not stop cleanly
   === Stopped
   
   ```
   and the application hangs.
   
   Thread dump:
   
   ```
   2020-12-11 12:02:45                                                                                    
   Full thread dump OpenJDK 64-Bit Server VM (16-ea+26-1764 mixed mode):                                  
                                                                                                          
   Threads class SMR info:                                                                                
   _java_thread_list=0x0000ffff806d7a50, length=14, elements={                                      
   0x0000ffff801d1390, 0x0000ffff801d2b30, 0x0000ffff801ffda0, 0x0000ffff802012e0,                        
   0x0000ffff80202810, 0x0000ffff802042e0, 0x0000ffff802058e0, 0x0000ffff80206e70,                  
   0x0000ffff80297670, 0x0000ffff802a23f0, 0x0000ffff80697560, 0x0000fffef8001100,                        
   0x0000ffff806e3050, 0x0000ffff800248b0                                                                 
   }                                                                                                                                                                                                              
                                                                                                          
   "Reference Handler" #2 daemon prio=10 os_prio=0 cpu=0.42ms elapsed=413.75s tid=0x0000ffff801d1390 nid=0x2c113f waiting on condition  [0x0000ffff609fc000]                                                      
      java.lang.Thread.State: RUNNABLE                                                                                                                                                                            
           at java.lang.ref.Reference.waitForReferencePendingList(java.base@16-ea/Native Method)          
           at java.lang.ref.Reference.processPendingReferences(java.base@16-ea/Reference.java:243)                                                                                                                
           at java.lang.ref.Reference$ReferenceHandler.run(java.base@16-ea/Reference.java:215)            
                                                                                                                                                                                                                  
   "Finalizer" #3 daemon prio=8 os_prio=0 cpu=0.46ms elapsed=413.75s tid=0x0000ffff801d2b30 nid=0x2c1140 in Object.wait()  [0x0000ffff607fc000]                                                                   
      java.lang.Thread.State: WAITING (on object monitor)                                                                                                                                                         
           at java.lang.Object.wait(java.base@16-ea/Native Method)                                                                                                                                                
           - waiting on <0x00000007148016c8> (a java.lang.ref.ReferenceQueue$Lock)                                                                                                                                
           at java.lang.ref.ReferenceQueue.remove(java.base@16-ea/ReferenceQueue.java:155)                                                                                                                        
           - locked <0x00000007148016c8> (a java.lang.ref.ReferenceQueue$Lock)                      
           at java.lang.ref.ReferenceQueue.remove(java.base@16-ea/ReferenceQueue.java:176)                
           at java.lang.ref.Finalizer$FinalizerThread.run(java.base@16-ea/Finalizer.java:171)                                                                                                                     
                                                                                                                                                                                                                  
   "Signal Dispatcher" #4 daemon prio=9 os_prio=0 cpu=0.53ms elapsed=413.75s tid=0x0000ffff801ffda0 nid=0x2c1141 waiting on condition  [0x0000000000000000]                                                       
      java.lang.Thread.State: RUNNABLE                                                                    
                                                                                                                                                                                                                  
   "Service Thread" #5 daemon prio=9 os_prio=0 cpu=0.38ms elapsed=413.75s tid=0x0000ffff802012e0 nid=0x2c1142 runnable  [0x0000000000000000]                                                                      
      java.lang.Thread.State: RUNNABLE                                                                                                                                                                            
                                                                                                          
   "Monitor Deflation Thread" #6 daemon prio=9 os_prio=0 cpu=2.44ms elapsed=413.75s tid=0x0000ffff80202810 nid=0x2c1143 runnable  [0x0000000000000000]                                                            
      java.lang.Thread.State: RUNNABLE   
   
                                                                                                                                                                                                                 
   "C2 CompilerThread0" #7 daemon prio=9 os_prio=0 cpu=439.33ms elapsed=413.75s tid=0x0000ffff802042e0 nid=0x2c1144 waiting on condition  [0x0000000000000000]
      java.lang.Thread.State: RUNNABLE
      No compile task                                 
   
   "C1 CompilerThread0" #10 daemon prio=9 os_prio=0 cpu=605.34ms elapsed=413.75s tid=0x0000ffff802058e0 nid=0x2c1145 waiting on condition  [0x0000000000000000]
      java.lang.Thread.State: RUNNABLE
      No compile task                                 
   
   "Sweeper thread" #11 daemon prio=9 os_prio=0 cpu=0.09ms elapsed=413.75s tid=0x0000ffff80206e70 nid=0x2c1146 runnable  [0x0000000000000000]
      java.lang.Thread.State: RUNNABLE
   
   "Notification Thread" #12 daemon prio=9 os_prio=0 cpu=0.10ms elapsed=413.72s tid=0x0000ffff80297670 nid=0x2c1147 runnable  [0x0000000000000000]
      java.lang.Thread.State: RUNNABLE
   
   "Common-Cleaner" #13 daemon prio=8 os_prio=0 cpu=0.69ms elapsed=413.71s tid=0x0000ffff802a23f0 nid=0x2c1149 in Object.wait()  [0x0000ffff50ffc000]
      java.lang.Thread.State: TIMED_WAITING (on object monitor)
           at java.lang.Object.wait(java.base@16-ea/Native Method)
           - waiting on <0x0000000714802c70> (a java.lang.ref.ReferenceQueue$Lock)
           at java.lang.ref.ReferenceQueue.remove(java.base@16-ea/ReferenceQueue.java:155)
           - locked <0x0000000714802c70> (a java.lang.ref.ReferenceQueue$Lock)
           at jdk.internal.ref.CleanerImpl.run(java.base@16-ea/CleanerImpl.java:140)
           at java.lang.Thread.run(java.base@16-ea/Thread.java:831)
           at jdk.internal.misc.InnocuousThread.run(java.base@16-ea/InnocuousThread.java:134)
   
   "Catalina-utility-1" #15 prio=1 os_prio=0 cpu=22.45ms elapsed=412.77s tid=0x0000ffff80697560 nid=0x2c1151 waiting on condition  [0x0000ffff127fd000]
      java.lang.Thread.State: WAITING (parking)
           at jdk.internal.misc.Unsafe.park(java.base@16-ea/Native Method)
           - parking to wait for  <0x00000007149aaa60> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
           at java.util.concurrent.locks.LockSupport.park(java.base@16-ea/LockSupport.java:341)
           at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode.block(java.base@16-ea/AbstractQueuedSynchronizer.java:505)
           at java.util.concurrent.ForkJoinPool.managedBlock(java.base@16-ea/ForkJoinPool.java:3137)
           at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(java.base@16-ea/AbstractQueuedSynchronizer.java:1614)
           at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@16-ea/ScheduledThreadPoolExecutor.java:1177)
           at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@16-ea/ScheduledThreadPoolExecutor.java:899)
           at java.util.concurrent.ThreadPoolExecutor.getTask(java.base@16-ea/ThreadPoolExecutor.java:1056)
           at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@16-ea/ThreadPoolExecutor.java:1116)
           at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@16-ea/ThreadPoolExecutor.java:630)
           at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
           at java.lang.Thread.run(java.base@16-ea/Thread.java:831)
   
   
   "Catalina-utility-2" #16 prio=1 os_prio=0 cpu=22.19ms elapsed=412.77s tid=0x0000fffef8001100 nid=0x2c1152 waiting on condition  [0x0000ffff125fd000]
      java.lang.Thread.State: WAITING (parking)
           at jdk.internal.misc.Unsafe.park(java.base@16-ea/Native Method)
           - parking to wait for  <0x00000007149aaa60> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
           at java.util.concurrent.locks.LockSupport.park(java.base@16-ea/LockSupport.java:341)
           at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode.block(java.base@16-ea/AbstractQueuedSynchronizer.java:505)
           at java.util.concurrent.ForkJoinPool.managedBlock(java.base@16-ea/ForkJoinPool.java:3137)
           at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(java.base@16-ea/AbstractQueuedSynchronizer.java:1614)
           at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@16-ea/ScheduledThreadPoolExecutor.java:1170)
           at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(java.base@16-ea/ScheduledThreadPoolExecutor.java:899)
           at java.util.concurrent.ThreadPoolExecutor.getTask(java.base@16-ea/ThreadPoolExecutor.java:1056)
           at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@16-ea/ThreadPoolExecutor.java:1116)
           at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@16-ea/ThreadPoolExecutor.java:630)
           at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
           at java.lang.Thread.run(java.base@16-ea/Thread.java:831)
   
   "https-openssl-apr-/tmp/tomcat-uds.sock-Acceptor" #28 daemon prio=5 os_prio=0 cpu=0.24ms elapsed=412.76s tid=0x0000ffff806e3050 nid=0x2c115e runnable  [0x0000ffff10dfe000]
      java.lang.Thread.State: RUNNABLE
           at org.apache.tomcat.jni.Socket.accept(Native Method)
           at org.apache.tomcat.util.net.AprEndpoint.serverSocketAccept(AprEndpoint.java:729)
           at org.apache.tomcat.util.net.AprEndpoint.serverSocketAccept(AprEndpoint.java:82)
           at org.apache.tomcat.util.net.Acceptor.run(Acceptor.java:106)
           at java.lang.Thread.run(java.base@16-ea/Thread.java:831)
   
   "DestroyJavaVM" #30 prio=5 os_prio=0 cpu=1003.31ms elapsed=169.91s tid=0x0000ffff800248b0 nid=0x2c1138 waiting on condition  [0x0000000000000000]
      java.lang.Thread.State: RUNNABLE
   
   "VM Thread" os_prio=0 cpu=8.42ms elapsed=413.76s tid=0x0000ffff801c2f70 nid=0x2c113e runnable  
   
   "GC Thread#0" os_prio=0 cpu=5.94ms elapsed=413.78s tid=0x0000ffff80073930 nid=0x2c1139 runnable  
   
   "GC Thread#1" os_prio=0 cpu=4.91ms elapsed=412.94s tid=0x0000ffff48004f60 nid=0x2c114c runnable  
   
   "GC Thread#2" os_prio=0 cpu=4.92ms elapsed=412.94s tid=0x0000ffff48005af0 nid=0x2c114d runnable  
   
   "GC Thread#3" os_prio=0 cpu=4.90ms elapsed=412.94s tid=0x0000ffff48006680 nid=0x2c114e runnable  
   
   "GC Thread#4" os_prio=0 cpu=4.87ms elapsed=412.94s tid=0x0000ffff48007210 nid=0x2c114f runnable  
   
   "GC Thread#5" os_prio=0 cpu=4.88ms elapsed=412.94s tid=0x0000ffff48007da0 nid=0x2c1150 runnable  
   
   "G1 Main Marker" os_prio=0 cpu=0.11ms elapsed=413.78s tid=0x0000ffff800848c0 nid=0x2c113a runnable  
   
   "G1 Conc#0" os_prio=0 cpu=0.06ms elapsed=413.78s tid=0x0000ffff80085950 nid=0x2c113b runnable  
   
   "G1 Refine#0" os_prio=0 cpu=0.08ms elapsed=413.78s tid=0x0000ffff8013ac50 nid=0x2c113c runnable  
   
   "G1 Service" os_prio=0 cpu=15.07ms elapsed=413.78s tid=0x0000ffff8013bc70 nid=0x2c113d runnable  
   
   "VM Periodic Task Thread" os_prio=0 cpu=45.58ms elapsed=413.72s tid=0x0000ffff80299160 nid=0x2c1148 waiting on condition  
   
   JNI global refs: 27, weak refs: 0
   
   Heap                                               
    garbage-first heap   total 256000K, used 17190K [0x0000000706600000, 0x0000000800000000)
     region size 2048K, 9 young (18432K), 2 survivors (4096K)
    Metaspace       used 19940K, committed 20160K, reserved 1073152K
     class space    used 1918K, committed 2048K, reserved 1048576K
   
   
   ```
   
   The application I use to test it could be found at https://github.com/martin-g/http2-server-perf-tests/blob/feature/jakartaee-9/java/tomcat/src/main/java/info/mgsolutions/tomcat/TomcatEmbedded.java


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-737203904


   > OK, my slight counter proposal is not use rw-rw-rw- as default, but rw-rw---- because this would reflect the default umask of 027, i.e, not to create anything world readable. For those who need more permissions, they can supply a custom string.
   
   The problem with this is that it makes the default behaviour between windows and unix inconsistent, and this is likely to cause headaches for people who either don't read the docs properly, or read a response on stack overflow aimed at unix people and use it thinking it also applies to windows.
   
   Setting a default on windows is itself hard - windows doesn't have a concept of a "primary group" like posix, but the possibility of zero or more users and/or groups that have access to a file or directory. There is no practical default behaviour for any of that, which is why java itself doesn't try. Java gives you "access to owner" and "access to everyone", and that's it. "Access to owner" is the same as "no uds support", that leaves just "access to everyone, protect me by protecting my parent directory".
   
   > I also do understand that localhost is open for everyone on that box, but isn't that the whole point of UDS to have more control over the socket?
   
   Yes - and the most simplest way to protect a socket is to put it in a suitably protected directory. You don't have to protect the socket file itself, just make it impossible for the file to be seen by making its parent directory inaccessible.
   
   I am very mindful of decisions made now being difficult to change down the line. Adding new behaviour in future is easy, but changing existing behaviour (like a default) is a headache for all concerned.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o edited a comment on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736761640


   > 
   > 
   > > > To be in any way useful the socket must be writable, and to do that it either needs to default to being writable, or needs to explicitly set as writable with at least `pathPermissions="rw-rw----"`.
   > > 
   > > 
   > > So not to undermine the default umask, are we good to take your `pathPermissions="rw-rw----"` proposal?
   > 
   > I'm not following - the umask makes no sense, not even as a default, so we have to override the umask to make it work at all.
   > 
   > I think a sensible approach is "defaults to the same behaviour as localhost, visible to all on the box, while offering posixPermissions to the unix people, and a protected parent directory for the windows people."
   > 
   > That's where we stand now.
   
   OK, my slight counter proposal is not use `rw-rw-rw-` as default, but `rw-rw----` because this would reflect the default umask of 027, i.e, not to create anything world readable. For those who need more permissions, they can supply a custom string.
   
   I also do understand that localhost is open for everyone on that box, but isn't that the whole point of UDS to have more control over the socket?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r533488953



##########
File path: java/org/apache/tomcat/util/net/LocalStrings.properties
##########
@@ -88,6 +88,7 @@ endpoint.init.bind=Socket bind failed: [{0}] [{1}]
 endpoint.init.bind.inherited=No inherited channel while the connector was configured to use one
 endpoint.init.listen=Socket listen failed: [{0}] [{1}]
 endpoint.init.notavail=APR not available
+endpoint.init.unixnotavail=Unix domain socket support not available

Review comment:
       Same here

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this

Review comment:
       Here

##########
File path: java/org/apache/catalina/core/LocalStrings.properties
##########
@@ -74,7 +74,7 @@ aprListener.aprInitDebug=The Apache Tomcat Native library could not be found usi
 aprListener.aprInitError=The Apache Tomcat Native library failed to load. The error reported was [{0}]
 aprListener.currentFIPSMode=Current FIPS mode: [{0}]
 aprListener.enterAlreadyInFIPSMode=AprLifecycleListener is configured to force entering FIPS mode, but library is already in FIPS mode [{0}]
-aprListener.flags=APR capabilities: IPv6 [{0}], sendfile [{1}], accept filters [{2}], random [{3}].
+aprListener.flags=APR capabilities: IPv6 [{0}], sendfile [{1}], accept filters [{2}], random [{3}], uds [{4}].

Review comment:
       Please uppercase uds in all properties files

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The

Review comment:
       Here

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The
+      permissions are specified as a string of nine characters, in three sets
+      of three: (r)ead, (w)rite and e(x)ecute for owner, group and everyone
+      else respectively. If a permission is not granted, a dash is used. If

Review comment:
       dash => hyphen

##########
File path: webapps/docs/changelog.xml
##########
@@ -124,6 +124,11 @@
         uses the correct setting for the secure attribute for any session
         cookies it creates. Based on a pull request by Andreas Kurth. (markt)
       </fix>
+      <add>
+        <bug>64943</bug>: Add support for unix domain sockets to

Review comment:
       And here

##########
File path: webapps/docs/config/http.xml
##########
@@ -1130,6 +1148,38 @@
   </subsection>
 
 
+  <subsection name="Unix Domain Socket Support">
+
+  <p>When the <code>path</code> attribute is used, connectors that support
+  unix domain sockets will bind to the socket at the given path. The first

Review comment:
       Upper

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The
+      permissions are specified as a string of nine characters, in three sets
+      of three: (r)ead, (w)rite and e(x)ecute for owner, group and everyone

Review comment:
       everyone is Windows, Unix is others.

##########
File path: webapps/docs/config/http.xml
##########
@@ -1130,6 +1148,38 @@
   </subsection>
 
 
+  <subsection name="Unix Domain Socket Support">
+
+  <p>When the <code>path</code> attribute is used, connectors that support
+  unix domain sockets will bind to the socket at the given path. The first
+  connector to support this is the
+  <code>org.apache.coyote.http11.Http11AprProtocol</code> connector when
+  used with the Apache Tomcat Native library v1.2.26 and up, along with
+  Apache Portable Runtime v1.6 and higher.
+  </p>
+
+  <p>The socket path is created with read and write permissions for all
+  users. To protect this socket, place it in a directory with suitable
+  permissions appropriately configured to restrict access as required.
+  Alternatively, on platforms that support posix permissions, the
+  permissions on the socket can be set directly with the
+  <code>pathPermissions</code> option.
+  </p>
+
+  <p>Tomcat will automatically remove the socket on server shutdown. If the
+  socket already exists startup will fail. Care must be taken by the
+  administrator to remove the socket after verifying that the socket isn't
+  already being used by an existing Tomcat process.</p>
+
+  <p>The unix domain socket can be accessed using the

Review comment:
       Here




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735452576


   > Is this complete from your POV? I'd like to give this a spin next month on FreeBSD.
   > Did you run some numbers how it compares for your usecase against localhost?
   
   It's complete from my POV.
   
   My chief interest is getting rid of passwords rather than performance. If I run a server on localhost I need to prevent someone or something trying to connect to that endpoint through the backdoor, and that means shared secrets to protect credentials that show up in backups, etc.
   
   What I want is for httpd to do it's proxy magic, and connect to tomcat over UDS. I can configure this so that only httpd is allowed to connect to tomcat and nothing else. I can then pass certificate credentials from httpd to tomcat using unencrypted JWT, and life becomes easy.
   
   Exposing tomcat directly is no good as there are many tomcats in my case, and I want them separate from one another, but exposed through the same webserver.
   
   AJP over UDS for credential transfer is also theoretically possible, but people are starting to withdraw support for AJP.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532818744



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -244,6 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
+            APR_HAVE_UNIX             = has(22);

Review comment:
       Reverted back, 21 is used by APR_POLLSET_WAKEABLE.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532520621



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Why not use https://stackoverflow.com/q/26649751/696632 and this [idea](https://serverfault.com/a/437128)?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532692425



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -247,7 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
-            APR_HAVE_UNIX             = has(22);
+            APR_HAVE_UNIX             = has(21);

Review comment:
       Is this 21 reflected in native code?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532677929



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {
+            getPath().toFile().setReadable(true, false);
+            getPath().toFile().setWritable(true, false);
+            getPath().toFile().setExecutable(false, false);

Review comment:
       Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750141172


   I added the feature for NIO, since it wasn't too difficult using https://openjdk.java.net/jeps/380 . Testing with curl works fine, I'll add a test in the testsuite next. It does have specific unlock accept, some compatibility code for port/address annoyances ("TCP local addresses" for access logs, JMX name, connector name), and the port attribute becomes optional. This cannot be added to NIO2 since the feature is not available.
   
   Some possible changes:
   - The permission attribute, is it really useful ?
   - Reflection is used for this attribute for now since this is NIO only
   - The socket is not deleted on shutdown (although the channel is closed)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761070369


   Life is too short to fight with git.
   
   Opened a fresh PR at https://github.com/apache/tomcat/pull/401.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532523379



##########
File path: webapps/docs/config/http.xml
##########
@@ -212,7 +212,11 @@
       <p>Where supported, the path to a unix domain socket that this
       <strong>Connector</strong> will create and await incoming connections.
       The socket is created with world read and write permissions. To protect
-      the socket create the socket in a suitably protected directory.</p>
+      the socket create the socket in a suitably protected directory. Tomcat
+      will automatically remove the socket on server shutdown. If the socket
+      already exists, care must be taken by the administrator to remove the
+      socket after verifying that the socket isn't already being used by an
+      existing tomcat process.</p>

Review comment:
       tomcat => Tomcat




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-737353972


   > 
   > 
   > > OK, my slight counter proposal is not use rw-rw-rw- as default, but rw-rw---- because this would reflect the default umask of 027, i.e, not to create anything world readable. For those who need more permissions, they can supply a custom string.
   > 
   > The problem with this is that it makes the default behaviour between windows and unix inconsistent, and this is likely to cause headaches for people who either don't read the docs properly, or read a response on stack overflow aimed at unix people and use it thinking it also applies to windows.
   
   While I agree here, you cannot really achieve consistency due to two completely diametral approach in both OS types. I wouldn't try to achive, as sad as it sounds.
   
   > Setting a default on windows is itself hard - windows doesn't have a concept of a "primary group" like posix, but the possibility of zero or more users and/or groups that have access to a file or directory. There is no practical default behaviour for any of that, which is why java itself doesn't try. Java gives you "access to owner" and "access to everyone", and that's it. "Access to owner" is the same as "no uds support", that leaves just "access to everyone, protect me by protecting my parent directory".
   
   I know it is hard, maybe we should not try at all? I believe that it will quite some time to be picked up by Windows users at all.
   
   > > I also do understand that localhost is open for everyone on that box, but isn't that the whole point of UDS to have more control over the socket?
   > 
   > Yes - and the most simplest way to protect a socket is to put it in a suitably protected directory. You don't have to protect the socket file itself, just make it impossible for the file to be seen by making its parent directory inaccessible.
   > 
   > I am very mindful of decisions made now being difficult to change down the line. Adding new behaviour in future is easy, but changing existing behaviour (like a default) is a headache for all concerned.
   
   Agree!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532682814



##########
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##########
@@ -568,6 +569,14 @@ public final int getLocalPort() {
     protected abstract InetSocketAddress getLocalAddress() throws IOException;
 
 
+    /**
+     * Address for the unix domain socket.
+     */
+    private Path path;

Review comment:
       The underlying API uses "path" as the reference to file that will become the socket:
   
   https://linux.die.net/man/7/unix
   
   ```
   #define UNIX_PATH_MAX    108
   
   struct sockaddr_un {
       sa_family_t sun_family;               /* AF_UNIX */
       char        sun_path[UNIX_PATH_MAX];  /* pathname */
   };
   ```
   
   Naming it something else introduces new/inconsistent terminology.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735896671


   > Nobody has cared about UDS for the past 15 years. And PEM files are supported for JSSE and JSSE/OpenSSL. So I don't understand what the problem is.
   
   While you may not have cared, 389ds LDAP does UDS, all the milters and the various in postfix do UDS, as well as most web applications based on FastCGI, as does Windows 10 and Windows Server 2019.
   
   As explained already, the problem is getting access to the filesystem permission model.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r540754092



##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">

Review comment:
       At the moment the `port` parameter is mandatory, i.e. if `path` is specified then both TCP and UDS will be exposed.
   I guess there will be users which will want to enable **only** UDS.
   Do you think this is reasonable and we should make it possible to disable TCP in case UDS is enabled ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532981406



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       Path used as is.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750260041


   @rmaucher https://github.com/apache/tomcat/commit/884b997f5a9a7da9f696d00574d3b727afbfae8c#diff-117ff4ae372c7a4f6643546174bcc2dbf5a25bd399fe1b89f55e72d2d4150285R212


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532274717



##########
File path: java/org/apache/tomcat/util/IntrospectionUtils.java
##########
@@ -136,6 +138,19 @@ public static boolean setProperty(Object o, String name, String value,
                         if (actualMethod != null) {
                             actualMethod.append(method.getName()).append("(InetAddress.getByName(\"").append(value).append("\"))");
                         }
+                        // Try a setFoo ( Path )
+                    } else if ("java.nio.file.Path".equals(paramType
+                            .getName())) {
+                        try {
+                            params[0] = Paths.get(value);
+                        } catch (InvalidPathException ipe) {
+                            if (log.isDebugEnabled())
+                                log.debug("IntrospectionUtils: Unable to resolve path:" + value);

Review comment:
       I've fixed this one.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532584495



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -177,6 +177,9 @@ private Library(String libraryName)
     /* Is the O_NONBLOCK flag inherited from listening sockets?
      */
     public static boolean APR_O_NONBLOCK_INHERITED  = false;
+    /* Support for Unix Domain Sockets.
+     */
+    public static boolean APR_HAVE_UNIX           = false;

Review comment:
       I had the same on my mind, but this is the define from APR. It should be consistent. Look into the header files.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r533571858



##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The
+      permissions are specified as a string of nine characters, in three sets
+      of three: (r)ead, (w)rite and e(x)ecute for owner, group and everyone
+      else respectively. If a permission is not granted, a dash is used. If

Review comment:
       Done.

##########
File path: webapps/docs/config/http.xml
##########
@@ -1130,6 +1148,38 @@
   </subsection>
 
 
+  <subsection name="Unix Domain Socket Support">
+
+  <p>When the <code>path</code> attribute is used, connectors that support
+  unix domain sockets will bind to the socket at the given path. The first
+  connector to support this is the
+  <code>org.apache.coyote.http11.Http11AprProtocol</code> connector when
+  used with the Apache Tomcat Native library v1.2.26 and up, along with
+  Apache Portable Runtime v1.6 and higher.
+  </p>
+
+  <p>The socket path is created with read and write permissions for all
+  users. To protect this socket, place it in a directory with suitable
+  permissions appropriately configured to restrict access as required.
+  Alternatively, on platforms that support posix permissions, the
+  permissions on the socket can be set directly with the
+  <code>pathPermissions</code> option.
+  </p>
+
+  <p>Tomcat will automatically remove the socket on server shutdown. If the
+  socket already exists startup will fail. Care must be taken by the
+  administrator to remove the socket after verifying that the socket isn't
+  already being used by an existing Tomcat process.</p>
+
+  <p>The unix domain socket can be accessed using the

Review comment:
       Done.

##########
File path: webapps/docs/config/http.xml
##########
@@ -1130,6 +1148,38 @@
   </subsection>
 
 
+  <subsection name="Unix Domain Socket Support">
+
+  <p>When the <code>path</code> attribute is used, connectors that support
+  unix domain sockets will bind to the socket at the given path. The first

Review comment:
       Done.

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The
+      permissions are specified as a string of nine characters, in three sets
+      of three: (r)ead, (w)rite and e(x)ecute for owner, group and everyone

Review comment:
       Done.

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this
+      <strong>Connector</strong> will create and await incoming connections.
+      Tomcat will automatically remove the socket on server shutdown. If the
+      socket already exists, care must be taken by the administrator to remove
+      the socket after verifying that the socket isn't already being used by an
+      existing Tomcat process.</p>
+    </attribute>
+
+    <attribute name="pathPermissions" required="false">
+      <p>Where supported, the posix permissions that will be applied to the
+      to the unix domain socket specified with <code>path</code> above. The

Review comment:
       Done.

##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">
+      <p>Where supported, the path to a unix domain socket that this

Review comment:
       Done.

##########
File path: webapps/docs/changelog.xml
##########
@@ -124,6 +124,11 @@
         uses the correct setting for the secure attribute for any session
         cookies it creates. Based on a pull request by Andreas Kurth. (markt)
       </fix>
+      <add>
+        <bug>64943</bug>: Add support for unix domain sockets to

Review comment:
       Done.

##########
File path: java/org/apache/tomcat/util/net/LocalStrings.properties
##########
@@ -88,6 +88,7 @@ endpoint.init.bind=Socket bind failed: [{0}] [{1}]
 endpoint.init.bind.inherited=No inherited channel while the connector was configured to use one
 endpoint.init.listen=Socket listen failed: [{0}] [{1}]
 endpoint.init.notavail=APR not available
+endpoint.init.unixnotavail=Unix domain socket support not available

Review comment:
       Done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532732287



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -247,7 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
-            APR_HAVE_UNIX             = has(22);
+            APR_HAVE_UNIX             = has(21);

Review comment:
       Done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532265195



##########
File path: java/org/apache/tomcat/util/IntrospectionUtils.java
##########
@@ -136,6 +138,19 @@ public static boolean setProperty(Object o, String name, String value,
                         if (actualMethod != null) {
                             actualMethod.append(method.getName()).append("(InetAddress.getByName(\"").append(value).append("\"))");
                         }
+                        // Try a setFoo ( Path )
+                    } else if ("java.nio.file.Path".equals(paramType
+                            .getName())) {
+                        try {
+                            params[0] = Paths.get(value);
+                        } catch (InvalidPathException ipe) {
+                            if (log.isDebugEnabled())
+                                log.debug("IntrospectionUtils: Unable to resolve path:" + value);

Review comment:
       Not following, [..] ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532540970



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -177,6 +177,9 @@ private Library(String libraryName)
     /* Is the O_NONBLOCK flag inherited from listening sockets?
      */
     public static boolean APR_O_NONBLOCK_INHERITED  = false;
+    /* Support for Unix Domain Sockets.
+     */
+    public static boolean APR_HAVE_UNIX           = false;

Review comment:
       `APR_HAVE_UNIX` sound unclear to me. Maybe `APR_HAVE_UNIX_DOMAIN_SOCKET` or `APR_HAVE_UDS` ?!

##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {
+            getPath().toFile().setReadable(true, false);
+            getPath().toFile().setWritable(true, false);
+            getPath().toFile().setExecutable(false, false);

Review comment:
       `getPath().toFile()` could be cached in a local variable

##########
File path: java/org/apache/tomcat/util/net/AbstractEndpoint.java
##########
@@ -568,6 +569,14 @@ public final int getLocalPort() {
     protected abstract InetSocketAddress getLocalAddress() throws IOException;
 
 
+    /**
+     * Address for the unix domain socket.
+     */
+    private Path path;

Review comment:
       s/path/udsAddress/ ?!

##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -244,6 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
+            APR_HAVE_UNIX             = has(22);

Review comment:
       Why not `21` ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532272524



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       We're not, no, as the permissions on the directory containing the socket come into play:
   
   ```
   Little-Net:tomcat-socket10 minfrin$ ls -al /tmp/protected/
   total 0
   drwxr-x---   3 minfrin  wheel   96 Nov 29 23:39 .
   drwxrwxrwt  11 root     wheel  352 Nov 29 23:39 ..
   srw-rw-rw-   1 minfrin  wheel    0 Nov 29 23:39 tomcat.socket
   ```
   
   Only minfrin and members of the wheel group can read/write the socket above.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532903806



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       I've updated it, now looks like this:
   
   ```
   30-Nov-2020 23:05:45.172 INFO [main] org.apache.coyote.AbstractProtocol.start Starting ProtocolHandler ["http-nio-8080"]
   30-Nov-2020 23:05:45.180 INFO [main] org.apache.coyote.AbstractProtocol.start Starting ProtocolHandler ["http-apr-/tmp/protected/tomcat.socket"]
   ```
   
   Is this all ok?
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-759497608


   > * The permission attribute, is it really useful ?
   
   In the absence of a permission attribute (and without the "everyone" default), the socket is equivalent to a TCP port that has been firewalled off, and thus pointless.
   
   Ignoring special cases like a personal development environment, or a system with no user separation, daemons (like tomcat) are secured with a user tomcat, group tomcat, and a typical umask of 0750 (or some variation). This means that the a) the tomcat user can write, b) the tomcat group can read (typically allowing read access to log files), and c) everyone else get nothing.
   
   In order for any unix domain socket to be of use to anyone, it must be possible to write to it. If you can't write to it, you cannot submit a request. A unix domain socket that only the tomcat user can write to pointless, as you've giving the client control over the tomcat process. A read only unix domain socket for a request/response protocol like HTTP has no practical effect - having written nothing you will read nothing.
   
   For this reason, every daemon out there that I have seen has a mechanism to make the socket writable to a group, and defaulting to being accessible to everyone:
   
   https://github.com/Cisco-Talos/clamav-devel/blob/31824a659dff37ae03e3419395bb68e659c2b165/etc/clamd.conf.sample#L104
   
   https://github.com/trusteddomainproject/OpenDMARC/blob/b0d6408d0859adb336428e3d0bd87749513a9e79/opendmarc/opendmarc.conf.sample#L357
   
   https://github.com/rspamd/rspamd/blob/9c2d72c6eba3fc05fd7459e388ea7c92eb87095f/conf/options.inc#L48
   
   In the absence of an explicit control over permissions, making the permissions world writable by default allows the admin to secure the socket by restricting permissions on the parent directory, such as the following example:
   
   ```
   [root@localhost clamav-milter]# ls -al
   total 0
   drwx--x---.  2 clamilt clamilt   60 Jan 11 13:03 .
   drwxr-xr-x. 39 root    root    1080 Jan 11 13:06 ..
   srw-rw-rw-.  1 clamilt clamilt    0 Jan 11 13:03 clamav-milter.socket
   ```
   
   In the above, the socket itself is world writable, but the parent directory is protected, and therefore the socket is protected.
   
   > * The socket is not deleted on shutdown (although the channel is closed)
   
   If the socket is not deleted on shutdown, the server cannot subsequently be started up. Deleting the socket on shutdown is the most common behaviour. Deleting the socket is startup is not done, as it means that multiple daemons can be started without error.
   
   I think I saw a commit go past fixing this, need to verify.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-743173688


   Here are the results for load testing APR protocol over TCP, both Tomcat and Vegeta running on the same machine:
   
   ```
   echo "GET http://localhost:8080/testbed/plaintext" | vegeta attack -rate 0 -max-workers 128 -duration 3
   0s | vegeta encode | vegeta report --type json | jq .
   {
     "latencies": {
       "total": 2993301754968,
       "mean": 2558679,
       "50th": 2097687,
       "90th": 5000518,
       "95th": 5898497,
       "99th": 8044202,
       "max": 151339083,
       "min": 71830
     },
     "bytes_in": {
       "total": 14038344,
       "mean": 12
     },
     "bytes_out": {
       "total": 0,
       "mean": 0
     },
     "earliest": "2020-12-11T12:45:29.355079583Z",
     "latest": "2020-12-11T12:45:59.355129897Z",
     "end": "2020-12-11T12:45:59.356131037Z",
     "duration": 30000050314,
     "wait": 1001140,
     "requests": 1169862,
     "rate": 38995.3345996245,
     "throughput": 38994.03331892302,
     "success": 1,
     "status_codes": {
       "200": 1169862
     },
     "errors": []
   }
   ```
   
   TCP: 38994
   UDS: 43379


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532256127



##########
File path: java/org/apache/tomcat/util/IntrospectionUtils.java
##########
@@ -136,6 +138,19 @@ public static boolean setProperty(Object o, String name, String value,
                         if (actualMethod != null) {
                             actualMethod.append(method.getName()).append("(InetAddress.getByName(\"").append(value).append("\"))");
                         }
+                        // Try a setFoo ( Path )
+                    } else if ("java.nio.file.Path".equals(paramType
+                            .getName())) {
+                        try {
+                            params[0] = Paths.get(value);
+                        } catch (InvalidPathException ipe) {
+                            if (log.isDebugEnabled())
+                                log.debug("IntrospectionUtils: Unable to resolve path:" + value);

Review comment:
       Use the common `[..]` pattern

##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       What umask will APR use to create the socket file?

##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();

Review comment:
       That hurts in terms of naming...

##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -347,22 +352,27 @@ public String getName() {
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
         name.append('-');
-        if (getAddress() != null) {
-            name.append(getAddress().getHostAddress());
-            name.append('-');
+        if (getPath() != null) {
+            name.append(getPath().getFileName().toString());

Review comment:
       Stupid question: What will happen when I bind two connectors on two different directory with the same socket file name?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] martin-g commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r540897626



##########
File path: webapps/docs/config/http.xml
##########
@@ -208,6 +208,24 @@
       The default is <code>POST</code></p>
     </attribute>
 
+    <attribute name="path" required="false">

Review comment:
       I've just tested it. It seems only UDS is enabled:
   
   ```
   openjdk version "16-ea" 2021-03-16
   OpenJDK Runtime Environment (build 16-ea+26-1764)
   OpenJDK 64-Bit Server VM (build 16-ea+26-1764, mixed mode)
   === Protocol: org.apache.coyote.http11.Http11AprProtocol
   === Starting : Connector[org.apache.coyote.http11.Http11AprProtocol-8080]
   Dec 11, 2020 11:55:52 AM org.apache.catalina.core.AprLifecycleListener lifecycleEvent
   INFO: Loaded Apache Tomcat Native library [1.2.27] using APR version [1.6.5].
   Dec 11, 2020 11:55:52 AM org.apache.catalina.core.AprLifecycleListener lifecycleEvent
   INFO: APR capabilities: IPv6 [true], sendfile [true], accept filters [false], random [true], UDS [true].
   Dec 11, 2020 11:55:52 AM org.apache.catalina.core.AprLifecycleListener initializeSSL
   INFO: OpenSSL successfully initialized [OpenSSL 1.1.1g-dev  xx XXX xxxx]
   Dec 11, 2020 11:55:52 AM org.apache.coyote.http11.AbstractHttp11Protocol configureUpgradeProtocol
   INFO: The ["https-openssl-apr-/tmp/tomcat-uds.sock"] connector has been configured to support negotiation to [h2] via ALPN
   Dec 11, 2020 11:55:52 AM org.apache.coyote.AbstractProtocol init
   INFO: Initializing ProtocolHandler ["https-openssl-apr-/tmp/tomcat-uds.sock"]
   Dec 11, 2020 11:55:52 AM org.apache.catalina.core.StandardService startInternal
   INFO: Starting service [Tomcat]
   Dec 11, 2020 11:55:52 AM org.apache.catalina.core.StandardEngine startInternal
   INFO: Starting Servlet engine: [Apache Tomcat/10.0.0-M11-dev]
   Dec 11, 2020 11:55:53 AM org.apache.coyote.AbstractProtocol start
   INFO: Starting ProtocolHandler ["https-openssl-apr-/tmp/tomcat-uds.sock"]
   === Started
   
   ```
   
   ```
   $  sudo netstat -anp | grep 2887991
   (standard input):35:unix  2      [ ACC ]     STREAM     LISTENING     14491510 2887991/java         /tmp/tomcat-uds.sock
   (standard input):165:unix  2      [ ]         STREAM     CONNECTED     14491508 2887991/java     
   ```
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750490202


   > 
   > 
   > It can 100% work with APR, except I personally don't want to add features to that component at this point.
   
   If you personally don't want to, @minfrin happily will.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-735887225


   > 
   > 
   > > > > That's correct, it was supposed to be dropped already in 10.0 [it will happen in 10.1]. Instead, it got some defaults changes so that using it requires more deliberate configuration.
   > > > 
   > > > 
   > > > At this stage JEP-380 is too far away for practical use, so having a library able to make native calls gives tomcat a significant edge.
   > > > The ability to use normal PEM files in the SSL configuration is also a significant benefit.
   > > 
   > > 
   > > I absolutely agree. This is so simple with APR/OpenSSL.
   > 
   > Nobody has cared about UDS for the past 15 years. And PEM files are supported for JSSE and JSSE/OpenSSL. So I don't understand what the problem is.
   
   Just because you never cared doesn't mean someone else does not. UDS is a fine thing on Unix. And for PEM files, they are supported because Tomcat supports it, not SunJSSE or OpenJSSE.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532862975



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       This is the name of the connector, for example:
   
   ```
   30-Nov-2020 20:33:08.007 INFO [main] org.apache.coyote.AbstractProtocol.start Starting ProtocolHandler ["http-nio-8080"]
   30-Nov-2020 20:33:08.015 INFO [main] org.apache.coyote.AbstractProtocol.start Starting ProtocolHandler ["http-apr-tmp-protected-tomcat.socket"]
   ```
   
   The name "http-apr-/tmp/protected/tomcat.socket" seems ugly to me.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r533484404



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Very nice. It's a pity that we cannot set the umask, but that's good too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532841983



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -351,12 +351,15 @@ public String getName() {
 
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
-        name.append('-');
         if (getPath() != null) {
-            name.append(getPath().getFileName().toString());
+            for (Path path: getPath()) {

Review comment:
       Any reason not to use the path as-is? Will this name be used as file name?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532667538



##########
File path: webapps/docs/config/http.xml
##########
@@ -212,7 +212,11 @@
       <p>Where supported, the path to a unix domain socket that this
       <strong>Connector</strong> will create and await incoming connections.
       The socket is created with world read and write permissions. To protect
-      the socket create the socket in a suitably protected directory.</p>
+      the socket create the socket in a suitably protected directory. Tomcat
+      will automatically remove the socket on server shutdown. If the socket
+      already exists, care must be taken by the administrator to remove the
+      socket after verifying that the socket isn't already being used by an
+      existing tomcat process.</p>

Review comment:
       Fixed.

##########
File path: webapps/docs/config/http.xml
##########
@@ -1152,6 +1156,11 @@
   permissions appropriately configured to restrict access as required.
   </p>
 
+  <p>Tomcat will automatically remove the socket on server shutdown. If the
+  socket already exists startup will fail. Care must be taken by the
+  administrator to remove the socket after verifying that the socket isn't
+  already being used by an existing tomcat process.</p>

Review comment:
       Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-743337472


   @minfrin Could you kindly add a test case for this? I would like to finalize this and checkout @martin-g's comments.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532669016



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -177,6 +177,9 @@ private Library(String libraryName)
     /* Is the O_NONBLOCK flag inherited from listening sockets?
      */
     public static boolean APR_O_NONBLOCK_INHERITED  = false;
+    /* Support for Unix Domain Sockets.
+     */
+    public static boolean APR_HAVE_UNIX           = false;

Review comment:
       Over at APR we call it APR_UNIX, it would be better to be consistent with this:
   
   https://github.com/apache/apr/blob/trunk/include/apr_network_io.h#L168
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532267259



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       The umask of the whole server, which is typically 0027 - not useful for clients that want group write access.
   
   I went on a mission trying to figure out how to handle permissions. Java7+ has the PosixFilePermissions which is great for unix, but not windows. Java7+ also has full access to being able to set permissions via java.nio.file.attribute.AttributeView, but no easy way to create this from a string.
   
   I eventually decided that a simple "explicitly set the socket as world read/write" would do without being mindbendingly complex, the admin can control access to the socket by restricting the parent directory the socket is placed in, outside of the scope of tomcat.
   
   Seems like a reasonable compromise.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-736674567


   > * A few nits in docs.
   > * I wonder whether we should set default permissions at all and rely on the umask.
   > * Tomcat has a umask check (startup listener) which these default permissions we basically break that promise...
   
   Relying on the umask makes no practical sense, unfortunately.
   
   The typical umask is 0027, meaning full access for tomcat itself, read access for members of the tomcat group (so that logfiles can be read but not changed), and no access for anyone else.
   
   The unix domain socket is useless if you can't write to it. What that means is that only the tomcat user can send requests to tomcat, and members of the tomcat group can't send requests at all, which is completely pointless.
   
   To be in any way useful the socket must be writable, and to do that it either needs to default to being writable, or needs to explicitly set as writable with at least `pathPermissions="rw-rw----"`.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532716306



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       I know, and I don't expect one, but I don't see a problem to have `pathPermissions` for POSIX only for now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532522222



##########
File path: java/org/apache/coyote/AbstractProtocol.java
##########
@@ -347,22 +352,27 @@ public String getName() {
     private String getNameInternal() {
         StringBuilder name = new StringBuilder(getNamePrefix());
         name.append('-');
-        if (getAddress() != null) {
-            name.append(getAddress().getHostAddress());
-            name.append('-');
+        if (getPath() != null) {
+            name.append(getPath().getFileName().toString());

Review comment:
       I think we should use the full path and I will explain why. The name contains the addresss where the socket is bound. TCP sockets contains IP and port which makes then fully idenfiable. UDS have they full path. I would use it, not think about it. Windows uses a special path for UDS.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532271195



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       But if you set the whole socket as world read/write you are back to the same situation as before with localhost?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532265524



##########
File path: java/org/apache/tomcat/util/IntrospectionUtils.java
##########
@@ -136,6 +138,19 @@ public static boolean setProperty(Object o, String name, String value,
                         if (actualMethod != null) {
                             actualMethod.append(method.getName()).append("(InetAddress.getByName(\"").append(value).append("\"))");
                         }
+                        // Try a setFoo ( Path )
+                    } else if ("java.nio.file.Path".equals(paramType
+                            .getName())) {
+                        try {
+                            params[0] = Paths.get(value);
+                        } catch (InvalidPathException ipe) {
+                            if (log.isDebugEnabled())
+                                log.debug("IntrospectionUtils: Unable to resolve path:" + value);

Review comment:
       All log statements in Tomcat guard placeholders with `[]`, e.g., `"Did not find file [foo.xml]"`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] michael-o commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532284639



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Not necessarily:
   ```
   $ ll /var/run/
   total 100
   -rw-r--r--  1 root   wheel       0 2020-11-29 11:45 clean_var
   -rw-------  1 root   wheel       4 2020-11-29 10:45 cron.pid
   drwxr-xr-x  3 root   wheel     512 2020-07-12 11:56 cups/
   drwxr-xr-x  2 root   wheel     512 2020-10-16 19:12 dbus/
   -rw-------  1 root   wheel       3 2020-11-29 10:45 devd.pid
   srw-rw-rw-  1 root   wheel       0 2020-11-29 10:45 devd.pipe=
   srw-rw-rw-  1 root   wheel       0 2020-11-29 10:45 devd.seqpacket.pipe=
   drwxr-xr-x  2 root   wheel     512 2020-11-29 10:45 dhclient/
   -rw-r--r--  1 root   wheel    8067 2020-11-29 10:45 dmesg.boot
   -rw-r--r--  1 root   wheel       5 2020-11-29 10:45 httpd.pid
   -r--r--r--  1 root   wheel     310 2020-11-29 11:45 ld-elf.so.hints
   -r--r--r--  1 root   wheel     139 2020-11-29 11:45 ld-elf32.so.hints
   srw-rw-rw-  1 root   wheel       0 2020-11-29 10:45 log=
   srw-------  1 root   wheel       0 2020-11-29 10:45 logpriv=
   drwxr-x---  2 nexus  nexus     512 2020-09-29 12:54 nexus2/
   -r--r--r--  1 root   wheel     220 2020-11-29 10:45 os-release
   drwxr-xr-x  2 root   wheel     512 2020-07-23 00:26 poudriere/
   -rw-------  1 root   wheel       3 2020-11-29 10:45 powerd.pid
   drwxrwx---  2 root   network   512 2020-07-02 04:39 ppp/
   drwxr-xr-x  2 redis  redis     512 2020-11-29 10:45 redis/
   drwxr-xr-x  4 root   wheel     512 2020-11-30 00:14 resolvconf/
   -rw-r--r--  1 root   wheel       0 2020-11-29 10:45 rtsold.dump
   -rw-r--r--  1 root   wheel       3 2020-11-29 10:45 rtsold.pid
   -rw-------  1 root   wheel      79 2020-11-29 10:45 sendmail.pid
   -rw-r--r--  1 root   wheel       5 2020-11-29 10:45 sshd.pid
   drwx--x--x  3 root   wheel     512 2020-07-23 01:12 sudo/
   -rw-------  1 root   wheel       3 2020-11-29 10:45 syslog.pid
   -rw-r--r--  1 root   wheel       0 2020-11-29 10:45 syslogd.sockets
   drwx------  2 _tss   _tss      512 2020-07-12 15:33 tpm/
   -rw-r--r--  1 root   wheel     788 2020-11-29 20:03 utx.active
   drwxr-xr-x  2 root   wheel     512 2020-07-02 04:39 wpa_supplicant/
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532283784



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       Using additional directories is almost universal, this is the /var/run directory on a CentOS8 machine:
   
   ```
   [root@host run]# ls -al /var/run/
   total 20
   drwxr-xr-x. 37 root      root        980 Nov 22 03:04 .
   dr-xr-xr-x. 17 root      root        248 Oct 19 22:35 ..
   srw-rw-rw-.  1 root      root          0 Nov 22 03:03 .heim_org.h5l.kcm-socket
   drwxr-xr-x.  3 root      root        100 Nov 22 03:03 NetworkManager
   -rw-------.  1 root      root          0 Nov 22 03:03 agetty.reload
   -rw-r--r--.  1 root      root          4 Nov 22 03:03 auditd.pid
   drwxr-x---.  2 chrony    chrony       80 Nov 30 01:08 chrony
   drwxr-xr-x.  2 root      root         80 Nov 22 03:03 chrony-helper
   drwx--x---.  2 clamilt   clamilt      60 Nov 22 03:04 clamav-milter
   drwx--x---.  2 clamscan  virusgroup   60 Nov 22 03:04 clamd.scan
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 console
   ----------.  1 root      root          0 Nov 22 03:03 cron.reboot
   drwx------.  2 root      root         40 Nov 22 03:03 cryptsetup
   drwxr-xr-x.  2 root      root         60 Nov 22 03:03 dbus
   drwxrwx---.  2 dirsrv    dirsrv       80 Nov 22 03:04 dirsrv
   prw-------.  1 root      root          0 Nov 22 03:03 dmeventd-client
   prw-------.  1 root      root          0 Nov 22 03:03 dmeventd-server
   drwxr-xr-x.  5 root      dovecot     800 Nov 22 03:17 dovecot
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 faillock
   drwxr-x---.  2 root      root         40 Nov 22 03:03 firewalld
   drwxr-xr-x.  2 root      root         80 Nov 22 03:03 fsck
   drwx--x---.  3 root      apache      100 Nov 29 03:14 httpd
   prw-------.  1 root      root          0 Nov 22 03:03 initctl
   drwxr-xr-x.  4 root      root        120 Nov 22 03:03 initramfs
   drwxr-xr-x.  5 root      root        120 Nov 22 03:03 lock
   drwxr-xr-x.  3 root      root         60 Nov 22 03:03 log
   drwx------.  4 root      root        120 Nov 22 03:03 lvm
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 mount
   drwxr-x---.  2 openarc   openarc      80 Nov 22 03:03 openarc
   drwxr-x---.  2 opendkim  opendkim     80 Nov 22 03:03 opendkim
   drwxr-x---.  2 opendmarc opendmarc    80 Nov 22 03:03 opendmarc
   drwxr-xr-x.  2 root      root         60 Nov 22 03:03 pcscd
   drwxr-xr-x.  2 redis     redis        60 Nov 22 03:03 redis
   drwxr-xr-x.  2 _rspamd   _rspamd      80 Nov 22 03:03 rspamd
   -rw-------.  1 root      root          4 Nov 22 03:03 rsyslogd.pid
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 samba
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 sepermit
   drwxr-xr-x.  2 root      root         40 Nov 22 03:03 setrans
   srw-rw-rw-.  1 root      root          0 Nov 22 03:04 slapd-beachfront.socket
   -rw-r--r--.  1 root      root          5 Nov 22 03:03 sshd.pid
   -rw-------.  1 root      root          4 Nov 22 03:03 sssd.pid
   drwx--x--x.  3 root      root         60 Nov 22 03:03 sudo
   drwxr-xr-x. 17 root      root        440 Nov 22 09:32 systemd
   drwxr-xr-x.  2 root      root         60 Nov 22 03:03 tmpfiles.d
   drwxr-xr-x.  2 root      root         60 Nov 22 03:03 tuned
   drwxr-xr-x.  7 root      root        160 Nov 22 03:03 udev
   drwxr-xr-x.  2 unbound   unbound      60 Nov 22 03:03 unbound
   drwxr-xr-x.  3 root      root         60 Nov 30 01:08 user
   -rw-rw-r--.  1 root      utmp       1920 Nov 30 01:08 utmp
   ```
   
   Directories that need privacy, like openarc, are protected (contains a socket).
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532674722



##########
File path: java/org/apache/tomcat/jni/Library.java
##########
@@ -244,6 +247,7 @@ public static synchronized boolean initialize(String libraryName) throws Excepti
             APR_CHARSET_EBCDIC      = has(18);
             APR_TCP_NODELAY_INHERITED = has(19);
             APR_O_NONBLOCK_INHERITED  = has(20);
+            APR_HAVE_UNIX             = has(22);

Review comment:
       I had this idea in my head that 21 had been used in an earlier version and removed. Can't find it now, let me change it.
   
   Changed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] minfrin commented on a change in pull request #382: Add support for unix domain sockets.

Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #382:
URL: https://github.com/apache/tomcat/pull/382#discussion_r532273756



##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -292,52 +295,79 @@ public void bind() throws Exception {
 
         // Create the pool for the server socket
         serverSockPool = Pool.create(rootPool);
+
         // Create the APR address that will be bound
-        String addressStr = null;
-        if (getAddress() != null) {
-            addressStr = getAddress().getHostAddress();
-        }
-        int family = Socket.APR_INET;
-        if (Library.APR_HAVE_IPV6) {
-            if (addressStr == null) {
-                if (!OS.IS_BSD) {
+        if (getPath() != null) {
+            if (Library.APR_HAVE_UNIX) {
+                hostname = getPath().toString();
+                family = Socket.APR_UNIX;
+            }
+            else {
+                throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+            }
+        }
+        else {
+
+            if (getAddress() != null) {
+                hostname = getAddress().getHostAddress();
+            }
+            family = Socket.APR_INET;
+            if (Library.APR_HAVE_IPV6) {
+                if (hostname == null) {
+                    if (!OS.IS_BSD) {
+                        family = Socket.APR_UNSPEC;
+                    }
+                } else if (hostname.indexOf(':') >= 0) {
                     family = Socket.APR_UNSPEC;
                 }
-            } else if (addressStr.indexOf(':') >= 0) {
-                family = Socket.APR_UNSPEC;
             }
-         }
+        }
+
+        long sockAddress = Address.info(hostname, family, getPortWithOffset(), 0, rootPool);
 
-        long inetAddress = Address.info(addressStr, family, getPortWithOffset(), 0, rootPool);
         // Create the APR server socket
-        serverSock = Socket.create(Address.getInfo(inetAddress).family,
+        if (family == Socket.APR_UNIX) {
+            serverSock = Socket.create(family, Socket.SOCK_STREAM, 0, rootPool);
+        }
+        else {
+            serverSock = Socket.create(Address.getInfo(sockAddress).family,
                 Socket.SOCK_STREAM,
                 Socket.APR_PROTO_TCP, rootPool);
-        if (OS.IS_UNIX) {
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
-        }
-        if (Library.APR_HAVE_IPV6) {
-            if (getIpv6v6only()) {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
-            } else {
-                Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+            if (OS.IS_UNIX) {
+                Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+            }
+            if (Library.APR_HAVE_IPV6) {
+                if (getIpv6v6only()) {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 1);
+                } else {
+                    Socket.optSet(serverSock, Socket.APR_IPV6_V6ONLY, 0);
+                }
             }
+            // Deal with the firewalls that tend to drop the inactive sockets
+            Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
         }
-        // Deal with the firewalls that tend to drop the inactive sockets
-        Socket.optSet(serverSock, Socket.APR_SO_KEEPALIVE, 1);
+
         // Bind the server socket
-        int ret = Socket.bind(serverSock, inetAddress);
+        int ret = Socket.bind(serverSock, sockAddress);
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.bind", "" + ret, Error.strerror(ret)));
         }
+
         // Start listening on the server socket
         ret = Socket.listen(serverSock, getAcceptCount());
         if (ret != 0) {
             throw new Exception(sm.getString("endpoint.init.listen", "" + ret, Error.strerror(ret)));
         }
-        if (OS.IS_WIN32 || OS.IS_WIN64) {
-            // On Windows set the reuseaddr flag after the bind/listen
-            Socket.optSet(serverSock, Socket.APR_SO_REUSEADDR, 1);
+
+        if (family == Socket.APR_UNIX) {

Review comment:
       A further thing to keep in mind - this implementation is mindful of the JEP-380 work on java as raised here: https://bz.apache.org/bugzilla/show_bug.cgi?id=64850
   
   In future, JEP-380 will allow UDS on wIndows and unix, and I have made sure that nothing done here closes the door on future support.
   
   The https://docs.oracle.com/javase/7/docs/api/java/nio/file/Path.html object is used as the parameter type as it exists in jdks as old as java7+ and it is used in the constructor of the proposed java.net.UnixDomainSocketAddress.
   
   This should allow seamless JEP-380 support in future, while giving me my UDS today.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org