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 2021/01/15 17:16:58 UTC
[GitHub] [tomcat] minfrin opened a new pull request #401: Add support for unix domain sockets (APR Protocol)
minfrin opened a new pull request #401:
URL: https://github.com/apache/tomcat/pull/401
Add support and test case for Unix Domain Sockets in
org.apache.coyote.http11.Http11AprProtocol, to support
users using Java versions older than 16.
Requires tomcat-native 1.2.26 and 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] martin-g commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770727714
@michael-o Are new PRs needed ? We can cherry-pick the commits to the other branches
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-769728053
Thanks for 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] martin-g commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770738772
@michael-o You just pushed two commits to master: https://github.com/apache/tomcat/commits/master:
* https://github.com/apache/tomcat/commit/a616bf385a350175a33a0ebf09d8b6688344e9e3 (I guess this is a squashed one)
* https://github.com/apache/tomcat/commit/c4f5e2c9ccd7d65953e3c685d25e436e8330e142
I'd expect only those two to be backported.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-767869298
Alright, I will merge this this week.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-771777818
Yes, let's do 9.0.x first. @rmaucher has added an acceptable solution for Windows. This should be ported to the `AprEndpoint` 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] minfrin commented on a change in pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564045870
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
+ // 100ms == 100,000,000ns
+ private static final long ERROR_MARGIN = 100000000;
+
+ private long pool = 0;
+ private long serverSocket = 0;
+ private long clientSocket = 0;
+
+ @Before
+ public void init() throws Exception {
+ Assume.assumeTrue(Library.APR_HAVE_UNIX);
+ pool = Pool.create(0);
Review comment:
This is 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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-767651363
For me yes, but I want Rémy give a chance to do his improvements on top.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770716609
@minfrin I have now pushed this to master. Can you please backport to 9.0.x and 8.5.x and then I will merge them 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] minfrin commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-771765720
> Backported to 9.0.x, "ant test-apr" passes.
>
> #402
In trying to backport to 8.5.x, there are some further commits needing backporting to address conflicts:
https://github.com/minfrin/tomcat/commit/6945b88b1224b87e6a4ead667a8bb3640974cc03
https://github.com/minfrin/tomcat/commit/e99071f5c235e86e9781e8df9f62709d2e57cc03
https://github.com/minfrin/tomcat/commit/1d108f4ff477354ba065fc99d4f8c781af42bd6c
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564327772
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
Review comment:
Indentation?
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564327293
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
Leave it that way for consistency, maybe a comment above would help to clarify.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-771765720
> Backported to 9.0.x, "ant test-apr" passes.
>
> #402
In trying to backport to 8.5.x, there are some further commits needing backporting to address conflicts:
https://github.com/minfrin/tomcat/commit/6945b88b1224b87e6a4ead667a8bb3640974cc03
https://github.com/minfrin/tomcat/commit/e99071f5c235e86e9781e8df9f62709d2e57cc03
https://github.com/minfrin/tomcat/commit/1d108f4ff477354ba065fc99d4f8c781af42bd6c
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564480667
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
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] minfrin commented on a change in pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564042386
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
The APR term is hostname:
http://apr.apache.org/docs/apr/1.7/group__apr__network__io.html#gaa2f399ca2b60b35c0abf7630298c6c9f
So is the underlying getaddrinfo() function: https://en.wikipedia.org/wiki/Getaddrinfo
I can change it?
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
+ // 100ms == 100,000,000ns
+ private static final long ERROR_MARGIN = 100000000;
+
+ private long pool = 0;
+ private long serverSocket = 0;
+ private long clientSocket = 0;
+
+ @Before
+ public void init() throws Exception {
+ Assume.assumeTrue(Library.APR_HAVE_UNIX);
+ pool = Pool.create(0);
Review comment:
This is 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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-761842091
I still think it is a bad idea to continue to promote the APR connector. At least domain socket support won't be an excuse to keep it around longer than absolutely necessary.
Anyway, if this ends up being included, I will then refactor the feature a bit more.
----------------------------------------------------------------
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] markt-asf closed pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #401:
URL: 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] minfrin commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-767632095
In theory 96e241c should do the trick, can you confirm this is ready to merge?
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770733763
@martin-g Not really, but I do not fully know which are revelant. I'd rather merge a complete, blessed set rather then some halfbreed.
----------------------------------------------------------------
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] markt-asf commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-844365000
As far as I can tell, this PR was applied in a616bf38
--
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-761074064
Give me a couple of days and I will run this on FreeBSD and HP-UX.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564480836
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
I've added a comment to be clear.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770733763
@martin-g Not really, but I do not fully know which are revelant. I'd rathe merge a complete, blessed set rather then some halfbreed.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770906646
> @minfrin I have now pushed this to master. Can you please backport to 9.0.x and 8.5.x and then I will merge them too?
Backported to 9.0.x, "ant test-apr" passes.
https://github.com/apache/tomcat/pull/402
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r563890291
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
Why is this variable called `hostname`? The AF-agnostic name is `address`, isn't it?
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -296,52 +324,88 @@ 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 (getUnixDomainSocketPath() != null) {
+ if (Library.APR_HAVE_UNIX) {
+ hostname = getUnixDomainSocketPath();
+ family = Socket.APR_UNIX;
+ }
+ else {
+ throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+ }
+ }
+ else {
+
+ if (getAddress() != null) {
+ hostname = getAddress().getHostAddress();
Review comment:
same here, address != hostname.
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
+ // 100ms == 100,000,000ns
+ private static final long ERROR_MARGIN = 100000000;
+
+ private long pool = 0;
+ private long serverSocket = 0;
+ private long clientSocket = 0;
+
+ @Before
+ public void init() throws Exception {
+ Assume.assumeTrue(Library.APR_HAVE_UNIX);
+ pool = Pool.create(0);
Review comment:
Indentation is inconsistent.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-771777818
Yes, let's do 9.0.x first. @rmaucher has added an acceptable solution for Windows. This should be ported to the `AprEndpoint` 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] rmaucher commented on pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-767860861
I don't plan any improvements or changes at this time.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770748775
>
>
> @michael-o You just pushed two commits to master: https://github.com/apache/tomcat/commits/master:
>
> * [a616bf3](https://github.com/apache/tomcat/commit/a616bf385a350175a33a0ebf09d8b6688344e9e3) (I guess this is a squashed one)
>
> * [c4f5e2c](https://github.com/apache/tomcat/commit/c4f5e2c9ccd7d65953e3c685d25e436e8330e142)
>
>
> I'd expect only those two to be backported.
I tried with 9.0.x and compilation fails, precending commits are missing. That is why I requested a backport from @minfrin.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r563890291
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
Why is this variable called `hostname`? The AF-agnostic name is `address`, isn't it?
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -296,52 +324,88 @@ 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 (getUnixDomainSocketPath() != null) {
+ if (Library.APR_HAVE_UNIX) {
+ hostname = getUnixDomainSocketPath();
+ family = Socket.APR_UNIX;
+ }
+ else {
+ throw new Exception(sm.getString("endpoint.init.unixnotavail"));
+ }
+ }
+ else {
+
+ if (getAddress() != null) {
+ hostname = getAddress().getHostAddress();
Review comment:
same here, address != hostname.
##########
File path: test/org/apache/tomcat/jni/TestUnixDomainSocketServer.java
##########
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jni;
+
+import java.util.concurrent.CountDownLatch;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+/*
+ * Tests for unix domain sockets.
+ *
+ * While System.nanotime() is available and may have a resolution of ~100ns on
+ * some platforms, those same platforms do not use as precise a timer for socket
+ * timeouts. Therefore, a much larger error margin (100ms) is used.
+ *
+ * It is known that this larger error margin is required for Windows 10. It may
+ * be worth revisiting the choice of error margin once that platform is no
+ * longer supported.
+ *
+ * @deprecated The scope of the APR/Native Library will be reduced in Tomcat
+ * 10.1.x onwards to only those components required to provide
+ * OpenSSL integration with the NIO and NIO2 connectors.
+ */
+@Deprecated
+public class TestUnixDomainSocketServer extends AbstractJniTest {
+
+ private static final String PATH = System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "tomcat.socket";
+ // 100ms == 100,000,000ns
+ private static final long ERROR_MARGIN = 100000000;
+
+ private long pool = 0;
+ private long serverSocket = 0;
+ private long clientSocket = 0;
+
+ @Before
+ public void init() throws Exception {
+ Assume.assumeTrue(Library.APR_HAVE_UNIX);
+ pool = Pool.create(0);
Review comment:
Indentation is inconsistent.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on a change in pull request #401:
URL: https://github.com/apache/tomcat/pull/401#discussion_r564042386
##########
File path: java/org/apache/tomcat/util/net/AprEndpoint.java
##########
@@ -287,6 +312,9 @@ public int getSendfileCount() {
@Override
public void bind() throws Exception {
+ int family;
+ String hostname = null;
Review comment:
The APR term is hostname:
http://apr.apache.org/docs/apr/1.7/group__apr__network__io.html#gaa2f399ca2b60b35c0abf7630298c6c9f
So is the underlying getaddrinfo() function: https://en.wikipedia.org/wiki/Getaddrinfo
I can change 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 pull request #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
minfrin commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770829860
> @minfrin I have now pushed this to master. Can you please backport to 9.0.x and 8.5.x and then I will merge them too?
On the case, thank you.
----------------------------------------------------------------
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 #401: Add support for unix domain sockets (APR Protocol)
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #401:
URL: https://github.com/apache/tomcat/pull/401#issuecomment-770799039
>
>
> @michael-o You just pushed two commits to master: https://github.com/apache/tomcat/commits/master:
>
> * [a616bf3](https://github.com/apache/tomcat/commit/a616bf385a350175a33a0ebf09d8b6688344e9e3) (I guess this is a squashed one)
>
> * [c4f5e2c](https://github.com/apache/tomcat/commit/c4f5e2c9ccd7d65953e3c685d25e436e8330e142)
>
>
> I'd expect only those two to be backported.
A few fixups will follow, so they need to soak in then someone can cherry pick them to a branch and the squash.
----------------------------------------------------------------
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