# HG changeset patch # User chegar # Date 1551350291 0 # Node ID 86e1d9d76ef4a24ae4d41d9ddade2b4aaa47a877 # Parent 28b0946d3b8112b547182ca3f1edce7cbb36e265 niosocketimpl-branch: a non-platform impl cannot accept a connection with a platform impl diff -r 28b0946d3b81 -r 86e1d9d76ef4 src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java --- 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 diff -r 28b0946d3b81 -r 86e1d9d76ef4 src/java.base/share/classes/java/net/ServerSocket.java --- 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 } /** diff -r 28b0946d3b81 -r 86e1d9d76ef4 src/java.base/share/classes/sun/net/PlatformSocketImpl.java --- 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 diff -r 28b0946d3b81 -r 86e1d9d76ef4 src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java --- 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 diff -r 28b0946d3b81 -r 86e1d9d76ef4 test/jdk/java/net/SocketImpl/BadSocketImpls.java --- /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 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 void assertThrows(Class 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"); + } +} diff -r 28b0946d3b81 -r 86e1d9d76ef4 test/jdk/java/net/SocketImpl/SocketImplCombinations.java --- 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);