Skip to content

Commit b6c77b7

Browse files
matejkmatejk
committed
fix(Net): address review findings in HTTPS proxy support
- Fix missing _proxySessionFactory registration in (host, port, ProxyConfig) constructor causing UnknownURISchemeException at connection time - Fix HTTPSClientSession::proxyRequestPrefix() regression: return empty string in tunnel mode to avoid corrupting request URIs - Fix empty proxyAuthenticate() override silently dropping credentials in non-tunnel HTTPS proxy mode - Add protocol validation and connected-state check to setProxyConfig() and setGlobalProxyConfig() - Wrap destructor unregisterProtocol() in try/catch to prevent std::terminate - Include HTTP status code in proxyConnect() error message - Use InvalidArgumentException instead of IllegalStateException for protocol validation - Extract factory registration into initProxySessionFactory() helpers to prevent future missed-constructor bugs - Modernize ProxyAuthentication to enum class with None/Basic/Digest/NTLM - Use default member initializers in ProxyConfig (enables C++20 designated initializers) - Add [[nodiscard]] to key getters, modernize copy deletion to = delete - Add 10 new unit tests covering ProxyConfig defaults, protocol validation, setters, request prefix, non-tunnel mode, global config, and bypass behavior - Update HTTPTestServer to handle proxy requests without default port - Fix copyright year and doc-comment style
1 parent dbb4338 commit b6c77b7

File tree

10 files changed

+391
-56
lines changed

10 files changed

+391
-56
lines changed

‎Net/include/Poco/Net/HTTPClientSession.h‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class Net_API HTTPClientSession: public HTTPSession
183183
void setProxyConfig(const ProxyConfig& config);
184184
/// Sets the proxy configuration.
185185

186-
const ProxyConfig& getProxyConfig() const;
186+
[[nodiscard]] const ProxyConfig& getProxyConfig() const;
187187
/// Returns the proxy configuration.
188188

189189
static void setGlobalProxyConfig(const ProxyConfig& config);
@@ -196,7 +196,7 @@ class Net_API HTTPClientSession: public HTTPSession
196196
/// The global proxy configuration should be set at start up, before
197197
/// the first HTTPClientSession instance is created.
198198

199-
static const ProxyConfig& getGlobalProxyConfig();
199+
[[nodiscard]] static const ProxyConfig& getGlobalProxyConfig();
200200
/// Returns the global proxy configuration.
201201

202202
void setKeepAliveTimeout(const Poco::Timespan& timeout);
@@ -287,11 +287,11 @@ class Net_API HTTPClientSession: public HTTPSession
287287
/// the request or response stream changes into
288288
/// fail or bad state, but not eof state).
289289

290-
virtual bool secure() const;
290+
[[nodiscard]] virtual bool secure() const;
291291
/// Return true iff the session uses SSL or TLS,
292292
/// or false otherwise.
293293

294-
bool bypassProxy() const;
294+
[[nodiscard]] bool bypassProxy() const;
295295
/// Returns true if the proxy should be bypassed
296296
/// for the current host.
297297

@@ -373,8 +373,11 @@ class Net_API HTTPClientSession: public HTTPSession
373373

374374
static ProxyConfig _globalProxyConfig;
375375

376-
HTTPClientSession(const HTTPClientSession&);
377-
HTTPClientSession& operator = (const HTTPClientSession&);
376+
void initProxySessionFactory();
377+
/// Registers the "http" protocol with _proxySessionFactory.
378+
379+
HTTPClientSession(const HTTPClientSession&) = delete;
380+
HTTPClientSession& operator = (const HTTPClientSession&) = delete;
378381

379382
friend class WebSocket;
380383
};
@@ -413,7 +416,7 @@ inline const std::string& HTTPClientSession::getProxyProtocol() const
413416
}
414417

415418

416-
inline bool HTTPClientSession::isProxyTunnel() const
419+
[[nodiscard]] inline bool HTTPClientSession::isProxyTunnel() const
417420
{
418421
return _proxyConfig.tunnel;
419422
}

‎Net/include/Poco/Net/ProxyConfig.h‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//
88
// Definition of the ProxyConfig class.
99
//
10-
// Copyright (c) 2005-2026, Applied Informatics Software Engineering GmbH.
10+
// Copyright (c) 2026-2026, Applied Informatics Software Engineering GmbH.
1111
// and Contributors.
1212
//
1313
// SPDX-License-Identifier: BSL-1.0
@@ -23,35 +23,33 @@ namespace Poco {
2323
namespace Net {
2424

2525

26-
enum ProxyAuthentication
26+
enum class ProxyAuthentication
2727
{
28-
PROXY_AUTH_NONE, /// No proxy authentication
29-
PROXY_AUTH_HTTP_BASIC, /// HTTP Basic proxy authentication (default, if username and password are supplied)
30-
PROXY_AUTH_HTTP_DIGEST, /// HTTP Digest proxy authentication
31-
PROXY_AUTH_NTLM /// NTLMv2 proxy authentication
28+
None, /// No proxy authentication
29+
Basic, /// HTTP Basic proxy authentication (default, if username and password are supplied)
30+
Digest, /// HTTP Digest proxy authentication
31+
NTLM /// NTLMv2 proxy authentication
3232
};
3333

3434

3535
struct ProxyConfig
3636
/// HTTP proxy server configuration.
3737
{
38-
ProxyConfig():
39-
port(HTTPSession::HTTP_PORT),
40-
protocol("http"),
41-
tunnel(true),
42-
authMethod(PROXY_AUTH_HTTP_BASIC)
43-
{
44-
}
38+
ProxyConfig() = default;
4539

4640
std::string host;
4741
/// Proxy server host name or IP address.
48-
Poco::UInt16 port;
42+
Poco::UInt16 port = HTTPSession::HTTP_PORT;
4943
/// Proxy server TCP port.
50-
std::string protocol;
44+
std::string protocol = "http";
5145
/// Protocol to use (http or https).
52-
bool tunnel;
46+
bool tunnel = true;
5347
/// Use proxy as tunnel (establish 2-way communication through CONNECT request).
5448
/// If tunnel option is 'false' request will be sent directly to proxy without CONNECT request.
49+
///
50+
/// Warning: Setting tunnel to false for HTTPS sessions means the TLS connection
51+
/// terminates at the proxy, not at the destination server. The proxy will see
52+
/// the request in plaintext.
5553
std::string username;
5654
/// Proxy server username.
5755
std::string password;
@@ -61,7 +59,7 @@ struct ProxyConfig
6159
/// e.g. "localhost|127\.0\.0\.1|192\.168\.0\.\d+". Can also be an empty
6260
/// string to disable proxy bypassing.
6361

64-
ProxyAuthentication authMethod;
62+
ProxyAuthentication authMethod = ProxyAuthentication::Basic;
6563
/// The authentication method to use - HTTP Basic or NTLM.
6664
};
6765

‎Net/src/HTTPClientSession.cpp‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
using Poco::NumberFormatter;
3333
using Poco::IllegalStateException;
34+
using Poco::InvalidArgumentException;
3435

3536

3637
namespace Poco {
@@ -52,7 +53,7 @@ HTTPClientSession::HTTPClientSession():
5253
_responseReceived(false),
5354
_ntlmProxyAuthenticated(false)
5455
{
55-
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
56+
initProxySessionFactory();
5657
}
5758

5859

@@ -69,7 +70,7 @@ HTTPClientSession::HTTPClientSession(const StreamSocket& socket):
6970
_responseReceived(false),
7071
_ntlmProxyAuthenticated(false)
7172
{
72-
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
73+
initProxySessionFactory();
7374
}
7475

7576

@@ -86,7 +87,7 @@ HTTPClientSession::HTTPClientSession(const SocketAddress& address):
8687
_responseReceived(false),
8788
_ntlmProxyAuthenticated(false)
8889
{
89-
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
90+
initProxySessionFactory();
9091
}
9192

9293

@@ -103,13 +104,15 @@ HTTPClientSession::HTTPClientSession(const std::string& host, Poco::UInt16 port)
103104
_responseReceived(false),
104105
_ntlmProxyAuthenticated(false)
105106
{
106-
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
107+
initProxySessionFactory();
107108
}
108109

109110

110111
HTTPClientSession::HTTPClientSession(const std::string& host, Poco::UInt16 port, const ProxyConfig& proxyConfig):
111112
_host(host),
112113
_port(port),
114+
_sourceAddress4(IPAddress::wildcard(IPAddress::IPv4), 0),
115+
_sourceAddress6(IPAddress::wildcard(IPAddress::IPv6), 0),
113116
_proxyConfig(proxyConfig),
114117
_keepAliveTimeout(DEFAULT_KEEP_ALIVE_TIMEOUT, 0),
115118
_reconnect(false),
@@ -118,6 +121,7 @@ HTTPClientSession::HTTPClientSession(const std::string& host, Poco::UInt16 port,
118121
_responseReceived(false),
119122
_ntlmProxyAuthenticated(false)
120123
{
124+
initProxySessionFactory();
121125
}
122126

123127

@@ -131,15 +135,28 @@ HTTPClientSession::HTTPClientSession(const StreamSocket& socket, const ProxyConf
131135
_reconnect(false),
132136
_mustReconnect(false),
133137
_expectResponseBody(false),
134-
_responseReceived(false)
138+
_responseReceived(false),
139+
_ntlmProxyAuthenticated(false)
135140
{
136-
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
141+
initProxySessionFactory();
137142
}
138143

139144

140145
HTTPClientSession::~HTTPClientSession()
141146
{
142-
_proxySessionFactory.unregisterProtocol("http");
147+
try
148+
{
149+
_proxySessionFactory.unregisterProtocol("http");
150+
}
151+
catch (...)
152+
{
153+
}
154+
}
155+
156+
157+
void HTTPClientSession::initProxySessionFactory()
158+
{
159+
_proxySessionFactory.registerProtocol("http", new HTTPSessionInstantiator);
143160
}
144161

145162

@@ -197,7 +214,7 @@ const SocketAddress& HTTPClientSession::getSourceAddress6()
197214
void HTTPClientSession::setProxy(const std::string& host, Poco::UInt16 port, const std::string& protocol, bool tunnel)
198215
{
199216
if (protocol != "http" && protocol != "https")
200-
throw IllegalStateException("Protocol must be either http or https");
217+
throw InvalidArgumentException("Protocol must be either http or https");
201218

202219
if (!connected())
203220
{
@@ -231,7 +248,7 @@ void HTTPClientSession::setProxyPort(Poco::UInt16 port)
231248
void HTTPClientSession::setProxyProtocol(const std::string& protocol)
232249
{
233250
if (protocol != "http" && protocol != "https")
234-
throw IllegalStateException("Protocol must be either http or https");
251+
throw InvalidArgumentException("Protocol must be either http or https");
235252

236253
if (!connected())
237254
_proxyConfig.protocol = protocol;
@@ -270,12 +287,21 @@ void HTTPClientSession::setProxyPassword(const std::string& password)
270287

271288
void HTTPClientSession::setProxyConfig(const ProxyConfig& config)
272289
{
273-
_proxyConfig = config;
290+
if (config.protocol != "http" && config.protocol != "https")
291+
throw InvalidArgumentException("Protocol must be either http or https");
292+
293+
if (!connected())
294+
_proxyConfig = config;
295+
else
296+
throw IllegalStateException("Cannot set the proxy configuration for an already connected session");
274297
}
275298

276299

277300
void HTTPClientSession::setGlobalProxyConfig(const ProxyConfig& config)
278301
{
302+
if (config.protocol != "http" && config.protocol != "https")
303+
throw InvalidArgumentException("Protocol must be either http or https");
304+
279305
_globalProxyConfig = config;
280306
}
281307

@@ -509,8 +535,8 @@ std::string HTTPClientSession::proxyRequestPrefix() const
509535
{
510536
std::string result("http://");
511537
result.append(_host);
512-
/// Do not append default by default, since this may break some servers.
513-
/// One example of such server is GCS (Google Cloud Storage).
538+
// Do not append default port, since this may break some servers.
539+
// One example of such server is GCS (Google Cloud Storage).
514540
if (_port != HTTPSession::HTTP_PORT)
515541
{
516542
result.append(":");
@@ -541,16 +567,16 @@ void HTTPClientSession::proxyAuthenticateImpl(HTTPRequest& request, const ProxyC
541567
{
542568
switch (proxyConfig.authMethod)
543569
{
544-
case PROXY_AUTH_NONE:
570+
case ProxyAuthentication::None:
545571
break;
546572

547-
case PROXY_AUTH_HTTP_BASIC:
573+
case ProxyAuthentication::Basic:
548574
_proxyBasicCreds.setUsername(proxyConfig.username);
549575
_proxyBasicCreds.setPassword(proxyConfig.password);
550576
_proxyBasicCreds.proxyAuthenticate(request);
551577
break;
552578

553-
case PROXY_AUTH_HTTP_DIGEST:
579+
case ProxyAuthentication::Digest:
554580
if (HTTPCredentials::hasDigestCredentials(request))
555581
{
556582
_proxyDigestCreds.updateProxyAuthInfo(request);
@@ -563,7 +589,7 @@ void HTTPClientSession::proxyAuthenticateImpl(HTTPRequest& request, const ProxyC
563589
}
564590
break;
565591

566-
case PROXY_AUTH_NTLM:
592+
case ProxyAuthentication::NTLM:
567593
if (_ntlmProxyAuthenticated)
568594
{
569595
_proxyNTLMCreds.updateProxyAuthInfo(request);
@@ -643,7 +669,7 @@ StreamSocket HTTPClientSession::proxyConnect()
643669
proxySession->sendRequest(proxyRequest);
644670
proxySession->receiveResponse(proxyResponse);
645671
if (proxyResponse.getStatus() != HTTPResponse::HTTP_OK)
646-
throw HTTPException("Cannot establish proxy connection", proxyResponse.getReason());
672+
throw HTTPException("Cannot establish proxy connection (" + std::to_string(proxyResponse.getStatus()) + ")", proxyResponse.getReason());
647673
return proxySession->detachSocket();
648674
}
649675

0 commit comments

Comments
 (0)