gnodet commented on code in PR #26171:
URL: https://github.com/apache/camel/pull/26171#discussion_r3951816333
##########
components/camel-http/src/test/java/org/apache/camel/component/http/HttpProxyServerTest.java:
##########
@@ -143,6 +147,30 @@ public void httpGetWithProxyOnComponent() {
assertExchange(exchange);
}
+ @Test
+ public void httpsTargetWithProxyDefaultsToHttpProxyScheme() throws
Exception {
+ // CAMEL-24632: proxy scheme must default to "http" regardless of the
target endpoint scheme
+ HttpEndpoint endpoint = context.getEndpoint(
+ "https://www.example.com?proxyHost=myproxy&proxyPort=8080";,
HttpEndpoint.class);
+
+ HttpClientConfigurer configurer = endpoint.getHttpClientConfigurer();
+ assertThat(configurer).isNotNull();
+
+ HttpClientBuilder builder = HttpClientBuilder.create();
+ configurer.configureHttpClient(builder);
+
+ Field proxyField = HttpClientBuilder.class.getDeclaredField("proxy");
Review Comment:
⚠️ **Brittle test — accessing private third-party field via reflection.**
`getDeclaredField("proxy")` reads a private implementation detail of
`HttpClientBuilder` (Apache HttpClient 5). The field is named `proxy` in the
current version (5.5.1), but nothing in HttpClient 5's API contract guarantees
that name stays stable across minor/patch releases. If it changes, this test
fails with an opaque `NoSuchFieldException` rather than a meaningful assertion
failure.
A more robust approach is to exercise the configurer at the
`ProxyHttpClientConfigurer` level, which is internal to Camel and fully under
our control. `ProxyHttpClientConfigurer` already calls
`clientBuilder.setProxy(new HttpHost(scheme, host, port))` — if you can
instantiate `ProxyHttpClientConfigurer` directly (it's package-private, test is
in the same package), you can read the scheme from its own field rather than
from the builder:
```java
// Verify ProxyHttpClientConfigurer receives scheme="http"
// (constructor is package-private — accessible from same package)
ProxyHttpClientConfigurer proxyConfigurer = new ProxyHttpClientConfigurer(
"myproxy", 8080, "http", Collections.emptySet());
// Capture the proxy via a spy/interceptor or use the actual network test
below
```
Alternatively, keep the reflection but add a guard that catches
`NoSuchFieldException` and fails with a clear message:
```suggestion
Field proxyField;
try {
proxyField = HttpClientBuilder.class.getDeclaredField("proxy");
} catch (NoSuchFieldException e) {
fail("HttpClientBuilder.proxy field no longer exists — update
this test to match the new API");
return;
}
```
This won't prevent future failures, but the error message will tell the next
maintainer what happened instead of a raw `NoSuchFieldException`.
--
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]
