Skip to content

Commit bae40fc

Browse files
yhiroseyhirose
committed
Resolve #2237 
1 parent db561f5 commit bae40fc

File tree

2 files changed

+143
-33
lines changed

2 files changed

+143
-33
lines changed

‎httplib.h‎

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,9 @@ class RegexMatcher final : public MatcherBase {
10521052

10531053
ssize_t write_headers(Stream &strm, const Headers &headers);
10541054

1055+
std::string make_host_and_port_string(const std::string &host, int port,
1056+
bool is_ssl);
1057+
10551058
} // namespace detail
10561059

10571060
class Server {
@@ -1719,8 +1722,6 @@ class ClientImpl {
17191722
const std::string &boundary, const UploadFormDataItems &items,
17201723
const FormDataProviderItems &provider_items) const;
17211724

1722-
std::string adjust_host_string(const std::string &host) const;
1723-
17241725
virtual bool
17251726
process_socket(const Socket &socket,
17261727
std::chrono::time_point<std::chrono::steady_clock> start_time,
@@ -7261,6 +7262,30 @@ inline bool RegexMatcher::match(Request &request) const {
72617262
return std::regex_match(request.path, request.matches, regex_);
72627263
}
72637264

7265+
inline std::string make_host_and_port_string(const std::string &host, int port,
7266+
bool is_ssl) {
7267+
std::string result;
7268+
7269+
// Enclose IPv6 address in brackets (but not if already enclosed)
7270+
if (host.find(':') == std::string::npos ||
7271+
(!host.empty() && host[0] == '[')) {
7272+
// IPv4, hostname, or already bracketed IPv6
7273+
result = host;
7274+
} else {
7275+
// IPv6 address without brackets
7276+
result = "[" + host + "]";
7277+
}
7278+
7279+
// Append port if not default
7280+
if ((!is_ssl && port == 80) || (is_ssl && port == 443)) {
7281+
; // do nothing
7282+
} else {
7283+
result += ":" + std::to_string(port);
7284+
}
7285+
7286+
return result;
7287+
}
7288+
72647289
} // namespace detail
72657290

72667291
// HTTP server implementation
@@ -8522,7 +8547,7 @@ inline ClientImpl::ClientImpl(const std::string &host, int port,
85228547
const std::string &client_cert_path,
85238548
const std::string &client_key_path)
85248549
: host_(detail::escape_abstract_namespace_unix_domain(host)), port_(port),
8525-
host_and_port_(adjust_host_string(host_) + ":" + std::to_string(port)),
8550+
host_and_port_(detail::make_host_and_port_string(host_, port, is_ssl())),
85268551
client_cert_path_(client_cert_path), client_key_path_(client_key_path) {}
85278552

85288553
inline ClientImpl::~ClientImpl() {
@@ -8703,8 +8728,9 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) {
87038728
{
87048729
std::lock_guard<std::mutex> guard(socket_mutex_);
87058730

8706-
// Set this to false immediately - if it ever gets set to true by the end of
8707-
// the request, we know another thread instructed us to close the socket.
8731+
// Set this to false immediately - if it ever gets set to true by the end
8732+
// of the request, we know another thread instructed us to close the
8733+
// socket.
87088734
socket_should_be_closed_when_request_is_done_ = false;
87098735

87108736
auto is_alive = false;
@@ -8720,10 +8746,10 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) {
87208746
#endif
87218747

87228748
if (!is_alive) {
8723-
// Attempt to avoid sigpipe by shutting down non-gracefully if it seems
8724-
// like the other side has already closed the connection Also, there
8725-
// cannot be any requests in flight from other threads since we locked
8726-
// request_mutex_, so safe to close everything immediately
8749+
// Attempt to avoid sigpipe by shutting down non-gracefully if it
8750+
// seems like the other side has already closed the connection Also,
8751+
// there cannot be any requests in flight from other threads since we
8752+
// locked request_mutex_, so safe to close everything immediately
87278753
const bool shutdown_gracefully = false;
87288754
shutdown_ssl(socket_, shutdown_gracefully);
87298755
shutdown_socket(socket_);
@@ -9027,7 +9053,8 @@ inline bool ClientImpl::create_redirect_client(
90279053
}
90289054
}
90299055

9030-
// New method for robust client setup (based on basic_manual_redirect.cpp logic)
9056+
// New method for robust client setup (based on basic_manual_redirect.cpp
9057+
// logic)
90319058
template <typename ClientType>
90329059
inline void ClientImpl::setup_redirect_client(ClientType &client) {
90339060
// Copy basic settings first
@@ -9131,18 +9158,8 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req,
91319158
// curl behavior)
91329159
if (address_family_ == AF_UNIX) {
91339160
req.set_header("Host", "localhost");
9134-
} else if (is_ssl()) {
9135-
if (port_ == 443) {
9136-
req.set_header("Host", host_);
9137-
} else {
9138-
req.set_header("Host", host_and_port_);
9139-
}
91409161
} else {
9141-
if (port_ == 80) {
9142-
req.set_header("Host", host_);
9143-
} else {
9144-
req.set_header("Host", host_and_port_);
9145-
}
9162+
req.set_header("Host", host_and_port_);
91469163
}
91479164
}
91489165

@@ -9409,12 +9426,6 @@ inline Result ClientImpl::send_with_content_provider(
94099426
#endif
94109427
}
94119428

9412-
inline std::string
9413-
ClientImpl::adjust_host_string(const std::string &host) const {
9414-
if (host.find(':') != std::string::npos) { return "[" + host + "]"; }
9415-
return host;
9416-
}
9417-
94189429
inline void ClientImpl::output_log(const Request &req,
94199430
const Response &res) const {
94209431
if (logger_) {
@@ -9538,8 +9549,8 @@ inline ContentProviderWithoutLength ClientImpl::get_multipart_content_provider(
95389549
const FormDataProviderItems &provider_items) const {
95399550
size_t cur_item = 0;
95409551
size_t cur_start = 0;
9541-
// cur_item and cur_start are copied to within the std::function and maintain
9542-
// state between successive calls
9552+
// cur_item and cur_start are copied to within the std::function and
9553+
// maintain state between successive calls
95439554
return [&, cur_item, cur_start](size_t offset,
95449555
DataSink &sink) mutable -> bool {
95459556
if (!offset && !items.empty()) {
@@ -10251,8 +10262,8 @@ inline void ClientImpl::stop() {
1025110262
// If there is anything ongoing right now, the ONLY thread-safe thing we can
1025210263
// do is to shutdown_socket, so that threads using this socket suddenly
1025310264
// discover they can't read/write any more and error out. Everything else
10254-
// (closing the socket, shutting ssl down) is unsafe because these actions are
10255-
// not thread-safe.
10265+
// (closing the socket, shutting ssl down) is unsafe because these actions
10266+
// are not thread-safe.
1025610267
if (socket_requests_in_flight_ > 0) {
1025710268
shutdown_socket(socket_);
1025810269

@@ -10889,7 +10900,8 @@ inline void SSLClient::set_ca_cert_store(X509_STORE *ca_cert_store) {
1088910900
if (ca_cert_store) {
1089010901
if (ctx_) {
1089110902
if (SSL_CTX_get_cert_store(ctx_) != ca_cert_store) {
10892-
// Free memory allocated for old cert and use new store `ca_cert_store`
10903+
// Free memory allocated for old cert and use new store
10904+
// `ca_cert_store`
1089310905
SSL_CTX_set_cert_store(ctx_, ca_cert_store);
1089410906
ca_cert_store_ = ca_cert_store;
1089510907
}
@@ -10914,7 +10926,8 @@ inline bool SSLClient::create_and_connect_socket(Socket &socket, Error &error) {
1091410926
return is_valid() && ClientImpl::create_and_connect_socket(socket, error);
1091510927
}
1091610928

10917-
// Assumes that socket_mutex_ is locked and that there are no requests in flight
10929+
// Assumes that socket_mutex_ is locked and that there are no requests in
10930+
// flight
1091810931
inline bool SSLClient::connect_with_proxy(
1091910932
Socket &socket,
1092010933
std::chrono::time_point<std::chrono::steady_clock> start_time,

‎test/test.cc‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10319,6 +10319,103 @@ TEST(FileSystemTest, FileAndDirExistenceCheck) {
1031910319
EXPECT_TRUE(stat_dir.is_dir());
1032010320
}
1032110321

10322+
TEST(MakeHostAndPortStringTest, VariousPatterns) {
10323+
// IPv4 with default HTTP port (80)
10324+
EXPECT_EQ("example.com",
10325+
detail::make_host_and_port_string("example.com", 80, false));
10326+
10327+
// IPv4 with default HTTPS port (443)
10328+
EXPECT_EQ("example.com",
10329+
detail::make_host_and_port_string("example.com", 443, true));
10330+
10331+
// IPv4 with non-default HTTP port
10332+
EXPECT_EQ("example.com:8080",
10333+
detail::make_host_and_port_string("example.com", 8080, false));
10334+
10335+
// IPv4 with non-default HTTPS port
10336+
EXPECT_EQ("example.com:8443",
10337+
detail::make_host_and_port_string("example.com", 8443, true));
10338+
10339+
// IPv6 with default HTTP port (80)
10340+
EXPECT_EQ("[::1]", detail::make_host_and_port_string("::1", 80, false));
10341+
10342+
// IPv6 with default HTTPS port (443)
10343+
EXPECT_EQ("[::1]", detail::make_host_and_port_string("::1", 443, true));
10344+
10345+
// IPv6 with non-default HTTP port
10346+
EXPECT_EQ("[::1]:8080",
10347+
detail::make_host_and_port_string("::1", 8080, false));
10348+
10349+
// IPv6 with non-default HTTPS port
10350+
EXPECT_EQ("[::1]:8443", detail::make_host_and_port_string("::1", 8443, true));
10351+
10352+
// IPv6 full address with default port
10353+
EXPECT_EQ("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]",
10354+
detail::make_host_and_port_string(
10355+
"2001:0db8:85a3:0000:0000:8a2e:0370:7334", 443, true));
10356+
10357+
// IPv6 full address with non-default port
10358+
EXPECT_EQ("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:9000",
10359+
detail::make_host_and_port_string(
10360+
"2001:0db8:85a3:0000:0000:8a2e:0370:7334", 9000, false));
10361+
10362+
// IPv6 localhost with non-default port
10363+
EXPECT_EQ("[::1]:3000",
10364+
detail::make_host_and_port_string("::1", 3000, false));
10365+
10366+
// IPv6 with zone ID (link-local address) with default port
10367+
EXPECT_EQ("[fe80::1%eth0]",
10368+
detail::make_host_and_port_string("fe80::1%eth0", 80, false));
10369+
10370+
// IPv6 with zone ID (link-local address) with non-default port
10371+
EXPECT_EQ("[fe80::1%eth0]:8080",
10372+
detail::make_host_and_port_string("fe80::1%eth0", 8080, false));
10373+
10374+
// Edge case: Port 443 with is_ssl=false (should add port)
10375+
EXPECT_EQ("example.com:443",
10376+
detail::make_host_and_port_string("example.com", 443, false));
10377+
10378+
// Edge case: Port 80 with is_ssl=true (should add port)
10379+
EXPECT_EQ("example.com:80",
10380+
detail::make_host_and_port_string("example.com", 80, true));
10381+
10382+
// IPv6 edge case: Port 443 with is_ssl=false (should add port)
10383+
EXPECT_EQ("[::1]:443", detail::make_host_and_port_string("::1", 443, false));
10384+
10385+
// IPv6 edge case: Port 80 with is_ssl=true (should add port)
10386+
EXPECT_EQ("[::1]:80", detail::make_host_and_port_string("::1", 80, true));
10387+
10388+
// Security fix: Already bracketed IPv6 should not get double brackets
10389+
EXPECT_EQ("[::1]", detail::make_host_and_port_string("[::1]", 80, false));
10390+
EXPECT_EQ("[::1]", detail::make_host_and_port_string("[::1]", 443, true));
10391+
EXPECT_EQ("[::1]:8080",
10392+
detail::make_host_and_port_string("[::1]", 8080, false));
10393+
EXPECT_EQ("[2001:db8::1]:8080",
10394+
detail::make_host_and_port_string("[2001:db8::1]", 8080, false));
10395+
EXPECT_EQ("[fe80::1%eth0]",
10396+
detail::make_host_and_port_string("[fe80::1%eth0]", 80, false));
10397+
EXPECT_EQ("[fe80::1%eth0]:8080",
10398+
detail::make_host_and_port_string("[fe80::1%eth0]", 8080, false));
10399+
10400+
// Edge case: Empty host (should return as-is)
10401+
EXPECT_EQ("", detail::make_host_and_port_string("", 80, false));
10402+
10403+
// Edge case: Colon in hostname (non-IPv6) - will be treated as IPv6
10404+
// This is a known limitation but shouldn't crash
10405+
EXPECT_EQ("[host:name]",
10406+
detail::make_host_and_port_string("host:name", 80, false));
10407+
10408+
// Port number edge cases (no validation, but should not crash)
10409+
EXPECT_EQ("example.com:0",
10410+
detail::make_host_and_port_string("example.com", 0, false));
10411+
EXPECT_EQ("example.com:-1",
10412+
detail::make_host_and_port_string("example.com", -1, false));
10413+
EXPECT_EQ("example.com:65535",
10414+
detail::make_host_and_port_string("example.com", 65535, false));
10415+
EXPECT_EQ("example.com:65536",
10416+
detail::make_host_and_port_string("example.com", 65536, false));
10417+
}
10418+
1032210419
TEST(DirtyDataRequestTest, HeadFieldValueContains_CR_LF_NUL) {
1032310420
Server svr;
1032410421

0 commit comments

Comments
 (0)