From 892dd12a16f9daee6656ccfe16e2033cbea68d71 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:46:42 +0800 Subject: [PATCH] [NET-735] Apply the read timeout before implicit IMAPS TLS The implicit handshake starts before the superclass applies socket attributes, so a configured default read timeout does not bound the handshake. Apply the existing helper first, matching FTPSClient. Add loopback regressions for the configured and unlimited timeouts, explicit STARTTLS runtime timeout, and layered socket inheritance. Credit John van der Kamp for the original diagnosis and workaround. Generated-by: DeepSeek (deepseek-v4-flash) Generated-by: Codex --- src/changes/changes.xml | 1 + .../apache/commons/net/imap/IMAPSClient.java | 1 + .../net/imap/IMAPSClientTimeoutTest.java | 327 ++++++++++++++++++ 3 files changed, 329 insertions(+) create mode 100644 src/test/java/org/apache/commons/net/imap/IMAPSClientTimeoutTest.java diff --git a/src/changes/changes.xml b/src/changes/changes.xml index da1552f31..5eb7ca389 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -63,6 +63,7 @@ The type attribute can be add,update,fix,remove. + Apply the configured read timeout before the implicit IMAPS TLS handshake. Reject CR/LF in SimpleSMTPHeader and SimpleNNTPHeader fields #397. Reject CR/LF in IMAP quoteMailboxName (#399). Reject CR/LF in FTP, POP3, NNTP and SMTP command senders (#400). diff --git a/src/main/java/org/apache/commons/net/imap/IMAPSClient.java b/src/main/java/org/apache/commons/net/imap/IMAPSClient.java index 162d6164c..3f85bc3d0 100644 --- a/src/main/java/org/apache/commons/net/imap/IMAPSClient.java +++ b/src/main/java/org/apache/commons/net/imap/IMAPSClient.java @@ -176,6 +176,7 @@ public IMAPSClient(final String proto, final boolean implicit, final SSLContext protected void _connectAction_() throws IOException { // Implicit mode. if (isImplicit) { + applySocketAttributes(); performSSLNegotiation(); } super._connectAction_(); diff --git a/src/test/java/org/apache/commons/net/imap/IMAPSClientTimeoutTest.java b/src/test/java/org/apache/commons/net/imap/IMAPSClientTimeoutTest.java new file mode 100644 index 000000000..a5054d6bc --- /dev/null +++ b/src/test/java/org/apache/commons/net/imap/IMAPSClientTimeoutTest.java @@ -0,0 +1,327 @@ +/* + * 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.commons.net.imap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.BufferedReader; +import java.io.Closeable; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; + +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + +import org.junit.jupiter.api.Test; + +/** Checks configured read timeouts during implicit and explicit IMAPS TLS handshakes. */ +class IMAPSClientTimeoutTest { + + private static final int READ_TIMEOUT = 150; + + private static final long AWAIT_MILLIS = 3000; + + private static final ThreadFactory DAEMONS = new ThreadFactory() { + + public Thread newThread(final Runnable runnable) { + final Thread thread = new Thread(runnable, "imaps-timeout-peer"); + thread.setDaemon(true); + return thread; + } + }; + + @Test + void implicitConnectUsesTheDefaultReadTimeoutForTheHandshake() throws Exception { + final IMAPSClient client = new IMAPSClient(true); + client.setConnectTimeout(300); + client.setDefaultTimeout(READ_TIMEOUT); + final Loopback loopback = new Loopback(false); + try { + final Future call = loopback.execute(new ClientAction() { + + public void run() throws IOException { + client.connect(loopback.address(), loopback.port()); + } + }); + assertReadTimedOut(awaitFailure(call, AWAIT_MILLIS)); + loopback.assertTlsStageReached(); + } finally { + try { + loopback.close(); + } finally { + client.disconnect(); + } + } + } + + @Test + void explicitStartTlsKeepsTheReadTimeoutSetAfterConnect() throws Exception { + final IMAPSClient client = new IMAPSClient(false); + final Loopback loopback = new Loopback(true); + try { + final Future call = loopback.execute(new ClientAction() { + + public void run() throws IOException { + client.connect(loopback.address(), loopback.port()); + assertEquals(0, client.getSoTimeout(), "explicit mode must connect without a read timeout"); + client.setSoTimeout(READ_TIMEOUT); + assertEquals(READ_TIMEOUT, client.getSoTimeout(), "setSoTimeout must apply the read timeout to the socket"); + client.execTLS(); + } + }); + assertReadTimedOut(awaitFailure(call, AWAIT_MILLIS)); + loopback.assertTlsStageReached(); + try { + assertEquals(READ_TIMEOUT, client.getSoTimeout(), "the TLS command must not reset the caller's read timeout"); + } catch (final SocketException socketClosedByFailedHandshake) { + // The failed handshake closed the socket, so the stored value can no longer be read back. + } + } finally { + try { + loopback.close(); + } finally { + client.disconnect(); + } + } + } + + @Test + void implicitConnectWithoutReadTimeoutKeepsWaiting() throws Exception { + final IMAPSClient client = new IMAPSClient(true); + client.setConnectTimeout(150); + client.setDefaultTimeout(0); + final Loopback loopback = new Loopback(false); + try { + final Future call = loopback.execute(new ClientAction() { + + public void run() throws IOException { + client.connect(loopback.address(), loopback.port()); + } + }); + try { + final IOException premature = call.get(400, TimeUnit.MILLISECONDS); + fail("the handshake read must keep waiting when no read timeout is configured, but the call ended with: " + + premature); + } catch (final TimeoutException expected) { + // No read timeout was configured, so the handshake read is still pending. + } + loopback.assertTlsStageReached(); + } finally { + try { + loopback.close(); + } finally { + client.disconnect(); + } + } + } + + @Test + void layeredSslSocketReportsTheUnderlyingReadTimeout() throws Exception { + final ServerSocket server = new ServerSocket(0, 1, InetAddress.getLoopbackAddress()); + Socket raw = null; + Socket accepted = null; + try { + raw = new Socket(); + raw.connect(new InetSocketAddress(server.getInetAddress(), server.getLocalPort()), 1000); + accepted = server.accept(); + raw.setSoTimeout(READ_TIMEOUT); + final SSLSocketFactory factory = (SSLSocketFactory) SSLSocketFactory.getDefault(); + final SSLSocket layered = (SSLSocket) factory.createSocket(raw, server.getInetAddress().getHostAddress(), + server.getLocalPort(), true); + assertEquals(READ_TIMEOUT, layered.getSoTimeout(), + "a layered SSLSocket must report the read timeout of the socket it wraps"); + } finally { + closeQuietly(raw); + closeQuietly(accepted); + closeQuietly(server); + } + } + + private static void assertReadTimedOut(final IOException failure) { + Throwable cause = failure; + while (cause != null) { + if (cause instanceof SocketTimeoutException) { + return; + } + cause = cause.getCause(); + } + fail("expected the pending TLS handshake read to time out, but the call failed with: " + failure); + } + + private static IOException awaitFailure(final Future call, final long timeoutMillis) throws Exception { + final IOException failure; + try { + failure = call.get(timeoutMillis, TimeUnit.MILLISECONDS); + } catch (final TimeoutException stillBlocked) { + return fail("the client call was still blocked after " + timeoutMillis + + " ms; the TLS handshake read did not observe the configured read timeout"); + } catch (final ExecutionException e) { + final Throwable cause = e.getCause(); + if (cause instanceof AssertionError) { + throw (AssertionError) cause; + } + throw new AssertionError("the client call failed with an unexpected exception", cause); + } + if (failure == null) { + return fail("the client call completed without an I/O failure; expected a socket read timeout"); + } + return failure; + } + + private static void closeQuietly(final Closeable closeable) { + if (closeable != null) { + try { + closeable.close(); + } catch (final IOException ignored) { + // Cleanup only. + } + } + } + + private static void hold(final Socket socket, final CountDownLatch tlsStage) { + try { + final InputStream in = socket.getInputStream(); + if (in.read() >= 0) { + // Real TLS bytes have arrived; the handshake is unambiguously in progress. + tlsStage.countDown(); + } + while (in.read() >= 0) { + // Keep reading, but never reply to the TLS handshake. + } + } catch (final IOException connectionClosed) { + // The client or the test closed the connection; stop holding it open. + } + } + + private interface ClientAction { + + void run() throws IOException; + } + + private static final class Loopback implements Closeable { + + private final ServerSocket server; + private final ExecutorService executor; + private final AtomicReference accepted = new AtomicReference(); + private final CountDownLatch tlsStage = new CountDownLatch(1); + private final Future peer; + + Loopback(final boolean explicit) throws IOException { + server = new ServerSocket(0, 1, InetAddress.getLoopbackAddress()); + executor = Executors.newFixedThreadPool(2, DAEMONS); + peer = executor.submit(new Callable() { + + public Void call() throws Exception { + serve(explicit); + return null; + } + }); + } + + InetAddress address() { + return server.getInetAddress(); + } + + int port() { + return server.getLocalPort(); + } + + Future execute(final ClientAction action) { + return executor.submit(new Callable() { + + public IOException call() throws Exception { + try { + action.run(); + return null; + } catch (final IOException e) { + return e; + } + } + }); + } + + /** Asserts that the peer reached the TLS stage, and surfaces any peer side assertion failure. */ + void assertTlsStageReached() throws Exception { + final boolean reached = tlsStage.await(2, TimeUnit.SECONDS); + if (peer.isDone()) { + try { + peer.get(); + } catch (final ExecutionException e) { + throw new AssertionError("the loopback peer did not answer as expected", e.getCause()); + } + } + assertTrue(reached, "the client never reached the TLS handshake stage"); + } + + public void close() { + closeQuietly(accepted.get()); + closeQuietly(server); + executor.shutdownNow(); + try { + executor.awaitTermination(5, TimeUnit.SECONDS); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private void serve(final boolean explicit) throws Exception { + final Socket socket = server.accept(); + accepted.set(socket); + if (explicit) { + final OutputStream out = socket.getOutputStream(); + out.write("* OK loopback IMAPS peer ready\r\n".getBytes(StandardCharsets.US_ASCII)); + out.flush(); + final BufferedReader in = new BufferedReader( + new InputStreamReader(socket.getInputStream(), StandardCharsets.US_ASCII)); + final String command = in.readLine(); + final int space = command == null ? -1 : command.indexOf(' '); + if (space < 0) { + throw new AssertionError("expected a tagged STARTTLS command, but received: " + command); + } + final String tag = command.substring(0, space); + if (!"STARTTLS".equalsIgnoreCase(command.substring(space + 1).trim())) { + throw new AssertionError("expected STARTTLS to be requested, but received: " + command); + } + out.write((tag + " OK begin TLS negotiation\r\n").getBytes(StandardCharsets.US_ASCII)); + out.flush(); + } + hold(socket, tlsStage); + } + } +}