(websocket client + server) WebSocketMessage class tweak to fix unsafe patterns

* Fix unsafe calls and safeguard WebSocketMessage from being called w/
temporaries

* Use unnamed namespace to express internal linkage

.github/workflows/mkdocs.yml

@@ -21,5 +21,7 @@ jobs:
         pip install pygments
     - name: Build doc
       run: |
+        git clean -dfx .
+        git fetch
         git pull
         mkdocs gh-deploy

README.md

@@ -24,6 +24,7 @@ A bad security bug affecting users compiling with SSL enabled and OpenSSL as the
 
 #include <ixwebsocket/IXNetSystem.h>
 #include <ixwebsocket/IXWebSocket.h>
+#include <ixwebsocket/IXUserAgent.h>
 #include <iostream>
 
 int main()
@@ -34,6 +35,8 @@ int main()
     // Our websocket object
     ix::WebSocket webSocket;
 
+    // Connect to a server with encryption
+    // See https://machinezone.github.io/IXWebSocket/usage/#tls-support-and-configuration
     std::string url("wss://echo.websocket.org");
     webSocket.setUrl(url);
 
@@ -53,6 +56,12 @@ int main()
                 std::cout << "Connection established" << std::endl;
                 std::cout << "> " << std::flush;
             }
+            else if (msg->type == ix::WebSocketMessageType::Error)
+            {
+                // Maybe SSL is not configured properly
+                std::cout << "Connection error: " << msg->errorInfo.reason << std::endl;
+                std::cout << "> " << std::flush;
+            }
         }
     );
 
@@ -123,6 +132,7 @@ To check the performance of a websocket library, you can look at the [autoroute]
 | Windows           | Disabled          | None              | [![Build2][5]][0] |
 | UWP               | Disabled          | None              | [![Build2][6]][0] |
 | Linux             | OpenSSL           | Address Sanitizer | [![Build2][7]][0] |
+| Mingw             | Disabled          | None              | [![Build2][8]][0] |
 
 * ASAN fails on Linux because of a known problem, we need a 
 * Some tests are disabled on Windows/UWP because of a pathing problem
@@ -136,4 +146,5 @@ To check the performance of a websocket library, you can look at the [autoroute]
 [5]: https://github.com/machinezone/IXWebSocket/workflows/windows/badge.svg
 [6]: https://github.com/machinezone/IXWebSocket/workflows/uwp/badge.svg
 [7]: https://github.com/machinezone/IXWebSocket/workflows/linux_asan/badge.svg
+[8]: https://github.com/machinezone/IXWebSocket/workflows/unittest_windows_gcc/badge.svg
 

docs/CHANGELOG.md

@@ -2,6 +2,22 @@
 
 All changes to this project will be documented in this file.
 
+## [11.2.8] - 2021-06-03
+
+(websocket client + server) WebSocketMessage class tweak to fix unsafe patterns
+
+## [11.2.7] - 2021-05-27
+
+(websocket server) Handle and accept firefox browser special upgrade value (keep-alive, Upgrade)
+
+## [11.2.6] - 2021-05-18
+
+(Windows) move EINVAL (re)definition from IXSocket.h to IXNetSystem.h (fix #289)
+
+## [11.2.5] - 2021-04-04
+
+(http client) DEL is not an HTTP method name, but DELETE is
+
 ## [11.2.4] - 2021-03-25
 
 (cmake) install IXUniquePtr.h

docs/usage.md

@@ -374,13 +374,10 @@ The webSocket reference is guaranteed to be always valid ; by design the callbac
 // Bound host name, max connections and listen backlog can also be passed in as parameters.
 ix::WebSocketServer server(port);
 
-server.setOnClientMessageCallback(std::shared_ptr<ConnectionState> connectionState,
-                                  WebSocket& webSocket,
-                                  const WebSocketMessagePtr& msg)
-{
+server.setOnClientMessageCallback([](std::shared_ptr<ix::ConnectionState> connectionState, ix::WebSocket & webSocket, const ix::WebSocketMessagePtr & msg) {
     // The ConnectionState object contains information about the connection,
     // at this point only the client ip address and the port.
-    std::cout << "Remote ip: " << connectionState->getRemoteIp();
+    std::cout << "Remote ip: " << connectionState->getRemoteIp() << std::endl;
 
     if (msg->type == ix::WebSocketMessageType::Open)
     {
@@ -398,7 +395,7 @@ server.setOnClientMessageCallback(std::shared_ptr<ConnectionState> connectionSta
         std::cout << "Headers:" << std::endl;
         for (auto it : msg->openInfo.headers)
         {
-            std::cout << it.first << ": " << it.second << std::endl;
+            std::cout << "\t" << it.first << ": " << it.second << std::endl;
         }
     }
     else if (msg->type == ix::WebSocketMessageType::Message)
@@ -407,9 +404,11 @@ server.setOnClientMessageCallback(std::shared_ptr<ConnectionState> connectionSta
         // All connected clients are available in an std::set. See the broadcast cpp example.
         // Second parameter tells whether we are sending the message in binary or text mode.
         // Here we send it in the same mode as it was received.
+        std::cout << "Received: " << msg->str << std::endl;
+
         webSocket.send(msg->str, msg->binary);
     }
-);
+});
 
 auto res = server.listen();
 if (!res.first)

ixwebsocket/IXHttp.cpp

@@ -137,7 +137,7 @@ namespace ix
             {
                 contentLength = std::stoi(headers["Content-Length"]);
             }
-            catch (std::exception)
+            catch (const std::exception&)
             {
                 return std::make_tuple(
                     false, "Error parsing HTTP Header 'Content-Length'", httpRequest);

ixwebsocket/IXHttpClient.cpp

@@ -20,10 +20,11 @@
 
 namespace ix
 {
+    // https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods
     const std::string HttpClient::kPost = "POST";
     const std::string HttpClient::kGet = "GET";
     const std::string HttpClient::kHead = "HEAD";
-    const std::string HttpClient::kDel = "DEL";
+    const std::string HttpClient::kDelete = "DELETE";
     const std::string HttpClient::kPut = "PUT";
     const std::string HttpClient::kPatch = "PATCH";
 
@@ -557,9 +558,9 @@ namespace ix
         return request(url, kHead, std::string(), args);
     }
 
-    HttpResponsePtr HttpClient::del(const std::string& url, HttpRequestArgsPtr args)
+    HttpResponsePtr HttpClient::Delete(const std::string& url, HttpRequestArgsPtr args)
     {
-        return request(url, kDel, std::string(), args);
+        return request(url, kDelete, std::string(), args);
     }
 
     HttpResponsePtr HttpClient::request(const std::string& url,

ixwebsocket/IXHttpClient.h

@@ -30,7 +30,7 @@ namespace ix
 
         HttpResponsePtr get(const std::string& url, HttpRequestArgsPtr args);
         HttpResponsePtr head(const std::string& url, HttpRequestArgsPtr args);
-        HttpResponsePtr del(const std::string& url, HttpRequestArgsPtr args);
+        HttpResponsePtr Delete(const std::string& url, HttpRequestArgsPtr args);
 
         HttpResponsePtr post(const std::string& url,
                              const HttpParameters& httpParameters,
@@ -94,7 +94,7 @@ namespace ix
         const static std::string kPost;
         const static std::string kGet;
         const static std::string kHead;
-        const static std::string kDel;
+        const static std::string kDelete;
         const static std::string kPut;
         const static std::string kPatch;
 

ixwebsocket/IXNetSystem.cpp

@@ -193,6 +193,7 @@ namespace ix
 #endif
     }
 
+#if defined(_WIN32) && defined(__GNUC__)
     static int hexval(unsigned c)
     {
         if (c - '0' < 10) return c - '0';
@@ -200,6 +201,7 @@ namespace ix
         if (c - 'a' < 6) return c - 'a' + 10;
         return -1;
     }
+#endif
 
     //
     // mingw does not have inet_pton, which were taken as is from the musl C library.

ixwebsocket/IXNetSystem.h

@@ -18,6 +18,19 @@
 #include <io.h>
 #include <ws2def.h>
 
+#undef EWOULDBLOCK
+#undef EAGAIN
+#undef EINPROGRESS
+#undef EBADF
+#undef EINVAL
+
+// map to WSA error codes
+#define EWOULDBLOCK WSAEWOULDBLOCK
+#define EAGAIN WSATRY_AGAIN
+#define EINPROGRESS WSAEINPROGRESS
+#define EBADF WSAEBADF
+#define EINVAL WSAEINVAL
+
 // Define our own poll on Windows, as a wrapper on top of select
 typedef unsigned long int nfds_t;
 

ixwebsocket/IXSocket.h

@@ -15,20 +15,6 @@
 #ifdef _WIN32
 #include <BaseTsd.h>
 typedef SSIZE_T ssize_t;
-
-#undef EWOULDBLOCK
-#undef EAGAIN
-#undef EINPROGRESS
-#undef EBADF
-#undef EINVAL
-
-// map to WSA error codes
-#define EWOULDBLOCK WSAEWOULDBLOCK
-#define EAGAIN WSATRY_AGAIN
-#define EINPROGRESS WSAEINPROGRESS
-#define EBADF WSAEBADF
-#define EINVAL WSAEINVAL
-
 #endif
 
 #include "IXCancellationRequest.h"

ixwebsocket/IXWebSocket.cpp

@@ -15,6 +15,12 @@
 #include <cmath>
 
 
+namespace
+{
+    const std::string emptyMsg;
+} // namespace
+
+
 namespace ix
 {
     OnTrafficTrackerCallback WebSocket::_onTrafficTrackerCallback = nullptr;
@@ -38,7 +44,7 @@ namespace ix
             [this](uint16_t code, const std::string& reason, size_t wireSize, bool remote) {
                 _onMessageCallback(
                     ix::make_unique<WebSocketMessage>(WebSocketMessageType::Close,
-                                                      "",
+                                                      emptyMsg,
                                                       wireSize,
                                                       WebSocketErrorInfo(),
                                                       WebSocketOpenInfo(),
@@ -217,7 +223,7 @@ namespace ix
 
         _onMessageCallback(ix::make_unique<WebSocketMessage>(
             WebSocketMessageType::Open,
-            "",
+            emptyMsg,
             0,
             WebSocketErrorInfo(),
             WebSocketOpenInfo(status.uri, status.headers, status.protocol),
@@ -251,7 +257,7 @@ namespace ix
 
         _onMessageCallback(
             ix::make_unique<WebSocketMessage>(WebSocketMessageType::Open,
-                                              "",
+                                              emptyMsg,
                                               0,
                                               WebSocketErrorInfo(),
                                               WebSocketOpenInfo(status.uri, status.headers),
@@ -338,7 +344,7 @@ namespace ix
                 connectErr.http_status = status.http_status;
 
                 _onMessageCallback(ix::make_unique<WebSocketMessage>(WebSocketMessageType::Error,
-                                                                     "",
+                                                                     emptyMsg,
                                                                      0,
                                                                      connectErr,
                                                                      WebSocketOpenInfo(),

ixwebsocket/IXWebSocketHandshake.cpp

@@ -204,6 +204,9 @@ namespace ix
         // Check the value of the connection field
         // Some websocket servers (Go/Gorilla?) send lowercase values for the
         // connection header, so do a case insensitive comparison
+        //
+        // See https://github.com/apache/thrift/commit/7c4bdf9914fcba6c89e0f69ae48b9675578f084a
+        //
         if (!insensitiveStringCompare(headers["connection"], "Upgrade"))
         {
             std::stringstream ss;
@@ -296,7 +299,8 @@ namespace ix
             return sendErrorResponse(400, "Missing Upgrade header");
         }
 
-        if (!insensitiveStringCompare(headers["upgrade"], "WebSocket"))
+        if (!insensitiveStringCompare(headers["upgrade"], "WebSocket") &&
+            headers["Upgrade"] != "keep-alive, Upgrade") // special case for firefox
         {
             return sendErrorResponse(400,
                                      "Invalid Upgrade header, "

ixwebsocket/IXWebSocketMessage.h

@@ -42,6 +42,18 @@ namespace ix
         {
             ;
         }
+
+        /**
+         * @brief Deleted overload to prevent binding `str` to a temporary, which would cause
+         * undefined behavior since class members don't extend lifetime beyond the constructor call.
+         */
+        WebSocketMessage(WebSocketMessageType t,
+                         std::string&& s,
+                         size_t w,
+                         WebSocketErrorInfo e,
+                         WebSocketOpenInfo o,
+                         WebSocketCloseInfo c,
+                         bool b = false) = delete;
     };
 
     using WebSocketMessagePtr = std::unique_ptr<WebSocketMessage>;

ixwebsocket/IXWebSocketServer.cpp

@@ -191,7 +191,6 @@ namespace ix
                         // Make sure the OS send buffer is flushed before moving on
                         do
                         {
-                            size_t bufferedAmount = client->bufferedAmount();
                             std::chrono::duration<double, std::milli> duration(500);
                             std::this_thread::sleep_for(duration);
                         } while (client->bufferedAmount() != 0);

ixwebsocket/IXWebSocketVersion.h

@@ -6,4 +6,4 @@
 
 #pragma once
 
-#define IX_WEBSOCKET_VERSION "11.2.4"
+#define IX_WEBSOCKET_VERSION "11.2.8"

main.cpp

@@ -15,6 +15,7 @@
 
 #include <ixwebsocket/IXNetSystem.h>
 #include <ixwebsocket/IXWebSocket.h>
+#include <ixwebsocket/IXUserAgent.h>
 #include <iostream>
 
 int main()
@@ -25,9 +26,12 @@ int main()
     // Our websocket object
     ix::WebSocket webSocket;
 
+    // Connect to a server with encryption
+    // See https://machinezone.github.io/IXWebSocket/usage/#tls-support-and-configuration
     std::string url("wss://echo.websocket.org");
     webSocket.setUrl(url);
 
+    std::cout << ix::userAgent() << std::endl;
     std::cout << "Connecting to " << url << "..." << std::endl;
 
     // Setup a callback to be fired (in a background thread, watch out for race conditions !)
@@ -44,6 +48,12 @@ int main()
                 std::cout << "Connection established" << std::endl;
                 std::cout << "> " << std::flush;
             }
+            else if (msg->type == ix::WebSocketMessageType::Error)
+            {
+                // Maybe SSL is not configured properly
+                std::cout << "Connection error: " << msg->errorInfo.reason << std::endl;
+                std::cout << "> " << std::flush;
+            }
         }
     );