Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8251188: Update LDAP tests not to use wildcard addresses #252

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions test/jdk/ProblemList.txt
Expand Up @@ -889,8 +889,6 @@ sun/tools/jhsdb/BasicLauncherTest.java 8211767 linux-pp

# jdk_other

com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942 linux-i586,macosx-all,windows-x64

javax/rmi/ssl/SSLSocketParametersTest.sh 8162906 generic-all

javax/script/Test7.java 8239361 generic-all
Expand Down
23 changes: 20 additions & 3 deletions test/jdk/com/sun/jndi/ldap/BalancedParentheses.java
Expand Up @@ -24,18 +24,23 @@
/**
* @test
* @bug 6449574
* @library /test/lib
* @summary Invalid ldap filter is accepted and processed
*/

import java.io.*;
import javax.naming.*;
import javax.naming.directory.*;
import java.util.Properties;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Hashtable;

import java.net.Socket;
import java.net.ServerSocket;

import jdk.test.lib.net.URIBuilder;

public class BalancedParentheses {
// Should we run the client or server in a separate thread?
//
Expand All @@ -54,7 +59,13 @@ public class BalancedParentheses {
// If the server prematurely exits, serverReady will be set to true
// to avoid infinite hangs.
void doServerSide() throws Exception {
ServerSocket serverSock = new ServerSocket(serverPort);
// Create unbound server socket
ServerSocket serverSock = new ServerSocket();

// And bind it to the loopback address
SocketAddress sockAddr = new InetSocketAddress(
InetAddress.getLoopbackAddress(), 0);
serverSock.bind(sockAddr);

// signal client, it's ready to accecpt connection
serverPort = serverSock.getLocalPort();
Expand Down Expand Up @@ -106,7 +117,13 @@ void doClientSide() throws Exception {
Hashtable<Object, Object> env = new Hashtable<>();
env.put(Context.INITIAL_CONTEXT_FACTORY,
"com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.PROVIDER_URL, "ldap://localhost:" + serverPort);
// Construct the provider URL
String providerURL = URIBuilder.newBuilder()
.scheme("ldap")
.loopback()
.port(serverPort)
.build().toString();
env.put(Context.PROVIDER_URL, providerURL);
env.put("com.sun.jndi.ldap.read.timeout", "1000");

// env.put(Context.SECURITY_AUTHENTICATION, "simple");
Expand Down
128 changes: 69 additions & 59 deletions test/jdk/com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java
Expand Up @@ -21,34 +21,32 @@
* questions.
*/

/**
/*
* @test
* @run main/othervm DeadSSLLdapTimeoutTest
* @bug 8141370
* @key intermittent
* @library /test/lib
* @run main/othervm DeadSSLLdapTimeoutTest
*/

import java.net.Socket;
import java.io.EOFException;
import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.SocketAddress;
import java.net.SocketTimeoutException;
import java.io.*;
import javax.naming.*;
import javax.naming.directory.*;
import java.util.List;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
import java.util.Hashtable;
import java.util.ArrayList;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.CountDownLatch;
import javax.naming.directory.InitialDirContext;
import javax.net.ssl.SSLHandshakeException;

import static java.util.concurrent.TimeUnit.MILLISECONDS;
import jdk.test.lib.net.URIBuilder;

import static java.util.concurrent.TimeUnit.NANOSECONDS;


Expand All @@ -57,26 +55,26 @@ class DeadServerTimeoutSSLTest implements Callable<Boolean> {
Hashtable<Object, Object> env;
DeadSSLServer server;
boolean passed = false;
private int HANGING_TEST_TIMEOUT = 20_000;

public DeadServerTimeoutSSLTest(Hashtable<Object, Object> env) throws IOException {
this.server = new DeadSSLServer();
SocketAddress sockAddr = new InetSocketAddress(
InetAddress.getLoopbackAddress(), 0);
this.server = new DeadSSLServer(sockAddr);
this.env = env;
}

public void performOp(InitialContext ctx) throws NamingException {}

public void handleNamingException(NamingException e, long start, long end) {
public void handleNamingException(NamingException e) {
if (e.getCause() instanceof SocketTimeoutException
|| e.getCause().getCause() instanceof SocketTimeoutException) {
// SSL connect will timeout via readReply using
// SocketTimeoutException
e.printStackTrace();
System.err.println("PASS: Observed expected SocketTimeoutException");
pass();
} else if (e.getCause() instanceof SSLHandshakeException
&& e.getCause().getCause() instanceof EOFException) {
// test seems to be failing intermittently on some
// platforms.
System.err.println("PASS: Observed expected SSLHandshakeException/EOFException");
pass();
} else {
fail(e);
Expand All @@ -92,6 +90,7 @@ public void fail() {
}

public void fail(Exception e) {
System.err.println("FAIL: Unexpected exception was observed:" + e.getMessage());
throw new RuntimeException("Test failed", e);
}

Expand All @@ -106,29 +105,31 @@ boolean shutItDown(InitialContext ctx) {

public Boolean call() {
InitialContext ctx = null;
ScheduledFuture<?> killer = null;
long start = System.nanoTime();

try {
while(!server.accepting())
Thread.sleep(200); // allow the server to start up
server.serverStarted.await(); // Wait for the server to start-up
Thread.sleep(200); // to be sure

env.put(Context.PROVIDER_URL, "ldap://localhost:" +
server.getLocalPort());
env.put(Context.PROVIDER_URL,
URIBuilder.newBuilder()
.scheme("ldap")
.loopback()
.port(server.getLocalPort())
.buildUnchecked().toString()
);

long start = System.nanoTime();
try {
ctx = new InitialDirContext(env);
performOp(ctx);
fail();
} catch (NamingException e) {
long end = System.nanoTime();
System.out.println(this.getClass().toString() + " - elapsed: "
+ NANOSECONDS.toMillis(end - start));
handleNamingException(e, start, end);
handleNamingException(e);
} finally {
if (killer != null && !killer.isDone())
killer.cancel(true);
// Stop the server side thread
server.testDone.countDown();
shutItDown(ctx);
server.close();
}
Expand All @@ -141,50 +142,59 @@ public Boolean call() {

class DeadSSLServer extends Thread {
ServerSocket serverSock;
boolean accepting = false;

public DeadSSLServer() throws IOException {
this.serverSock = new ServerSocket(0);
// Latch to be used by client to wait for server to start
CountDownLatch serverStarted = new CountDownLatch(1);

// Latch to be used by server thread to wait for client to finish testing
CountDownLatch testDone = new CountDownLatch(1);

public DeadSSLServer(SocketAddress socketAddress) throws IOException {
// create unbound server socket
var srvSock = new ServerSocket();
// bind it to the address provided
srvSock.bind(socketAddress);
this.serverSock = srvSock;
start();
}

public void run() {
while(true) {
try {
accepting = true;
Socket socket = serverSock.accept();
} catch (Exception e) {
break;
}
// Signal client to proceed with the test
serverStarted.countDown();
try (var socket = serverSock.accept()) {
System.err.println("Accepted connection:" + socket);
// Give LDAP client time to fully establish the connection.
// When client is done - close the accepted socket
testDone.await();
} catch (Exception e) {
System.err.println("Server socket. Failure to accept connection:");
e.printStackTrace();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if removing the while (true) loop will make the test more susceptible of failing in timeout if the server ever receives a connection request from an unexpected client (we've seen that happening in the past with networking tests). Is there anyway the server could attempt to verify that the accepted socket is from the expected client, and close it and go back to accepting if it's not? Maybe by looking at the accepted socket remote address & port?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the good suggestion Daniel. I will modify it to look at the remote socket's address.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Daniel,

Could you please take a look at the new version of DeadSSLLdapTimeoutTest.java with the following modifications:
DeadServerTimeoutSSLTest test was modified to use custom SocketFactory (DeadSSLSocketFactory) which tracks the first opened socket on LDAP client side. This socket is later used on server side to check if the connection initiated by test's LDAP client. If the connection was not established by LDAP client then new accept attempt is performed.

The test was never seen to fail during 200+ concurrent LDAP tests runs.

}

public int getLocalPort() {
return serverSock.getLocalPort();
}

public boolean accepting() {
return accepting;
}

public void close() throws IOException {
serverSock.close();
}
}

public class DeadSSLLdapTimeoutTest {
// com.sun.jndi.ldap.connect.timeout value to set
static final String CONNECT_TIMEOUT_MS = "10";

// com.sun.jndi.ldap.read.timeout value to set
static final String READ_TIMEOUT_MS = "3000";

static Hashtable<Object, Object> createEnv() {
Hashtable<Object, Object> env = new Hashtable<>(11);
env.put(Context.INITIAL_CONTEXT_FACTORY,
"com.sun.jndi.ldap.LdapCtxFactory");
"com.sun.jndi.ldap.LdapCtxFactory");
return env;
}

public static void main(String[] args) throws Exception {

InitialContext ctx = null;

//
// Running this test serially as it seems to tickle a problem
// on older kernels
Expand All @@ -193,19 +203,19 @@ public static void main(String[] args) throws Exception {
// and ssl enabled
// this should exit with a SocketTimeoutException as the root cause
// it should also use the connect timeout instead of the read timeout
System.out.println("Running connect timeout test with 10ms connect timeout, 3000ms read timeout & SSL");
System.out.printf("Running connect timeout test with %sms connect timeout," +
" %sms read timeout & SSL%n",
CONNECT_TIMEOUT_MS, READ_TIMEOUT_MS);

Hashtable<Object, Object> sslenv = createEnv();
sslenv.put("com.sun.jndi.ldap.connect.timeout", "10");
sslenv.put("com.sun.jndi.ldap.read.timeout", "3000");
sslenv.put("com.sun.jndi.ldap.connect.timeout", CONNECT_TIMEOUT_MS);
sslenv.put("com.sun.jndi.ldap.read.timeout", READ_TIMEOUT_MS);
sslenv.put(Context.SECURITY_PROTOCOL, "ssl");
boolean testFailed =
(new DeadServerTimeoutSSLTest(sslenv).call()) ? false : true;
boolean testFailed = !new DeadServerTimeoutSSLTest(sslenv).call();

if (testFailed) {
throw new AssertionError("some tests failed");
}

}

}

25 changes: 21 additions & 4 deletions test/jdk/com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java
Expand Up @@ -27,7 +27,7 @@
* @summary Verify capability to add a new entry to the directory using the
* ADD operation.
* @modules java.naming/com.sun.jndi.ldap
* @library ../../lib/ /javax/naming/module/src/test/test/
* @library /test/lib ../../lib/ /javax/naming/module/src/test/test/
* @build LDAPServer LDAPTestUtils
* @run main/othervm AddNewEntry
*/
Expand All @@ -41,19 +41,36 @@
import javax.naming.directory.InitialDirContext;
import javax.naming.directory.SearchControls;
import javax.naming.directory.SearchResult;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.SocketAddress;
import java.util.Hashtable;
import jdk.test.lib.net.URIBuilder;

public class AddNewEntry {

public static void main(String[] args) throws Exception {
ServerSocket serverSocket = new ServerSocket(0);
// Create unbound server socket
ServerSocket serverSocket = new ServerSocket();

// Bind it to the loopback address
SocketAddress sockAddr = new InetSocketAddress(
InetAddress.getLoopbackAddress(), 0);
serverSocket.bind(sockAddr);

// Construct the provider URL for LDAPTestUtils
String providerURL = URIBuilder.newBuilder()
.scheme("ldap")
.loopback()
.port(serverSocket.getLocalPort())
.buildUnchecked().toString();

Hashtable<Object, Object> env;

// initialize test
env = LDAPTestUtils
.initEnv(serverSocket, AddNewEntry.class.getName(), args, true);
env = LDAPTestUtils.initEnv(serverSocket, providerURL,
AddNewEntry.class.getName(), args, true);

/* Build attribute set */
String[] ids = { "objectClass", "sn", "cn", "telephoneNumber", "mail",
Expand Down
16 changes: 11 additions & 5 deletions test/jdk/com/sun/jndi/ldap/lib/LDAPTestUtils.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020, 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
Expand Down Expand Up @@ -50,12 +50,17 @@ public class LDAPTestUtils {
* Process command line arguments and return properties in a Hashtable.
*/
public static Hashtable<Object, Object> initEnv(String testname,
String[] args) {
String[] args) {
return initEnv(null, testname, args, false);
}

public static Hashtable<Object, Object> initEnv(ServerSocket socket,
String testname, String[] args, boolean authInfo) {
String testname, String[] args, boolean authInfo) {
return initEnv(socket, null, testname, args, authInfo);
}

public static Hashtable<Object, Object> initEnv(ServerSocket socket, String providerUrl,
String testname, String[] args, boolean authInfo) {

Hashtable<Object, Object> env = new Hashtable<>();
String root = "o=IMC,c=US";
Expand Down Expand Up @@ -103,8 +108,9 @@ public static Hashtable<Object, Object> initEnv(ServerSocket socket,
if (socket != null) {
env.put(TEST_LDAP_SERVER_THREAD,
startLDAPServer(socket, getCaptureFile(testname)));
env.put("java.naming.provider.url",
"ldap://localhost:" + socket.getLocalPort());
String url = providerUrl != null ? providerUrl :
"ldap://localhost:" + socket.getLocalPort();
env.put("java.naming.provider.url", url);
} else {
// for tests which run against remote server or no server
// required
Expand Down