From 8942cf6dc9f605095ec89b8c9d8d4bc1869562b9 Mon Sep 17 00:00:00 2001 From: Cosmin Date: Sat, 18 Apr 2026 16:57:08 +0300 Subject: [PATCH 1/4] Fix bug where bracketed IPv6 will fail to parse - e.g. [::1]:9001 --- ixwebsocket/IXUrlParser.cpp | 12 +++++++++++- test/IXWebSocketIPv6Test.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/ixwebsocket/IXUrlParser.cpp b/ixwebsocket/IXUrlParser.cpp index c5cdb57b..28f67820 100644 --- a/ixwebsocket/IXUrlParser.cpp +++ b/ixwebsocket/IXUrlParser.cpp @@ -251,7 +251,17 @@ namespace LocalString++; } - Result.m_Host = std::string(CurrentString, LocalString - CurrentString); + // For bracketed IPv6 addresses like [::1], strip the surrounding brackets + if (bHasBracket) + { + // CurrentString points to '[', LocalString points one past ']' + // so skip the '[' and exclude the ']' + Result.m_Host = std::string(CurrentString + 1, LocalString - CurrentString - 2); + } + else + { + Result.m_Host = std::string(CurrentString, LocalString - CurrentString); + } CurrentString = LocalString; diff --git a/test/IXWebSocketIPv6Test.cpp b/test/IXWebSocketIPv6Test.cpp index 94e5df3c..b394fe67 100644 --- a/test/IXWebSocketIPv6Test.cpp +++ b/test/IXWebSocketIPv6Test.cpp @@ -1,5 +1,6 @@ #include "IXTest.h" #include "catch.hpp" +#include #include #include @@ -7,6 +8,31 @@ using namespace ix; TEST_CASE("IPv6") { + SECTION("URL parser strips brackets from IPv6 host and parses port") + { + std::string protocol, host, path, query; + int port = -1; + + bool ok = UrlParser::parse("ws://[::1]:9001/chat", protocol, host, path, query, port); + CHECK(ok); + CHECK(protocol == "ws"); + CHECK(host == "::1"); + CHECK(port == 9001); + CHECK(path == "/chat"); + } + + SECTION("URL parser handles IPv6 without port") + { + std::string protocol, host, path, query; + int port = -1; + + bool ok = UrlParser::parse("ws://[::1]/", protocol, host, path, query, port); + CHECK(ok); + CHECK(protocol == "ws"); + CHECK(host == "::1"); + CHECK(port == 80); + } + SECTION("Listening on ::1 works with AF_INET6 works") { int port = getFreePort(); From d71fc8490d077a06b06f7caa8aeb515924ef311a Mon Sep 17 00:00:00 2001 From: Cosmin Date: Sat, 18 Apr 2026 17:05:26 +0300 Subject: [PATCH 2/4] Fix issue where client couldn't connect via IPv6 bracketed address, even after previous parsing fix --- ixwebsocket/IXWebSocketHandshake.cpp | 8 +++-- test/IXWebSocketIPv6Test.cpp | 45 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/ixwebsocket/IXWebSocketHandshake.cpp b/ixwebsocket/IXWebSocketHandshake.cpp index 744e3ab7..9ac86b61 100644 --- a/ixwebsocket/IXWebSocketHandshake.cpp +++ b/ixwebsocket/IXWebSocketHandshake.cpp @@ -112,11 +112,15 @@ namespace ix // See https://stackoverflow.com/questions/18265128/what-is-sec-websocket-key-for std::string secWebSocketKey = macaron::Base64::Encode(genRandomString(16)); + // For IPv6 addresses, brackets are required in Host and Origin headers (RFC 7230) + bool isIPv6Host = host.find(':') != std::string::npos; + std::string bracketedHost = isIPv6Host ? "[" + host + "]" : host; + std::stringstream ss; ss << "GET " << path << " HTTP/1.1\r\n"; if (extraHeaders.find("Host") == extraHeaders.end()) { - ss << "Host: " << host << ":" << port << "\r\n"; + ss << "Host: " << bracketedHost << ":" << port << "\r\n"; } ss << "Upgrade: websocket\r\n"; ss << "Connection: Upgrade\r\n"; @@ -132,7 +136,7 @@ namespace ix // Set an origin header if missing if (extraHeaders.find("Origin") == extraHeaders.end()) { - ss << "Origin: " << protocol << "://" << host << ":" << port << "\r\n"; + ss << "Origin: " << protocol << "://" << bracketedHost << ":" << port << "\r\n"; } for (auto& it : extraHeaders) diff --git a/test/IXWebSocketIPv6Test.cpp b/test/IXWebSocketIPv6Test.cpp index b394fe67..883cb377 100644 --- a/test/IXWebSocketIPv6Test.cpp +++ b/test/IXWebSocketIPv6Test.cpp @@ -48,4 +48,49 @@ TEST_CASE("IPv6") server.start(); server.stop(); } + + SECTION("Client connects to server via [::1]:port notation") + { + int port = getFreePort(); + ix::WebSocketServer server(port, + "::1", + SocketServer::kDefaultTcpBacklog, + SocketServer::kDefaultMaxConnections, + WebSocketServer::kDefaultHandShakeTimeoutSecs, + AF_INET6); + + server.setOnClientMessageCallback( + [](std::shared_ptr /*connectionState*/, + ix::WebSocket& /*webSocket*/, + const ix::WebSocketMessagePtr& /*msg*/) {}); + + auto res = server.listen(); + REQUIRE(res.first); + server.start(); + + // Connect using bracketed IPv6 URL notation + std::string url = "ws://[::1]:" + std::to_string(port) + "/"; + ix::WebSocket client; + client.setUrl(url); + + std::atomic connected{false}; + client.setOnMessageCallback([&connected](const ix::WebSocketMessagePtr& msg) { + if (msg->type == ix::WebSocketMessageType::Open) + { + connected = true; + } + }); + + client.start(); + int retries = 0; + while (!connected && retries < 20) + { + ix::msleep(100); + retries++; + } + client.stop(); + server.stop(); + + CHECK(connected); + } } From 4303a3721a5cf2efe1b4297f03430803ab32c569 Mon Sep 17 00:00:00 2001 From: Cosmin Date: Sat, 18 Apr 2026 17:38:05 +0300 Subject: [PATCH 3/4] Refactor clParseURL::ParseURL function and implement multiple malformed URI tests - implementation is a bit cleaner now and each step is much more explicit --- ixwebsocket/IXUrlParser.cpp | 345 ++++++++++++++++++++---------------- test/IXUrlParserTest.cpp | 62 +++++++ 2 files changed, 250 insertions(+), 157 deletions(-) diff --git a/ixwebsocket/IXUrlParser.cpp b/ixwebsocket/IXUrlParser.cpp index 28f67820..6bd72748 100644 --- a/ixwebsocket/IXUrlParser.cpp +++ b/ixwebsocket/IXUrlParser.cpp @@ -44,9 +44,18 @@ namespace LUrlParserError_NoUrlCharacter = 2, LUrlParserError_InvalidSchemeName = 3, LUrlParserError_NoDoubleSlash = 4, - LUrlParserError_NoAtSign = 5, - LUrlParserError_UnexpectedEndOfLine = 6, - LUrlParserError_NoSlash = 7, + LUrlParserError_NoSlash = 5, + LUrlParserError_MissingClosingBracketForIPv6 = 6, + LUrlParserError_InvalidCharacterAfterIPv6Host = 7, + LUrlParserError_MissingHost = 8, + LUrlParserError_InvalidPort = 9, + LUrlParserError_UnmatchedClosingBracketInHost = 10, + LUrlParserError_InvalidOpeningBracketInHost = 11, + LUrlParserError_InvalidPortSeparator = 12, + LUrlParserError_MissingPortAfterColon = 13, + LUrlParserError_EmptyIPv6Host = 14, + LUrlParserError_UnbracketedIPv6Host = 15, + LUrlParserError_MultipleAtSignsInAuthority = 16, }; class clParseURL @@ -119,230 +128,252 @@ namespace return true; } - // based on RFC 1738 and RFC 3986 + // Practical URI parser for ws/wss/http/https URLs. + // + // Standards references: + // - RFC 3986: generic URI syntax used for scheme, authority, path, query, fragment. + // - RFC 3986 Section 3.2.2: IP-literal host syntax (bracketed IPv6 in authority). + // - RFC 6455 Section 3: ws/wss URI schemes (default ports handled in getProtocolPort). + // + // Note: this is not a full RFC validator; it is a fast parser with targeted syntax checks. clParseURL clParseURL::ParseURL(const std::string& URL) { clParseURL Result; + const std::size_t npos = std::string::npos; + std::size_t current = 0; - const char* CurrentString = URL.c_str(); - - /* - * : - * := [a-z\+\-\.]+ - * For resiliency, programs interpreting URLs should treat upper case letters as - *equivalent to lower case in scheme names - */ - - // try to read scheme + // Step 1: read scheme (:) and normalize to lowercase. + std::size_t schemeEnd = URL.find(':', current); + if (schemeEnd == npos) { - const char* LocalString = strchr(CurrentString, ':'); - - if (!LocalString) - { - return clParseURL(LUrlParserError_NoUrlCharacter); - } + return clParseURL(LUrlParserError_NoUrlCharacter); + } - // save the scheme name - Result.m_Scheme = std::string(CurrentString, LocalString - CurrentString); + Result.m_Scheme = URL.substr(current, schemeEnd - current); + if (!IsSchemeValid(Result.m_Scheme)) + { + return clParseURL(LUrlParserError_InvalidSchemeName); + } - if (!IsSchemeValid(Result.m_Scheme)) - { - return clParseURL(LUrlParserError_InvalidSchemeName); - } + std::transform(Result.m_Scheme.begin(), + Result.m_Scheme.end(), + Result.m_Scheme.begin(), + ::tolower); - // scheme should be lowercase - std::transform( - Result.m_Scheme.begin(), Result.m_Scheme.end(), Result.m_Scheme.begin(), ::tolower); + current = schemeEnd + 1; - // skip ':' - CurrentString = LocalString + 1; + // Step 2: require authority prefix "//". + if (current + 1 >= URL.size() || URL[current] != '/' || URL[current + 1] != '/') + { + return clParseURL(LUrlParserError_NoDoubleSlash); } + current += 2; - /* - * //:@:/ - * any ":", "@" and "/" must be normalized - */ - - // skip "//" - if (*CurrentString++ != '/') return clParseURL(LUrlParserError_NoDoubleSlash); - if (*CurrentString++ != '/') return clParseURL(LUrlParserError_NoDoubleSlash); - - // check if the user name and password are specified - bool bHasUserName = false; + // Step 3: locate the authority boundary ( ends at '/' or '?'). + std::size_t authorityEnd = URL.find_first_of("/?", current); + if (authorityEnd == npos) + { + authorityEnd = URL.size(); + } - const char* LocalString = CurrentString; + // Step 4: parse optional user info ([:]@). + bool hasUserInfo = false; + std::size_t atPos = URL.find('@', current); + if (atPos != npos && atPos < authorityEnd) + { + hasUserInfo = true; + } - while (*LocalString) + if (hasUserInfo) { - if (*LocalString == '@') + // User info can contain ':' but must contain exactly one '@' separator + // within the authority segment. + if (URL.find('@', atPos + 1) != npos && URL.find('@', atPos + 1) < authorityEnd) + { + return clParseURL(LUrlParserError_MultipleAtSignsInAuthority); + } + + std::size_t colonPos = URL.find(':', current); + if (colonPos != npos && colonPos < atPos) { - // user name and password are specified - bHasUserName = true; - break; + Result.m_UserName = URL.substr(current, colonPos - current); + Result.m_Password = URL.substr(colonPos + 1, atPos - colonPos - 1); } - else if (*LocalString == '/' || *LocalString == '?') + else { - // end of : specification - bHasUserName = false; - break; + Result.m_UserName = URL.substr(current, atPos - current); } - LocalString++; + current = atPos + 1; } - // user name and password - LocalString = CurrentString; - - if (bHasUserName) + // Step 5: parse host and optional port from authority tail. + authorityEnd = URL.find_first_of("/?", current); + if (authorityEnd == npos) { - // read user name - while (*LocalString && *LocalString != ':' && *LocalString != '@') - LocalString++; + authorityEnd = URL.size(); + } - Result.m_UserName = std::string(CurrentString, LocalString - CurrentString); + std::string authority = URL.substr(current, authorityEnd - current); + std::size_t openingBracketPos = authority.find('['); + std::size_t closingBracketPos = authority.find(']'); + + // Malformed authority checks (RFC 3986): + // - IP-literals must be enclosed in '[' and ']' (Section 3.2.2). + // - A closing bracket without an opening bracket is invalid. + // - An opening bracket is only valid at the beginning of host when parsing [IPv6]. + // - Unbracketed multi-colon authorities are either invalid separators or IPv6 literals + // missing required brackets. + if (closingBracketPos != npos && openingBracketPos == npos) + { + return clParseURL(LUrlParserError_UnmatchedClosingBracketInHost); + } - // proceed with the current pointer - CurrentString = LocalString; + if (openingBracketPos != npos && openingBracketPos != 0) + { + return clParseURL(LUrlParserError_InvalidOpeningBracketInHost); + } - if (*CurrentString == ':') + if (openingBracketPos == npos) + { + const auto colonCount = std::count(authority.begin(), authority.end(), ':'); + if (colonCount > 1) { - // skip ':' - CurrentString++; - - // read password - LocalString = CurrentString; + bool looksLikeUnbracketedIPv6 = + authority.find('.') == npos && + authority.find_first_not_of("0123456789abcdefABCDEF:") == npos; - while (*LocalString && *LocalString != '@') - LocalString++; + if (looksLikeUnbracketedIPv6) + { + return clParseURL(LUrlParserError_UnbracketedIPv6Host); + } - Result.m_Password = std::string(CurrentString, LocalString - CurrentString); + return clParseURL(LUrlParserError_InvalidPortSeparator); + } + } - CurrentString = LocalString; + if (current < authorityEnd && URL[current] == '[') + { + // Step 5a: bracketed IPv6 host: [][:] + std::size_t closeBracket = URL.find(']', current + 1); + if (closeBracket == npos || closeBracket >= authorityEnd) + { + // Bracketed IPv6 literal started but did not terminate before authority end. + return clParseURL(LUrlParserError_MissingClosingBracketForIPv6); } - // skip '@' - if (*CurrentString != '@') + Result.m_Host = URL.substr(current + 1, closeBracket - current - 1); + if (Result.m_Host.empty()) { - return clParseURL(LUrlParserError_NoAtSign); + return clParseURL(LUrlParserError_EmptyIPv6Host); } - CurrentString++; + std::size_t afterBracket = closeBracket + 1; + if (afterBracket < authorityEnd) + { + if (URL[afterBracket] != ':') + { + // After ] only ':' is allowed before authority terminates. + return clParseURL(LUrlParserError_InvalidCharacterAfterIPv6Host); + } + Result.m_Port = URL.substr(afterBracket + 1, authorityEnd - afterBracket - 1); + if (Result.m_Port.empty()) + { + // ':' present, but no port digits followed. + return clParseURL(LUrlParserError_MissingPortAfterColon); + } + } } - - bool bHasBracket = (*CurrentString == '['); - - // go ahead, read the host name - LocalString = CurrentString; - - while (*LocalString) + else { - if (bHasBracket && *LocalString == ']') + // Step 5b: regular host: [:] + std::size_t colonPos = URL.find(':', current); + if (colonPos != npos && colonPos < authorityEnd) { - // end of IPv6 address - LocalString++; - break; + Result.m_Host = URL.substr(current, colonPos - current); + Result.m_Port = URL.substr(colonPos + 1, authorityEnd - colonPos - 1); + if (Result.m_Port.empty()) + { + // ':' present, but no port digits followed. + return clParseURL(LUrlParserError_MissingPortAfterColon); + } } - else if (!bHasBracket && (*LocalString == ':' || *LocalString == '/' || *LocalString == '?')) + else { - // port number is specified - break; + Result.m_Host = URL.substr(current, authorityEnd - current); } - - LocalString++; } - // For bracketed IPv6 addresses like [::1], strip the surrounding brackets - if (bHasBracket) - { - // CurrentString points to '[', LocalString points one past ']' - // so skip the '[' and exclude the ']' - Result.m_Host = std::string(CurrentString + 1, LocalString - CurrentString - 2); - } - else + // Step 5c: validate host/port extracted from authority. + if (Result.m_Host.empty()) { - Result.m_Host = std::string(CurrentString, LocalString - CurrentString); + // Host is mandatory in authority form: //host[:port] + return clParseURL(LUrlParserError_MissingHost); } - CurrentString = LocalString; - - // is port number specified? - if (*CurrentString == ':') + if (Result.m_Port.find_first_not_of("0123456789") != std::string::npos) { - CurrentString++; - - // read port number - LocalString = CurrentString; - - while (*LocalString && *LocalString != '/') - LocalString++; - - Result.m_Port = std::string(CurrentString, LocalString - CurrentString); - - CurrentString = LocalString; + // Port must be decimal digits only (conversion/range checked later by GetPort). + return clParseURL(LUrlParserError_InvalidPort); } - // end of string - if (!*CurrentString) + current = authorityEnd; + + // Step 6: if authority reaches end of string, URL parsing is complete. + if (current >= URL.size()) { Result.m_ErrorCode = LUrlParserError_Ok; - return Result; } - // skip '/' - if (*CurrentString != '/' && *CurrentString != '?') + // Step 7: parse path start (either '/' or query-only '?'). + if (URL[current] != '/' && URL[current] != '?') { return clParseURL(LUrlParserError_NoSlash); } - if (*CurrentString != '?') { - CurrentString++; + if (URL[current] == '/') + { + ++current; } - // parse the path - LocalString = CurrentString; - - while (*LocalString && *LocalString != '#' && *LocalString != '?') - LocalString++; - - Result.m_Path = std::string(CurrentString, LocalString - CurrentString); - - CurrentString = LocalString; - - // check for query - if (*CurrentString == '?') + // Step 8: parse path up to '?' or '#'. + std::size_t pathEnd = URL.find_first_of("?#", current); + if (pathEnd == npos) { - // skip '?' - CurrentString++; + Result.m_Path = URL.substr(current); + Result.m_ErrorCode = LUrlParserError_Ok; + return Result; + } - // read query - LocalString = CurrentString; + Result.m_Path = URL.substr(current, pathEnd - current); + current = pathEnd; - while (*LocalString && *LocalString != '#') - LocalString++; + // Step 9: parse optional query after '?', up to '#'. + if (current < URL.size() && URL[current] == '?') + { + ++current; - Result.m_Query = std::string(CurrentString, LocalString - CurrentString); + std::size_t queryEnd = URL.find('#', current); + if (queryEnd == npos) + { + Result.m_Query = URL.substr(current); + Result.m_ErrorCode = LUrlParserError_Ok; + return Result; + } - CurrentString = LocalString; + Result.m_Query = URL.substr(current, queryEnd - current); + current = queryEnd; } - // check for fragment - if (*CurrentString == '#') + // Step 10: parse optional fragment after '#', to end of URL. + if (current < URL.size() && URL[current] == '#') { - // skip '#' - CurrentString++; - - // read fragment - LocalString = CurrentString; - - while (*LocalString) - LocalString++; - - Result.m_Fragment = std::string(CurrentString, LocalString - CurrentString); + ++current; + Result.m_Fragment = URL.substr(current); } Result.m_ErrorCode = LUrlParserError_Ok; - return Result; } diff --git a/test/IXUrlParserTest.cpp b/test/IXUrlParserTest.cpp index dd2786dc..ded7b13d 100644 --- a/test/IXUrlParserTest.cpp +++ b/test/IXUrlParserTest.cpp @@ -9,6 +9,7 @@ #include #include #include +#include using namespace ix; @@ -147,6 +148,67 @@ namespace ix "status=true&format=protobuf"); REQUIRE(port == 7350); } + + SECTION("reject malformed IPv4 authority") + { + std::vector malformedUrls = { + "ws://:9001/", // empty host + "ws://127.0.0.1::9001/", // malformed port separator + "ws://127.0.0.1:abc/", // non-numeric port + "ws://127.0.0.1:9001abc/", // non-numeric port suffix + "ws://127.0.0.1:/", // missing digits after ':' + }; + + for (const auto& url : malformedUrls) + { + std::string protocol, host, path, query; + int port = -1; + + bool res = UrlParser::parse(url, protocol, host, path, query, port); + CHECK_FALSE(res); + } + } + + SECTION("reject malformed bracketed IPv6 authority") + { + std::vector malformedUrls = { + "ws://[::1/", // missing closing bracket + "ws://[::1]x", // invalid token after closing bracket + "ws://[]:9001/", // empty IPv6 host + "ws://[::1]:abc/", // non-numeric port + "ws://[::1]:9001x/", // non-numeric port suffix + "ws://[::1]:/", // missing digits after ':' + "ws://::1]/", // unmatched closing bracket in authority + "ws://[::1/:9001", // missing closing bracket before path + "ws://::1/", // unbracketed IPv6 authority + }; + + for (const auto& url : malformedUrls) + { + std::string protocol, host, path, query; + int port = -1; + + bool res = UrlParser::parse(url, protocol, host, path, query, port); + CHECK_FALSE(res); + } + } + + SECTION("reject malformed userinfo") + { + std::vector malformedUrls = { + "ws://user@@127.0.0.1/", // multiple @ separators in authority + "ws://user:pass@@[::1]:9001/" // multiple @ separators in authority + }; + + for (const auto& url : malformedUrls) + { + std::string protocol, host, path, query; + int port = -1; + + bool res = UrlParser::parse(url, protocol, host, path, query, port); + CHECK_FALSE(res); + } + } } } // namespace ix From 3e86cc813af9bae0cead0afe53d83a24afaac74f Mon Sep 17 00:00:00 2001 From: Cosmin Polifronie Date: Sat, 18 Apr 2026 20:56:49 +0300 Subject: [PATCH 4/4] Fix old remaining FIXME where too small of a buffer was used for IPv6 which caused crashes on Windows --- ixwebsocket/IXSocketServer.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ixwebsocket/IXSocketServer.cpp b/ixwebsocket/IXSocketServer.cpp index a0d98cff..790cfdf5 100644 --- a/ixwebsocket/IXSocketServer.cpp +++ b/ixwebsocket/IXSocketServer.cpp @@ -308,9 +308,11 @@ namespace ix } // Accept a connection. - // FIXME: Is this working for ipv6 ? - struct sockaddr_in client; // client address information - int clientFd; // socket connected to client + // Use sockaddr_storage to accommodate both AF_INET and AF_INET6 addresses. + // sockaddr_in is only 16 bytes; sockaddr_in6 is 28 bytes. On Windows, passing + // a too-small buffer to accept() causes WSAEFAULT (error 10014). + struct sockaddr_storage client; // client address information (IPv4 or IPv6) + int clientFd; // socket connected to client socklen_t addressLen = sizeof(client); memset(&client, 0, sizeof(client)); @@ -347,7 +349,8 @@ namespace ix if (_addressFamily == AF_INET) { char remoteIp4[INET_ADDRSTRLEN]; - if (ix::inet_ntop(AF_INET, &client.sin_addr, remoteIp4, INET_ADDRSTRLEN) == nullptr) + auto* client4 = reinterpret_cast(&client); + if (ix::inet_ntop(AF_INET, &client4->sin_addr, remoteIp4, INET_ADDRSTRLEN) == nullptr) { int err = Socket::getErrno(); std::stringstream ss; @@ -360,13 +363,14 @@ namespace ix continue; } - remotePort = ix::network_to_host_short(client.sin_port); + remotePort = ix::network_to_host_short(client4->sin_port); remoteIp = remoteIp4; } else // AF_INET6 { char remoteIp6[INET6_ADDRSTRLEN]; - if (ix::inet_ntop(AF_INET6, &client.sin_addr, remoteIp6, INET6_ADDRSTRLEN) == + auto* client6 = reinterpret_cast(&client); + if (ix::inet_ntop(AF_INET6, &client6->sin6_addr, remoteIp6, INET6_ADDRSTRLEN) == nullptr) { int err = Socket::getErrno(); @@ -380,7 +384,7 @@ namespace ix continue; } - remotePort = ix::network_to_host_short(client.sin_port); + remotePort = ix::network_to_host_short(client6->sin6_port); remoteIp = remoteIp6; }