davsclaus commented on code in PR #28265:
URL: https://github.com/apache/camel/pull/28265#discussion_r4357363890
##########
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:
Non-blocking: by the time this check runs, the `finally` block has already
released the connection to the pool with the strategy's keep-alive. If the
proxy keeps the connection open after the 407, its body may still be unread
when the next request uses that connection. Doing this check inside the `try`
and closing the endpoint before throwing (or releasing with a `null` keep-alive
for a proxy 407) avoids reusing it.
##########
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:
**Blocking**: if `createTunnel` or `upgrade` throws, this connection goes
back to the pool and still counts as connected. `release(...,
ZERO_MILLISECONDS)` in the `finally` block keeps an open connection with no
expiry, and `isConnected()` only checks that a socket is bound. The next
`send()` then skips `CONNECT` and TLS, and writes the AS2 request in plain text
on the proxy connection. See finding 1 in the review body.
Closing the connection on any failure here keeps the pool clean. This also
needs `import org.apache.hc.core5.io.CloseMode;`:
```suggestion
if (!endpoint.isConnected()) {
try {
connectionPoolManager.connect(endpoint,
TimeValue.ofMilliseconds(connectionTimeoutMilliseconds),
httpContext);
if (route.isTunnelled()) {
createTunnel(endpoint);
connectionPoolManager.upgrade(endpoint, httpContext);
}
} catch (IOException | HttpException | RuntimeException e) {
// a connection without its tunnel or its TLS layer must
not go back to the pool
endpoint.close(CloseMode.IMMEDIATE);
throw e;
}
}
```
##########
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:
Could we add a test that sends twice after a refusal? Today the second send
leases the pooled connection, skips `CONNECT`, and fails without a 407, so this
test fails without the fix in `send()` and passes with it:
```java
@Test
public void refusedTunnelIsNotReused() {
for (int i = 0; i < 2; i++) {
CamelExecutionException e =
assertThrows(CamelExecutionException.class,
() ->
requestBodyAndHeaders("direct://HTTPS_WRONG_PASSWORD", EDI_MESSAGE,
as2Headers()));
assertTrue(rootMessage(e).contains("407"), rootMessage(e));
}
// each send must ask for a new tunnel
assertEquals(List.of(httpsConnectLine(), httpsConnectLine()),
proxy.getRequestLines());
assertEquals(0, httpsRequestHandler.getCount());
}
```
`TestHttpProxy` always answers 407 with `Connection: close`. A variant that
keeps the client connection open after the 407 would cover proxies that keep
the connection alive for the auth handshake.
--
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]
