Skip to content

Commit 3eaa776

Browse files
predic8predic8
authored
Proxy: Fix URI for CONNECT Requests (#2886)
* Refactor code for consistency and readability: - Removed unused constants and imports (`TRACK_NODE_STATUS`) from several classes. - Replaced explicit type declarations with `var` in tests and implementations for modernized code. - Improved test case clarity (`ProxySSLTest` and `HttpClientTest`) by refining method logic and assertions. - Minor code cleanup and enhanced null safety in `HttpClient` and connection handling. * Remove unused logger from `Exchange` and cleanup unused imports in `HttpClient`. * Refactor `HttpClient` and related classes for improved maintainability: - Extracted and encapsulated logic into `adjustHostHeader` method for cleaner host header adjustments. - Enhanced null safety and annotations using `@NotNull` for better code guarantees. - Replaced explicit type declarations with `var` for modernized and consistent code. - Simplified `tunneled` proxy handling in `Connection`. - Improved error message formatting and streamlined exception handling in `getHostColonPort`. - Minor comments and formatting adjustments for readability across test and core implementations. * Update license headers and fix method reference in `HttpClientTest` - Added missing Apache 2.0 license headers to `AbstractFlowInterceptorTest`. - Replaced direct call to `client.getHostColonPort` with static `HttpClient.getHostColonPort` in `HttpClientTest`. * comments, test * comments, test * Exception text
1 parent 227a2fb commit 3eaa776

12 files changed

Lines changed: 156 additions & 92 deletions

File tree

‎core/src/main/java/com/predic8/membrane/core/exchange/AbstractExchange.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,17 @@
1818
import com.predic8.membrane.core.http.*;
1919
import com.predic8.membrane.core.interceptor.*;
2020
import com.predic8.membrane.core.interceptor.Interceptor.*;
21-
import com.predic8.membrane.core.interceptor.balancer.ExchangeNodeStatusTracker;
21+
import com.predic8.membrane.core.interceptor.balancer.*;
2222
import com.predic8.membrane.core.model.*;
23-
import com.predic8.membrane.core.proxies.Proxy;
2423
import com.predic8.membrane.core.proxies.*;
24+
import com.predic8.membrane.core.proxies.Proxy;
2525
import org.slf4j.*;
2626

27-
import java.io.*;
2827
import java.net.*;
2928
import java.text.*;
3029
import java.util.*;
3130

32-
import static com.predic8.membrane.core.exchange.Exchange.TRACK_NODE_STATUS;
3331
import static com.predic8.membrane.core.exchange.ExchangeState.*;
34-
import static java.lang.Boolean.TRUE;
3532

3633
public abstract class AbstractExchange {
3734
private static final Logger log = LoggerFactory.getLogger(AbstractExchange.class.getName());

‎core/src/main/java/com/predic8/membrane/core/exchange/Exchange.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,12 @@ public class Exchange extends AbstractExchange {
3737
* compatibility (i.e. for Java's internal HTTP client).
3838
*/
3939
public static final String ALLOW_H2 = "membrane.use.h2";
40-
public static final String TRACK_NODE_STATUS = "membrane.track.node.status";
4140
public static final String SSL_CONTEXT = "membrane.ssl.context";
4241
public static final String OAUTH2 = "membrane.oauth2";
4342
public static final String SNI_SERVER_NAME = "membrane.sni.server.name";
4443
public static final String WS_ORIGINAL_EXCHANGE = "membrane.ws.original.exchange";
4544
public static final String SECURITY_SCHEMES = "membrane.security.schemes";
4645

47-
private static final Logger log = LoggerFactory.getLogger(Exchange.class.getName());
48-
4946
private AbstractHttpHandler handler;
5047

5148
private String originalHostHeader = "";

‎core/src/main/java/com/predic8/membrane/core/interceptor/balancer/ExchangeNodeStatusTracker.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414

1515
package com.predic8.membrane.core.interceptor.balancer;
1616

17-
import static com.predic8.membrane.core.exchange.Exchange.TRACK_NODE_STATUS;
18-
import static java.lang.Boolean.TRUE;
19-
2017
/**
2118
* Used by the {@link LoadBalancingInterceptor} to track the status of nodes during load balancing.
2219
*

‎core/src/main/java/com/predic8/membrane/core/transport/http/Connection.java‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ public class Connection implements Closeable, MessageObserver, NonRelevantBodyOb
7777
private Exchange exchange;
7878
private boolean keepAttachedToExchange;
7979

80+
/**
81+
* If the connection is tunneled through a proxy and the connection to the real backend uses TLS
82+
*/
83+
private boolean tunneled;
84+
8085
public static Connection open(String host, int port, String localHost, SSLProvider sslProvider, int connectTimeout) throws IOException {
8186
return open(host, port, localHost, sslProvider, null, connectTimeout);
8287
}
@@ -120,18 +125,18 @@ public static Connection open(String host, int port, String localHost, SSLProvid
120125

121126
log.debug("Opened connection on localPort: {}", con.socket.getLocalPort());
122127

123-
setupStreams(con);
128+
con.setupStreams();
124129
return con;
125130
}
126131

127-
private static void setupStreams(Connection con) throws IOException {
132+
private void setupStreams() throws IOException {
128133
if (ByteStreamLogging.isLoggingEnabled()) {
129134
String connectionName = chooseNewConnectionName();
130-
con.out = new BufferedOutputStream(wrapConnectionOutputStream(con.socket.getOutputStream(), connectionName + " out"), BUFFER_SIZE);
131-
con.in = new BufferedInputStream(wrapConnectionInputStream(con.socket.getInputStream(), connectionName + " in"), BUFFER_SIZE);
135+
out = new BufferedOutputStream(wrapConnectionOutputStream(socket.getOutputStream(), connectionName + " out"), BUFFER_SIZE);
136+
in = new BufferedInputStream(wrapConnectionInputStream(socket.getInputStream(), connectionName + " in"), BUFFER_SIZE);
132137
} else {
133-
con.out = new BufferedOutputStream(con.socket.getOutputStream(), BUFFER_SIZE);
134-
con.in = new BufferedInputStream(con.socket.getInputStream(), BUFFER_SIZE);
138+
out = new BufferedOutputStream(socket.getOutputStream(), BUFFER_SIZE);
139+
in = new BufferedInputStream(socket.getInputStream(), BUFFER_SIZE);
135140
}
136141
}
137142

@@ -417,15 +422,17 @@ private void doTunnelHandshake(ProxyConfiguration proxy, Socket tunnel, String h
417422
throw new UnableToTunnelException("Unable to tunnel through %s:%d. Proxy returns '%s'".formatted(proxy.getHost(), proxy.getPort(), replyStr));
418423
}
419424

420-
/* tunneling Handshake was successful! */
425+
/* tunneling Handshake was successful! */
426+
tunneled = true;
421427
}
422428

423429
private static @NotNull String getHostString(ProxyConfiguration proxy) {
424430
return proxy.getHost() + ((proxy.isAuthentication()) ? " authenticated" : "");
425431
}
426432

427433
private static byte @NotNull [] createConnectMessage(ProxyConfiguration proxy, String host, int port) {
428-
var msg = "CONNECT %s:%d HTTP/1.0\r\nUser-Agent: %s\r\n%s\r\n".formatted(host, port, USERAGENT, getProxyAuthenticationHeader(proxy));
434+
var msg = "CONNECT %s:%d HTTP/1.0\r\nUser-Agent: %s\r\n%s\r\n"
435+
.formatted(host, port, USERAGENT, getProxyAuthenticationHeader(proxy));
429436
byte[] b;
430437
try {
431438
/*
@@ -450,4 +457,11 @@ private void doTunnelHandshake(ProxyConfiguration proxy, Socket tunnel, String h
450457
public SSLProvider getSslProvider() {
451458
return sslProvider;
452459
}
460+
461+
/**
462+
* @return true If the connection is tunneled through a proxy and the connection to the real backend uses TLS
463+
*/
464+
public boolean isTunneled() {
465+
return tunneled;
466+
}
453467
}

‎core/src/main/java/com/predic8/membrane/core/transport/http/HostColonPort.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,21 @@
1313
limitations under the License. */
1414
package com.predic8.membrane.core.transport.http;
1515

16+
import org.jetbrains.annotations.*;
17+
1618
import java.net.*;
1719
import java.util.regex.*;
1820

1921
public record HostColonPort(boolean useSSL, String host, int port) {
2022

2123
private static final Pattern pattern = Pattern.compile("^(https?)://([^:/#]+)(?::(\\d+))?.*");
2224

25+
/**
26+
*
27+
* @param useSSL To compute default ports
28+
* @param host Hostname, e.g. api.predic8.de, 127.0.0.1, [3:34:55]
29+
* @param port port number
30+
*/
2331
public HostColonPort(boolean useSSL, String host, int port) {
2432
this.useSSL = useSSL;
2533
this.host = host;
@@ -30,6 +38,11 @@ public HostColonPort(boolean useSSL, String host, int port) {
3038
}
3139
}
3240

41+
/**
42+
*
43+
* @param useSSL To compute default ports
44+
* @param hostAndPort e.g. api.predic8.de:443
45+
*/
3346
public HostColonPort(boolean useSSL, String hostAndPort) {
3447
this(useSSL, hostPart(hostAndPort), portPart(hostAndPort, useSSL ? 443 : 80));
3548
}
@@ -52,17 +65,17 @@ public URI toURI() throws URISyntaxException {
5265
}
5366

5467
@Override
55-
public String toString() {
68+
public @NotNull String toString() {
5669
return host + ":" + port;
5770
}
5871

5972
private static String hostPart(String addr) {
60-
var colon = addr.indexOf(":");
73+
var colon = addr.lastIndexOf(":");
6174
return (colon > -1 ? addr.substring(0, colon) : addr);
6275
}
6376

6477
private static int portPart(String addr, int defaultValue) {
65-
var colon = addr.indexOf(":");
78+
var colon = addr.lastIndexOf(":");
6679
return (colon > -1 ? Integer.parseInt(addr.substring(colon + 1)) : defaultValue);
6780
}
6881

‎core/src/main/java/com/predic8/membrane/core/transport/http/HttpClient.java‎

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@
2121
import com.predic8.membrane.core.transport.http.client.protocol.*;
2222
import com.predic8.membrane.core.transport.http.streampump.*;
2323
import com.predic8.membrane.core.util.*;
24+
import org.jetbrains.annotations.*;
2425
import org.slf4j.*;
2526

26-
import javax.annotation.*;
27+
import javax.annotation.Nullable;
2728
import java.io.*;
2829
import java.net.*;
2930

30-
import static com.predic8.membrane.core.exchange.Exchange.*;
3131
import static com.predic8.membrane.core.transport.http.client.protocol.Http2ProtocolHandler.*;
3232
import static com.predic8.membrane.core.util.HttpUtil.*;
3333

@@ -67,16 +67,18 @@ public HttpClient(@Nullable HttpClientConfiguration clientConfiguration, @Nullab
6767
}
6868

6969
public void call(Exchange exc) throws Exception {
70-
ProtocolHandler ph = protocolHandlerFactory.getHandler(exc, exc.getRequest().getHeader().getUpgradeProtocol());
70+
var ph = protocolHandlerFactory.getHandler(exc, exc.getRequest().getHeader().getUpgradeProtocol());
7171
ph.checkUpgradeRequest(exc);
7272
configuration.getRetryHandler().executeWithRetries(exc, this::dispatchCall);
7373
ph.cleanup(exc);
7474
}
7575

7676
private boolean dispatchCall(Exchange exc, String target, int attempt) throws Exception {
77-
HostColonPort hcp = initializeRequest(exc, target);
78-
79-
OutgoingConnectionType outConType = connectionFactory.getConnection(exc, hcp, attempt);
77+
setAuthorizationHeader(exc);
78+
var hcp = getHostColonPort(exc, target);
79+
adjustHostHeader(exc, hcp);
80+
var outConType = connectionFactory.getConnection(exc, hcp, attempt);
81+
setRequestURI(exc.getRequest(), target, outConType.con());
8082

8183
if (configuration.getProxy() != null && outConType.sslProvider() == null) {
8284
// if we use a proxy for a plain HTTP (=non-HTTPS) request, attach the proxy credentials.
@@ -89,38 +91,49 @@ private boolean dispatchCall(Exchange exc, String target, int attempt) throws Ex
8991
exc.getNodeStatusTracker().setNodeStatusCode(attempt, exc.getResponse().getStatusCode());
9092

9193
// Check for protocol upgrades
92-
String upgradedProtocol = exc.getProperty(UPGRADED_PROTOCOL, String.class);
94+
var upgradedProtocol = exc.getProperty(UPGRADED_PROTOCOL, String.class);
9395
if (upgradedProtocol == null)
9496
return false;
9597

96-
log.debug("Upgrading to {}",upgradedProtocol);
98+
log.debug("Upgrading to {}", upgradedProtocol);
9799

98100
StreamPump.setupConnectionForwarding(exc, outConType.con(), upgradedProtocol, streamPumpStats);
99101
outConType.con().setExchange(exc);
100102
return true;
101103
}
102104

103-
HostColonPort initializeRequest(Exchange exc, String dest) throws IOException {
104-
setRequestURI(exc.getRequest(), dest);
105-
if (configuration.getAuthentication() != null)
106-
exc.getRequest().getHeader().setAuthorization(configuration.getAuthentication().getUsername(), configuration.getAuthentication().getPassword());
107-
108-
HostColonPort target = getTargetHostAndPort(exc.getRequest().isCONNECTRequest(), dest);
105+
protected void adjustHostHeader(Exchange exc, HostColonPort target) {
109106
if (configuration.isAdjustHostHeader() && (exc.getProxy() == null || exc.getProxy().isTargetAdjustHostHeader())) {
110107
exc.getRequest().getHeader().setHost(target.toString());
111108
}
112-
return target;
113109
}
114110

111+
private void setAuthorizationHeader(Exchange exc) {
112+
if (configuration.getAuthentication() != null)
113+
exc.getRequest().getHeader().setAuthorization(configuration.getAuthentication().getUsername(), configuration.getAuthentication().getPassword());
114+
}
115115

116-
public void setStreamPumpStats(StreamPump.StreamPumpStats streamPumpStats) {
117-
this.streamPumpStats = streamPumpStats;
116+
/**
117+
* @param exc Exchange
118+
* @param dest URL for normal requests and host:port for CONNECT requests
119+
* @return HostColonPort
120+
*/
121+
protected @NotNull HostColonPort getHostColonPort(Exchange exc, String dest) throws MalformedURLException {
122+
if (exc.getRequest().isCONNECTRequest())
123+
return new HostColonPort(false, dest);
124+
125+
try {
126+
return HostColonPort.parse(dest);
127+
} catch (MalformedURLException e) {
128+
throw new MalformedURLException("""
129+
The exchange's destination URI %s is not valid. Specify a 'target' within
130+
the API configuration or make sure the exchanges destinations list contains a valid URI.
131+
""".formatted(dest));
132+
}
118133
}
119134

120-
private static boolean trackNodeStatus(Exchange exc) {
121-
if (exc.getProperty(TRACK_NODE_STATUS) instanceof Boolean status)
122-
return status;
123-
return false;
135+
public void setStreamPumpStats(StreamPump.StreamPumpStats streamPumpStats) {
136+
this.streamPumpStats = streamPumpStats;
124137
}
125138

126139
@Override
@@ -134,30 +147,31 @@ public ConnectionFactory getConnectionFactory() {
134147
return connectionFactory;
135148
}
136149

137-
void setRequestURI(Request req, String dest) throws MalformedURLException {
150+
void setRequestURI(Request req, String dest, @NotNull Connection con) throws MalformedURLException {
151+
// Only on none TLS connections
152+
if (req.isCONNECTRequest()) {
153+
req.setUri(getHostColonPort(dest));
154+
return;
155+
}
156+
138157
// Use complete URL with protocol and host. The proxy needs to know where to forward
139-
if (configuration.getProxy() != null || req.isCONNECTRequest()) {
158+
if (configuration.getProxy() != null && !con.isTunneled()) {
140159
req.setUri(dest);
141160
return;
142161
}
143162
if (!dest.startsWith("http")) {
144163
throw new MalformedURLException("""
145-
The exchange's destination URI %s does not start with 'http'. Specify a <target> within the API configuration or make sure the exchanges destinations list contains a valid URI.
164+
The exchange's destination URI %s does not start with 'http'. Specify a 'target' within
165+
the API configuration or make sure the exchanges destinations list contains a valid URI.
146166
""".formatted(dest));
147167
}
148168
req.setUri(getPathAndQueryString(dest));
149169
}
150170

151-
/**
152-
* @param connect If true, do not use TLS even when the URL starts with https
153-
* @param dest URL
154-
* @return HostColonPort
155-
* @throws MalformedURLException
156-
*/
157-
private HostColonPort getTargetHostAndPort(boolean connect, String dest) throws MalformedURLException {
158-
if (connect)
159-
return new HostColonPort(false, dest);
160-
return HostColonPort.parse(dest);
171+
protected static @NotNull String getHostColonPort(String dest) throws MalformedURLException {
172+
return dest.startsWith("http")
173+
? HostColonPort.parse(dest).toString()
174+
: dest;
161175
}
162176

163177
// TODO Rewrite all clients to use try with resources and then remove it

‎core/src/main/java/com/predic8/membrane/core/transport/http/client/RetryHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void executeWithRetries(Exchange exc, RetryableCall call) throws Exceptio
8585
Exception exceptionInLastCall = null;
8686
double currentDelay = delay;
8787
for (int attempt = 0; attempt <= retries; attempt++) {
88-
String dest = getDestination(exc, attempt);
88+
var dest = getDestination(exc, attempt);
8989
log.debug("Attempt #{} from #{} to {}", attempt, retries + 1, dest);
9090
try {
9191
if (call.execute(exc, dest, attempt)) {

‎core/src/test/java/com/predic8/membrane/core/interceptor/flow/AbstractFlowInterceptorTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
/* Copyright 2026 predic8 GmbH, www.predic8.com
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License. */
14+
115
package com.predic8.membrane.core.interceptor.flow;
216

317
import com.predic8.membrane.core.interceptor.log.*;

0 commit comments

Comments
 (0)