diff --git a/httplib.h b/httplib.h index db55d07e25..32bd094387 100644 --- a/httplib.h +++ b/httplib.h @@ -1052,6 +1052,9 @@ class RegexMatcher final : public MatcherBase { ssize_t write_headers(Stream &strm, const Headers &headers); +std::string make_host_and_port_string(const std::string &host, int port, + bool is_ssl); + } // namespace detail class Server { @@ -1719,8 +1722,6 @@ class ClientImpl { const std::string &boundary, const UploadFormDataItems &items, const FormDataProviderItems &provider_items) const; - std::string adjust_host_string(const std::string &host) const; - virtual bool process_socket(const Socket &socket, std::chrono::time_point start_time, @@ -7261,6 +7262,30 @@ inline bool RegexMatcher::match(Request &request) const { return std::regex_match(request.path, request.matches, regex_); } +inline std::string make_host_and_port_string(const std::string &host, int port, + bool is_ssl) { + std::string result; + + // Enclose IPv6 address in brackets (but not if already enclosed) + if (host.find(':') == std::string::npos || + (!host.empty() && host[0] == '[')) { + // IPv4, hostname, or already bracketed IPv6 + result = host; + } else { + // IPv6 address without brackets + result = "[" + host + "]"; + } + + // Append port if not default + if ((!is_ssl && port == 80) || (is_ssl && port == 443)) { + ; // do nothing + } else { + result += ":" + std::to_string(port); + } + + return result; +} + } // namespace detail // HTTP server implementation @@ -8522,7 +8547,7 @@ inline ClientImpl::ClientImpl(const std::string &host, int port, const std::string &client_cert_path, const std::string &client_key_path) : host_(detail::escape_abstract_namespace_unix_domain(host)), port_(port), - host_and_port_(adjust_host_string(host_) + ":" + std::to_string(port)), + host_and_port_(detail::make_host_and_port_string(host_, port, is_ssl())), client_cert_path_(client_cert_path), client_key_path_(client_key_path) {} inline ClientImpl::~ClientImpl() { @@ -8703,8 +8728,9 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) { { std::lock_guard guard(socket_mutex_); - // Set this to false immediately - if it ever gets set to true by the end of - // the request, we know another thread instructed us to close the socket. + // Set this to false immediately - if it ever gets set to true by the end + // of the request, we know another thread instructed us to close the + // socket. socket_should_be_closed_when_request_is_done_ = false; auto is_alive = false; @@ -8720,10 +8746,10 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) { #endif if (!is_alive) { - // Attempt to avoid sigpipe by shutting down non-gracefully if it seems - // like the other side has already closed the connection Also, there - // cannot be any requests in flight from other threads since we locked - // request_mutex_, so safe to close everything immediately + // Attempt to avoid sigpipe by shutting down non-gracefully if it + // seems like the other side has already closed the connection Also, + // there cannot be any requests in flight from other threads since we + // locked request_mutex_, so safe to close everything immediately const bool shutdown_gracefully = false; shutdown_ssl(socket_, shutdown_gracefully); shutdown_socket(socket_); @@ -9027,7 +9053,8 @@ inline bool ClientImpl::create_redirect_client( } } -// New method for robust client setup (based on basic_manual_redirect.cpp logic) +// New method for robust client setup (based on basic_manual_redirect.cpp +// logic) template inline void ClientImpl::setup_redirect_client(ClientType &client) { // Copy basic settings first @@ -9131,18 +9158,8 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req, // curl behavior) if (address_family_ == AF_UNIX) { req.set_header("Host", "localhost"); - } else if (is_ssl()) { - if (port_ == 443) { - req.set_header("Host", host_); - } else { - req.set_header("Host", host_and_port_); - } } else { - if (port_ == 80) { - req.set_header("Host", host_); - } else { - req.set_header("Host", host_and_port_); - } + req.set_header("Host", host_and_port_); } } @@ -9409,12 +9426,6 @@ inline Result ClientImpl::send_with_content_provider( #endif } -inline std::string -ClientImpl::adjust_host_string(const std::string &host) const { - if (host.find(':') != std::string::npos) { return "[" + host + "]"; } - return host; -} - inline void ClientImpl::output_log(const Request &req, const Response &res) const { if (logger_) { @@ -9538,8 +9549,8 @@ inline ContentProviderWithoutLength ClientImpl::get_multipart_content_provider( const FormDataProviderItems &provider_items) const { size_t cur_item = 0; size_t cur_start = 0; - // cur_item and cur_start are copied to within the std::function and maintain - // state between successive calls + // cur_item and cur_start are copied to within the std::function and + // maintain state between successive calls return [&, cur_item, cur_start](size_t offset, DataSink &sink) mutable -> bool { if (!offset && !items.empty()) { @@ -10251,8 +10262,8 @@ inline void ClientImpl::stop() { // If there is anything ongoing right now, the ONLY thread-safe thing we can // do is to shutdown_socket, so that threads using this socket suddenly // discover they can't read/write any more and error out. Everything else - // (closing the socket, shutting ssl down) is unsafe because these actions are - // not thread-safe. + // (closing the socket, shutting ssl down) is unsafe because these actions + // are not thread-safe. if (socket_requests_in_flight_ > 0) { shutdown_socket(socket_); @@ -10889,7 +10900,8 @@ inline void SSLClient::set_ca_cert_store(X509_STORE *ca_cert_store) { if (ca_cert_store) { if (ctx_) { if (SSL_CTX_get_cert_store(ctx_) != ca_cert_store) { - // Free memory allocated for old cert and use new store `ca_cert_store` + // Free memory allocated for old cert and use new store + // `ca_cert_store` SSL_CTX_set_cert_store(ctx_, ca_cert_store); ca_cert_store_ = ca_cert_store; } @@ -10911,10 +10923,15 @@ inline long SSLClient::get_openssl_verify_result() const { inline SSL_CTX *SSLClient::ssl_context() const { return ctx_; } inline bool SSLClient::create_and_connect_socket(Socket &socket, Error &error) { - return is_valid() && ClientImpl::create_and_connect_socket(socket, error); + if (!is_valid()) { + error = Error::SSLConnection; + return false; + } + return ClientImpl::create_and_connect_socket(socket, error); } -// Assumes that socket_mutex_ is locked and that there are no requests in flight +// Assumes that socket_mutex_ is locked and that there are no requests in +// flight inline bool SSLClient::connect_with_proxy( Socket &socket, std::chrono::time_point start_time, diff --git a/test/test.cc b/test/test.cc index a9ac0d17f1..3779ca29f6 100644 --- a/test/test.cc +++ b/test/test.cc @@ -8366,6 +8366,19 @@ TEST(SSLClientTest, Issue2004_Online) { EXPECT_EQ(body.substr(0, 15), ""); } +TEST(SSLClientTest, ErrorReportingWhenInvalid) { + // Create SSLClient with invalid cert/key to make is_valid() return false + SSLClient cli("localhost", 8080, "nonexistent_cert.pem", + "nonexistent_key.pem"); + + // is_valid() should be false due to cert loading failure + ASSERT_FALSE(cli.is_valid()); + + auto res = cli.Get("/"); + ASSERT_FALSE(res); + EXPECT_EQ(Error::SSLConnection, res.error()); +} + #if 0 TEST(SSLClientTest, SetInterfaceWithINET6) { auto cli = std::make_shared("https://httpbin.org"); @@ -10319,6 +10332,103 @@ TEST(FileSystemTest, FileAndDirExistenceCheck) { EXPECT_TRUE(stat_dir.is_dir()); } +TEST(MakeHostAndPortStringTest, VariousPatterns) { + // IPv4 with default HTTP port (80) + EXPECT_EQ("example.com", + detail::make_host_and_port_string("example.com", 80, false)); + + // IPv4 with default HTTPS port (443) + EXPECT_EQ("example.com", + detail::make_host_and_port_string("example.com", 443, true)); + + // IPv4 with non-default HTTP port + EXPECT_EQ("example.com:8080", + detail::make_host_and_port_string("example.com", 8080, false)); + + // IPv4 with non-default HTTPS port + EXPECT_EQ("example.com:8443", + detail::make_host_and_port_string("example.com", 8443, true)); + + // IPv6 with default HTTP port (80) + EXPECT_EQ("[::1]", detail::make_host_and_port_string("::1", 80, false)); + + // IPv6 with default HTTPS port (443) + EXPECT_EQ("[::1]", detail::make_host_and_port_string("::1", 443, true)); + + // IPv6 with non-default HTTP port + EXPECT_EQ("[::1]:8080", + detail::make_host_and_port_string("::1", 8080, false)); + + // IPv6 with non-default HTTPS port + EXPECT_EQ("[::1]:8443", detail::make_host_and_port_string("::1", 8443, true)); + + // IPv6 full address with default port + EXPECT_EQ("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", + detail::make_host_and_port_string( + "2001:0db8:85a3:0000:0000:8a2e:0370:7334", 443, true)); + + // IPv6 full address with non-default port + EXPECT_EQ("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:9000", + detail::make_host_and_port_string( + "2001:0db8:85a3:0000:0000:8a2e:0370:7334", 9000, false)); + + // IPv6 localhost with non-default port + EXPECT_EQ("[::1]:3000", + detail::make_host_and_port_string("::1", 3000, false)); + + // IPv6 with zone ID (link-local address) with default port + EXPECT_EQ("[fe80::1%eth0]", + detail::make_host_and_port_string("fe80::1%eth0", 80, false)); + + // IPv6 with zone ID (link-local address) with non-default port + EXPECT_EQ("[fe80::1%eth0]:8080", + detail::make_host_and_port_string("fe80::1%eth0", 8080, false)); + + // Edge case: Port 443 with is_ssl=false (should add port) + EXPECT_EQ("example.com:443", + detail::make_host_and_port_string("example.com", 443, false)); + + // Edge case: Port 80 with is_ssl=true (should add port) + EXPECT_EQ("example.com:80", + detail::make_host_and_port_string("example.com", 80, true)); + + // IPv6 edge case: Port 443 with is_ssl=false (should add port) + EXPECT_EQ("[::1]:443", detail::make_host_and_port_string("::1", 443, false)); + + // IPv6 edge case: Port 80 with is_ssl=true (should add port) + EXPECT_EQ("[::1]:80", detail::make_host_and_port_string("::1", 80, true)); + + // Security fix: Already bracketed IPv6 should not get double brackets + EXPECT_EQ("[::1]", detail::make_host_and_port_string("[::1]", 80, false)); + EXPECT_EQ("[::1]", detail::make_host_and_port_string("[::1]", 443, true)); + EXPECT_EQ("[::1]:8080", + detail::make_host_and_port_string("[::1]", 8080, false)); + EXPECT_EQ("[2001:db8::1]:8080", + detail::make_host_and_port_string("[2001:db8::1]", 8080, false)); + EXPECT_EQ("[fe80::1%eth0]", + detail::make_host_and_port_string("[fe80::1%eth0]", 80, false)); + EXPECT_EQ("[fe80::1%eth0]:8080", + detail::make_host_and_port_string("[fe80::1%eth0]", 8080, false)); + + // Edge case: Empty host (should return as-is) + EXPECT_EQ("", detail::make_host_and_port_string("", 80, false)); + + // Edge case: Colon in hostname (non-IPv6) - will be treated as IPv6 + // This is a known limitation but shouldn't crash + EXPECT_EQ("[host:name]", + detail::make_host_and_port_string("host:name", 80, false)); + + // Port number edge cases (no validation, but should not crash) + EXPECT_EQ("example.com:0", + detail::make_host_and_port_string("example.com", 0, false)); + EXPECT_EQ("example.com:-1", + detail::make_host_and_port_string("example.com", -1, false)); + EXPECT_EQ("example.com:65535", + detail::make_host_and_port_string("example.com", 65535, false)); + EXPECT_EQ("example.com:65536", + detail::make_host_and_port_string("example.com", 65536, false)); +} + TEST(DirtyDataRequestTest, HeadFieldValueContains_CR_LF_NUL) { Server svr;