seshuthota commented on code in PR #28265:
URL: https://github.com/apache/camel/pull/28265#discussion_r4358571482
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/AS2ClientConnection.java:
##########
@@ -201,6 +239,10 @@ public HttpResponse send(ClassicHttpRequest request,
HttpCoreContext httpContext
endpoint =
leaseRequest.get(Timeout.ofSeconds(RETRIEVE_CONNECTION_TIMEOUT_SECONDS));
if (!endpoint.isConnected()) {
connectionPoolManager.connect(endpoint,
TimeValue.ofMilliseconds(connectionTimeoutMilliseconds), httpContext);
+ if (route.isTunnelled()) {
+ createTunnel(endpoint);
+ connectionPoolManager.upgrade(endpoint, httpContext);
+ }
}
Review Comment:
Confirmed and fixed in d6423fea3816, thanks. A failed connect, CONNECT or
TLS upgrade now closes the endpoint before rethrowing, as you suggested. I
reproduced it before fixing: with a wrong password, the second send wrote the
request in plain text to the proxy and returned no error, and real Squid 6.13
does the same. I left the older `finally` block as it is, since it also covers
the direct path. While checking it, I found that a partner answering
`Connection: close` hits the same pooling problem on the direct path. That
predates this PR, so I'll file it as its own JIRA ticket and fix it in a
separate PR.
_Claude Code on behalf of @seshuthota_
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/AS2ClientConnection.java:
##########
@@ -236,12 +278,48 @@ public ClassicHttpResponse execute(ClassicHttpRequest
request, HttpClientConnect
}
}
}
+ if (route.getProxyHost() != null && !route.isTunnelled()
+ && response.getCode() ==
HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED) {
+ // only the proxy answers 407: the message has not reached the AS2
partner
+ throw new HttpException(
+ "Proxy " + route.getProxyHost().toHostString() + " refused
to forward the request to "
+ + targetHost.toHostString() + ": " + new
StatusLine(response));
Review Comment:
Done in d6423fea3816: the 407 check now runs inside the `try` and closes the
endpoint before throwing. With Squid the reused connection happened to work,
but nothing guarantees a proxy leaves it clean, so it isn't reused any more.
_Claude Code on behalf of @seshuthota_
##########
components/camel-as2/camel-as2-component/src/test/java/org/apache/camel/component/as2/AS2ClientProxyTest.java:
##########
@@ -0,0 +1,365 @@
+/*
+ * 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.camel.component.as2;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ConnectException;
+import java.net.ServerSocket;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyStore;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.TrustManagerFactory;
+
+import org.apache.camel.CamelExecutionException;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.as2.api.AS2ClientConnection;
+import org.apache.camel.component.as2.api.AS2MediaType;
+import org.apache.camel.component.as2.api.AS2MessageStructure;
+import org.apache.camel.component.as2.api.AS2ServerConnection;
+import org.apache.camel.component.as2.api.AS2SignatureAlgorithm;
+import org.apache.camel.component.as2.api.entity.ApplicationEntity;
+import
org.apache.camel.component.as2.api.entity.DispositionNotificationMultipartReportEntity;
+import org.apache.camel.spi.Registry;
+import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.protocol.HttpContext;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * The AS2 client sends through an HTTP proxy: plain http in absolute form,
https through a CONNECT tunnel.
+ */
+public class AS2ClientProxyTest extends AbstractAS2ITSupport {
+
+ private static final String PROXY_USER = "proxyUser";
+ private static final String PROXY_PASSWORD = "proxyPassword";
+ private static final String PROXY_AUTHORIZATION = "Basic " +
Base64.getEncoder()
+ .encodeToString((PROXY_USER + ":" +
PROXY_PASSWORD).getBytes(StandardCharsets.UTF_8));
+ // a name the client cannot resolve (RFC 2606): only the proxy knows where
it is
+ private static final String PARTNER_BEHIND_PROXY = "as2-partner.invalid";
+ private static final String EDI_MESSAGE = """
+ UNB+UNOA:1+005435656:1+006415160:1+060515:1434+00000000000778'
+ UNH+00000000000117+INVOIC:D:97B:UN'
+ BGM+380+342459+9'
+ DTM+3:20060515:102'
+ UNT+23+00000000000117'
+ UNZ+1+00000000000778'
+ """;
+
+ private SSLContext sslContext;
+ private TestHttpProxy proxy;
+ private AS2ServerConnection httpServer;
+ private AS2ServerConnection httpsServer;
+ private CountingRequestHandler httpRequestHandler;
+ private CountingRequestHandler httpsRequestHandler;
+
+ @Override
+ public void setupResources() throws Exception {
+ sslContext = createSslContext();
+ proxy = new TestHttpProxy(PROXY_AUTHORIZATION);
+ proxy.addHostAlias(PARTNER_BEHIND_PROXY, "localhost");
+
+ httpServer = new AS2ServerConnection(
+ "1.1", "AS2ClientProxyTest Server", "server.example.com", 0,
AS2SignatureAlgorithm.SHA256WITHRSA,
+ null, null, null, "TBD", null, null, null, null, null);
+ httpRequestHandler = new CountingRequestHandler();
+ httpServer.listen("/", httpRequestHandler);
+
+ httpsServer = new AS2ServerConnection(
+ "1.1", "AS2ClientProxyTest Server", "server.example.com", 0,
AS2SignatureAlgorithm.SHA256WITHRSA,
+ null, null, null, "TBD", null, sslContext, null, null, null);
+ httpsRequestHandler = new CountingRequestHandler();
+ httpsServer.listen("/", httpsRequestHandler);
+ }
+
+ @Override
+ public void cleanupResources() {
+ if (httpServer != null) {
+ httpServer.close();
+ }
+ if (httpsServer != null) {
+ httpsServer.close();
+ }
+ if (proxy != null) {
+ proxy.close();
+ }
+ }
+
+ @Override
+ protected void bindToRegistry(Registry registry) {
+ registry.bind("tlsContext", sslContext);
+ }
+
+ @Test
+ public void sendsHttpThroughProxy() {
+ HttpEntity mdn = requestBodyAndHeaders("direct://HTTP", EDI_MESSAGE,
as2Headers());
+
+ assertInstanceOf(DispositionNotificationMultipartReportEntity.class,
mdn);
+ assertEquals(List.of("POST http://localhost:"; +
httpServer.getLocalPort() + "/ HTTP/1.1"),
+ proxy.getRequestLines());
+ assertEquals(List.of(PROXY_AUTHORIZATION),
proxy.getProxyAuthorizations());
+ assertReceived(httpRequestHandler);
+ }
+
+ @Test
+ public void reachesPartnerOnlyResolvableByTheProxy() {
+ HttpEntity mdn =
requestBodyAndHeaders("direct://HTTP_PARTNER_BEHIND_PROXY", EDI_MESSAGE,
as2Headers());
+
+ assertInstanceOf(DispositionNotificationMultipartReportEntity.class,
mdn);
+ assertEquals(List.of("POST http://"; + PARTNER_BEHIND_PROXY + ":" +
httpServer.getLocalPort() + "/ HTTP/1.1"),
+ proxy.getRequestLines());
+ assertReceived(httpRequestHandler);
+ }
+
+ @Test
+ public void refusedHttpRequestFailsTheSend() {
+ CamelExecutionException e = assertThrows(CamelExecutionException.class,
+ () -> requestBodyAndHeaders("direct://HTTP_WRONG_PASSWORD",
EDI_MESSAGE, as2Headers()));
+
+ assertTrue(rootMessage(e).contains("407"), rootMessage(e));
+ assertEquals(0, httpRequestHandler.getCount());
+ }
+
+ @Test
+ public void sendsHttpsThroughProxyTunnel() {
+ HttpEntity mdn = requestBodyAndHeaders("direct://HTTPS", EDI_MESSAGE,
as2Headers());
+
+ assertInstanceOf(DispositionNotificationMultipartReportEntity.class,
mdn);
+ assertEquals(List.of(httpsConnectLine()), proxy.getRequestLines());
+ assertEquals(List.of(PROXY_AUTHORIZATION),
proxy.getProxyAuthorizations());
+ HttpRequest request = assertReceived(httpsRequestHandler);
+ // the proxy credentials are for the proxy only: they must not travel
through the tunnel to the partner
+ assertNull(request.getFirstHeader("Proxy-Authorization"));
+ }
+
+ @Test
+ public void sendsSeveralMessagesThroughTheTunnel() {
+ for (int i = 0; i < 3; i++) {
+ HttpEntity mdn = requestBodyAndHeaders("direct://HTTPS",
EDI_MESSAGE, as2Headers());
+
assertInstanceOf(DispositionNotificationMultipartReportEntity.class, mdn);
+ }
+
+ assertEquals(3, httpsRequestHandler.getCount());
+ // a connection kept alive by the pool is already tunnelled: it must
not be asked to CONNECT again
+
assertTrue(proxy.getRequestLines().stream().allMatch(httpsConnectLine()::equals),
proxy.getRequestLines()::toString);
+ assertTrue(proxy.getRequestLines().size() <= 3,
proxy.getRequestLines()::toString);
+ }
+
+ @Test
+ public void sendsConcurrentlyThroughTheTunnel() throws Exception {
+ int messages = 16;
+ ExecutorService executor = Executors.newFixedThreadPool(8);
+ try {
+ List<Future<Object>> mdns = new ArrayList<>();
+ for (int i = 0; i < messages; i++) {
+ mdns.add(executor.submit(() ->
requestBodyAndHeaders("direct://HTTPS", EDI_MESSAGE, as2Headers())));
+ }
+ for (Future<Object> mdn : mdns) {
+
assertInstanceOf(DispositionNotificationMultipartReportEntity.class,
mdn.get(30, TimeUnit.SECONDS));
+ }
+ } finally {
+ executor.shutdownNow();
+ }
+
+ assertEquals(messages, httpsRequestHandler.getCount());
+
assertTrue(proxy.getRequestLines().stream().allMatch(httpsConnectLine()::equals),
proxy.getRequestLines()::toString);
+ }
+
+ @Test
+ public void refusedTunnelFailsTheSend() {
Review Comment:
Added in d6423fea3816: `refusedTunnelIsNotReused` and
`refusedHttpConnectionIsNotReused`, each with a variant where `TestHttpProxy`
keeps the connection open after a 407 with a body. They check two 407s, two
requests and two connections at the proxy.
`verifiesTheTargetHostNameThroughTheTunnel` now sends twice and expects two
CONNECTs. All 5 fail without the fix.
_Claude Code on behalf of @seshuthota_
--
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]
