niosocketimpl-branch: a non-platform impl cannot accept a connection with a platform impl
--- a/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java Tue Feb 26 11:14:51 2019 +0000
+++ b/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java Thu Feb 28 10:38:11 2019 +0000
@@ -732,16 +732,6 @@
socketClose0(false);
}
-
- @Override
- public void postCustomAccept() {
- assert fd.valid() && localport != 0 && address != null && port != 0;
- stream = true;
-
- // assume the custom SocketImpl didn't register a cleaner
- SocketCleanable.register(fd);
- }
-
@Override
public void copyTo(SocketImpl si) {
// this SocketImpl should be connected
--- a/src/java.base/share/classes/java/net/ServerSocket.java Tue Feb 26 11:14:51 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java Thu Feb 28 10:38:11 2019 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1995, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -598,15 +598,25 @@
*/
private void implAccept(SocketImpl si) throws IOException {
assert !(si instanceof DelegatingSocketImpl);
+
+ // A non-platform SocketImpl cannot accept a connection with a platform
+ // SocketImpl
+ if (!(impl instanceof PlatformSocketImpl) && si instanceof PlatformSocketImpl) {
+ throw new IOException("An instance of " + impl.getClass() +
+ " cannot accept a connection with an instance of " + si.getClass());
+ }
+
impl.accept(si);
- try {
- // a custom impl has accepted the connection with a platform SocketImpl
- if (!(impl instanceof PlatformSocketImpl) && (si instanceof PlatformSocketImpl)) {
- ((PlatformSocketImpl) si).postCustomAccept();
+
+ if (si instanceof PlatformSocketImpl) {
+ var fd = si.fd;
+ if (fd == null || !fd.valid() || si.localport <= 0 ||
+ si.address == null || si.port <= 0) {
+ throw new IOException("Invalid accepted state:" + si);
}
- } finally {
- securityCheckAccept(si); // closes si if permission check fails
}
+
+ securityCheckAccept(si); // closes si if permission check fails
}
/**
--- a/src/java.base/share/classes/sun/net/PlatformSocketImpl.java Tue Feb 26 11:14:51 2019 +0000
+++ b/src/java.base/share/classes/sun/net/PlatformSocketImpl.java Thu Feb 28 10:38:11 2019 +0000
@@ -24,7 +24,6 @@
*/
package sun.net;
-import java.io.IOException;
import java.net.SocketImpl;
/**
@@ -34,12 +33,6 @@
public interface PlatformSocketImpl {
/**
- * Invoked by ServerSocket to fix up the SocketImpl state after a connection
- * is accepted by a custom SocketImpl
- */
- void postCustomAccept() throws IOException;
-
- /**
* Copy the state from this connected SocketImpl to a target SocketImpl. If
* the target SocketImpl is not a newly created SocketImpl then it is first
* closed to release any resources. The target SocketImpl becomes the owner
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Tue Feb 26 11:14:51 2019 +0000
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Thu Feb 28 10:38:11 2019 +0000
@@ -465,23 +465,6 @@
}
/**
- * For use by ServerSocket to set the state and other fields after a
- * connection is accepted by a ServerSocket using a custom SocketImpl.
- * The protected fields defined by SocketImpl should be set.
- */
- @Override
- public void postCustomAccept() throws IOException {
- synchronized (stateLock) {
- assert state == ST_NEW;
- assert fd.valid() && localport != 0 && address != null && port != 0;
- IOUtil.configureBlocking(fd, true);
- stream = true;
- closer = FileDescriptorCloser.create(this);
- state = ST_CONNECTED;
- }
- }
-
- /**
* For use by ServerSocket to copy the state from this connected SocketImpl
* to a target SocketImpl. If the target SocketImpl is not a newly created
* SocketImpl then it is first closed to release any resources. The target
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/SocketImpl/BadSocketImpls.java Thu Feb 28 10:38:11 2019 +0000
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Combinations of bad SocketImpls
+ * @run testng/othervm BadSocketImpls
+ * @run testng/othervm -Djdk.net.usePlainSocketImpl BadSocketImpls
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.SocketAddress;
+import java.net.SocketImpl;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+import static java.lang.String.format;
+import static java.lang.System.out;
+import static org.testng.Assert.fail;
+
+public class BadSocketImpls {
+
+ Class<IOException> IOE = IOException.class;
+
+ /**
+ * Tests that a server-side custom socket impl, whose accept is a no-op,
+ * does not have any adverse negative effects. Default accept impl.
+ */
+ @Test
+ public void testNoOpAccept() throws IOException {
+ ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) { };
+ try (ss) {
+ ss.bind(new InetSocketAddress(0));
+ assertThrows(IOE, ss::accept);
+ }
+ }
+
+ /**
+ * Tests that a server-side custom socket impl, whose accept is a no-op,
+ * does not have any adverse negative effects. Custom accept, client has
+ * an explicit null impl.
+ */
+ @Test
+ public void testNoOpAcceptWithNullClientImpl() throws IOException {
+ ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) {
+ @Override
+ public Socket accept() throws IOException {
+ Socket s = new Socket((SocketImpl)null) { };
+ implAccept(s);
+ return s;
+ }
+ };
+ try (ss) {
+ ss.bind(new InetSocketAddress(0));
+ assertThrows(IOE, ss::accept);
+ }
+ }
+
+ /**
+ * Tests that a server-side custom socket impl, whose accept is a no-op,
+ * does not have any adverse negative effects. Custom accept, client has
+ * a platform impl.
+ */
+ @Test
+ public void testNoOpAcceptWithPlatformClientImpl() throws IOException {
+ ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) {
+ @Override
+ public Socket accept() throws IOException {
+ Socket s = new Socket();
+ implAccept(s);
+ return s;
+ }
+ };
+ try (ss) {
+ ss.bind(new InetSocketAddress(0));
+ assertThrows(IOE, ss::accept);
+ }
+ }
+
+ static class NoOpSocketImpl extends SocketImpl {
+ @Override protected void create(boolean b) { }
+ @Override protected void connect(String s, int i) { }
+ @Override protected void connect(InetAddress inetAddress, int i) { }
+ @Override protected void connect(SocketAddress socketAddress, int i) { }
+ @Override protected void bind(InetAddress inetAddress, int i) { }
+ @Override protected void listen(int i) { }
+ @Override protected void accept(SocketImpl socket) { }
+ @Override protected InputStream getInputStream() { return null; }
+ @Override protected OutputStream getOutputStream() { return null; }
+ @Override protected int available() { return 0; }
+ @Override protected void close() { }
+ @Override protected void sendUrgentData(int i) { }
+ @Override public void setOption(int i, Object o) { }
+ @Override public Object getOption(int i) { return null; }
+ }
+
+ static <T extends Throwable> void assertThrows(Class<T> throwableClass,
+ ThrowingRunnable runnable) {
+ try {
+ runnable.run();
+ fail(format("Expected %s to be thrown, but nothing was thrown",
+ throwableClass.getSimpleName()));
+ } catch (Throwable t) {
+ if (!throwableClass.isInstance(t)) {
+ fail(format("Expected %s to be thrown, but %s was thrown",
+ throwableClass.getSimpleName(),
+ t.getClass().getSimpleName()),
+ t);
+ }
+ out.println("caught expected exception: " + t);
+ }
+ }
+
+ interface ThrowingRunnable {
+ void run() throws Throwable;
+ }
+
+ @BeforeMethod
+ public void breakBetweenTests() {
+ System.out.println("\n-------\n");
+ }
+}
--- a/test/jdk/java/net/SocketImpl/SocketImplCombinations.java Tue Feb 26 11:14:51 2019 +0000
+++ b/test/jdk/java/net/SocketImpl/SocketImplCombinations.java Thu Feb 28 10:38:11 2019 +0000
@@ -361,50 +361,10 @@
/**
* Test ServerSocket.accept returning a Socket that initially doesn't have a
- * SocketImpl. The ServerSocket uses a custom SocketImpl.
- */
- public void testServerSocketAccept5() throws Exception {
- Socket socket = new Socket((SocketImpl) null) { };
- assertTrue(getSocketImpl(socket) == null);
-
- SocketImpl serverImpl = new CustomSocketImpl(true);
- serverSocketAccept(serverImpl, socket, (ss, s) -> {
- assertTrue(getSocketImpl(ss) == serverImpl);
- assertTrue(s == socket);
- SocketImpl si = getSocketImpl(s);
- assertTrue(isPlatformSocketImpl(si));
- checkFields(si);
- });
- }
-
-
- /**
- * Test ServerSocket.accept returning a Socket that has an existing
- * SocketImpl. The ServerSocket uses a custom SocketImpl.
- */
- public void testServerSocketAccept6() throws Exception {
- var socket = new Socket();
- SocketImpl si = getSocketImpl(socket);
- assertTrue(isSocksSocketImpl(si));
- SocketImpl delegate = getDelegate(si);
- assertTrue(isPlatformSocketImpl(delegate));
-
- SocketImpl serverImpl = new CustomSocketImpl(true);
- serverSocketAccept(serverImpl, socket, (ss, s) -> {
- assertTrue(getSocketImpl(ss) == serverImpl);
- assertTrue(s == socket);
- assertTrue(getSocketImpl(s) == si);
- assertTrue(getDelegate(si) == delegate);
- checkFields(delegate);
- });
- }
-
- /**
- * Test ServerSocket.accept returning a Socket that initially doesn't have a
* SocketImpl. A SocketImplFactory is set to create custom SocketImpls.
* The ServerSocket also uses custom SocketImpl.
*/
- public void testServerSocketAccept7() throws Exception {
+ public void testServerSocketAccept5() throws Exception {
var socket = new Socket((SocketImpl) null) { };
assertTrue(getSocketImpl(socket) == null);
@@ -423,7 +383,7 @@
* SocketImpl. A SocketImplFactory is set to create custom SocketImpls.
* The ServerSocket also uses custom SocketImpl.
*/
- public void testServerSocketAccept8() throws Exception {
+ public void testServerSocketAccept6() throws Exception {
SocketImpl si = new CustomSocketImpl(false);
Socket socket = new Socket(si) { };
assertTrue(getSocketImpl(socket) == si);