gnodet commented on code in PR #26205:
URL: https://github.com/apache/camel/pull/26205#discussion_r3958732289
##########
components/camel-oauth/src/test/java/org/apache/camel/test/oauth/SSLCertTrustTest.java:
##########
@@ -78,13 +141,17 @@ void testCheckKeycloakCertificateTrust() {
@Test
void testUntrustedCertificate() {
- String url = "https://untrusted-root.badssl.com";; // Example of an
untrusted cert
- Assertions.assertThrows(SSLHandshakeException.class, () ->
connectToUrl(url), "Certificate should not be trusted");
+ // Connect to local HTTPS server whose self-signed cert is NOT in the
default trust store
+ String url = "https://localhost:"; + localHttpsPort;
+ Assertions.assertThrows(SSLHandshakeException.class, () ->
connectToUrl(url),
+ "Certificate should not be trusted");
}
private static void connectToUrl(String httpsUrl) throws IOException {
var url = URI.create(httpsUrl).toURL();
var con = (HttpsURLConnection) url.openConnection();
+ con.setConnectTimeout(10_000);
+ con.setReadTimeout(10_000);
Review Comment:
```suggestion
con.setReadTimeout(READ_TIMEOUT_MS);
```
##########
components/camel-oauth/src/test/java/org/apache/camel/test/oauth/SSLCertTrustTest.java:
##########
@@ -78,13 +141,17 @@ void testCheckKeycloakCertificateTrust() {
@Test
void testUntrustedCertificate() {
- String url = "https://untrusted-root.badssl.com";; // Example of an
untrusted cert
- Assertions.assertThrows(SSLHandshakeException.class, () ->
connectToUrl(url), "Certificate should not be trusted");
+ // Connect to local HTTPS server whose self-signed cert is NOT in the
default trust store
+ String url = "https://localhost:"; + localHttpsPort;
+ Assertions.assertThrows(SSLHandshakeException.class, () ->
connectToUrl(url),
+ "Certificate should not be trusted");
}
private static void connectToUrl(String httpsUrl) throws IOException {
var url = URI.create(httpsUrl).toURL();
var con = (HttpsURLConnection) url.openConnection();
+ con.setConnectTimeout(10_000);
Review Comment:
⚠️ **Regression:** The current `main` branch (commit `575405797e2`, "Fix
flaky test SSLCertTrustTest.testUntrustedCertificate") introduced named
constants `CONNECT_TIMEOUT_MS = 10_000` and `READ_TIMEOUT_MS = 10_000` at the
top of the class to make the values discoverable and adjustable without hunting
through method bodies. This PR inlines the literals again, undoing that
intentional refactor.
Restore the constants — they belong at class level, not inline:
```suggestion
con.setConnectTimeout(CONNECT_TIMEOUT_MS);
```
##########
components/camel-oauth/src/test/java/org/apache/camel/test/oauth/SSLCertTrustTest.java:
##########
@@ -39,6 +45,63 @@ class SSLCertTrustTest extends AbstractKeycloakTest {
private static final Logger LOG =
LoggerFactory.getLogger(SSLCertTrustTest.class);
+ /** PKCS12 keystore containing a self-signed certificate not in any
default trust store. */
+ private static final String SELFSIGNED_KEYSTORE =
"selfsigned-keystore.p12";
+ private static final String KEYSTORE_PASSWORD = "changeit";
+
+ private static SSLServerSocket serverSocket;
+ private static Thread serverThread;
+ private static int localHttpsPort;
+
+ @BeforeAll
+ static void startLocalHttpsServer() throws Exception {
+ // Load the self-signed keystore from test resources
+ KeyStore ks = KeyStore.getInstance("PKCS12");
+ try (InputStream is =
SSLCertTrustTest.class.getClassLoader().getResourceAsStream(SELFSIGNED_KEYSTORE))
{
+ Assertions.assertNotNull(is, "Test keystore not found on
classpath: " + SELFSIGNED_KEYSTORE);
+ ks.load(is, KEYSTORE_PASSWORD.toCharArray());
+ }
+
+ KeyManagerFactory kmf =
KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ kmf.init(ks, KEYSTORE_PASSWORD.toCharArray());
+
+ SSLContext sslContext = SSLContext.getInstance("TLS");
+ sslContext.init(kmf.getKeyManagers(), null, null);
+
+ serverSocket = (SSLServerSocket)
sslContext.getServerSocketFactory().createServerSocket(0);
+ localHttpsPort = serverSocket.getLocalPort();
+
+ // Accept connections in a daemon thread — just complete TLS handshake
and respond
+ serverThread = new Thread(() -> {
+ while (!serverSocket.isClosed()) {
+ try (var socket = serverSocket.accept()) {
+ // Read enough to satisfy the HTTP request, then send a
minimal response
+ socket.getInputStream().read(new byte[1]);
+ socket.getOutputStream().write("HTTP/1.1 200
OK\r\nContent-Length: 0\r\n\r\n".getBytes());
Review Comment:
⚠️ **Charset:** `String.getBytes()` without an explicit charset uses the
platform default, which could theoretically differ from ASCII on exotic JVM
configurations. HTTP header text is pure ASCII — use an explicit charset:
```suggestion
socket.getOutputStream().write("HTTP/1.1 200
OK\r\nContent-Length:
0\r\n\r\n".getBytes(java.nio.charset.StandardCharsets.US_ASCII));
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
