RDKB-66319:[OneWifi] Add WebSocket library with RFC 6455 compliant client - #1311
Open
Aniket0606 wants to merge 2 commits into
Open
RDKB-66319:[OneWifi] Add WebSocket library with RFC 6455 compliant client#1311Aniket0606 wants to merge 2 commits into
Aniket0606 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new C++11 WebSocket client library intended to be RFC 6455–compliant and integrates it into the OneWifi build, including adding a dedicated logging module selector.
Changes:
- Added a new
WsClientimplementation (TLS + RFC6455 framing + background recv thread + simple “ack” helper). - Integrated the new
web_socketutility subcomponent into autotools build (configure.ac,source/utils/Makefile.am, newsource/utils/web_socket/Makefile.am). - Extended
wifi_utillogging module enum/switch to supportWIFI_WS.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| source/utils/wifi_util.h | Adds WIFI_WS log module enum value. |
| source/utils/wifi_util.c | Adds WIFI_WS handling in wifi_util_print() module routing. |
| source/utils/web_socket/src/ws_client.cpp | New WebSocket client implementation (handshake, framing, recv thread, ack helpers). |
| source/utils/web_socket/inc/ws_client.h | Public API for WsClient, including callback + ack helper methods. |
| source/utils/web_socket/Makefile.am | Builds new libwifi_web_socket.la and links OpenSSL/pthread. |
| source/utils/Makefile.am | Adds web_socket to SUBDIRS. |
| configure.ac | Adds source/utils/web_socket/Makefile to generated build files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <vector> | ||
|
|
||
| #include <cerrno> | ||
| #include <csignal> |
Comment on lines
+229
to
+236
| static int ssl_write_nosigpipe(SSL *ssl, const uint8_t *buf, int len) | ||
| { | ||
| sigset_t set; | ||
| sigemptyset(&set); | ||
| sigaddset(&set, SIGPIPE); | ||
| (void)pthread_sigmask(SIG_BLOCK, &set, nullptr); | ||
| return SSL_write(ssl, buf, len); | ||
| } |
Comment on lines
+129
to
+148
| if (url.substr(pos, 6) == "wss://") { | ||
| info.use_tls = true; pos += 6; | ||
| } else if (url.substr(pos, 5) == "ws://") { | ||
| info.use_tls = false; pos += 5; | ||
| } else { | ||
| info.use_tls = true; // assume wss for unknown scheme | ||
| } | ||
|
|
||
| // Host (ends at ':', '/', '?', or end) | ||
| std::size_t host_end = url.find_first_of(":/?\0", pos); | ||
| info.host = url.substr(pos, host_end == std::string::npos | ||
| ? std::string::npos : host_end - pos); | ||
| if (info.host.empty()) return info; // valid stays false | ||
| pos += info.host.size(); | ||
|
|
||
| // Optional port | ||
| long parsed_port = -1; | ||
| if (pos < url.size() && url[pos] == ':') { | ||
| ++pos; | ||
| std::size_t pend = url.find_first_of("/?\0", pos); |
Comment on lines
+336
to
+348
| static void notify_ack_waiters(WsClient::Impl& impl, const char *reason) | ||
| { | ||
| std::lock_guard<std::mutex> lk(impl.ack_mtx); | ||
| if (reason) { | ||
| impl.ack_resp = reason; | ||
| impl.ack_ready = true; | ||
| impl.ack_cv.notify_all(); | ||
| } else { | ||
| impl.ack_ready = false; | ||
| impl.ack_resp.clear(); | ||
| impl.ack_cv.notify_all(); // wake waiters so they detect the close | ||
| } | ||
| } |
Comment on lines
+727
to
+739
| int WsClient::connect(const std::string& url) | ||
| { | ||
| std::lock_guard<std::recursive_mutex> lk(impl_->mtx); | ||
| impl_->url = url; | ||
|
|
||
| int rc = connect_locked(*impl_); | ||
| if (rc != 0) return rc; | ||
|
|
||
| impl_->recv_running = true; | ||
| impl_->recv_thread = std::thread(recv_thread_fn, impl_.get()); | ||
| WS_INFO(*impl_, "connected — recv thread started\n"); | ||
| return 0; | ||
| } |
Comment on lines
+561
to
+564
| frame.payload.resize(len); | ||
| if (len > 0) { | ||
| if (!impl_read_exact(impl, frame.payload.data(), len)) return WsFrame{}; | ||
| if (is_masked) { |
Comment on lines
+303
to
+317
| if (payload_len > 65535) return false; // extend to 64-bit length if needed | ||
|
|
||
| uint8_t header[10]; | ||
| std::size_t header_len; | ||
| header[0] = static_cast<uint8_t>(0x80u | static_cast<uint8_t>(opcode)); | ||
|
|
||
| if (payload_len <= 125) { | ||
| header[1] = static_cast<uint8_t>(0x80u | payload_len); | ||
| header_len = 2; | ||
| } else { | ||
| header[1] = static_cast<uint8_t>(0x80u | 126u); | ||
| header[2] = static_cast<uint8_t>((payload_len >> 8) & 0xFFu); | ||
| header[3] = static_cast<uint8_t>(payload_len & 0xFFu); | ||
| header_len = 4; | ||
| } |
…ient
Reason for change:
- Implement libwifi_web_socket library with generic WsClient for wss:// connections
- Implement RFC 6455 WebSocket frame handling with automatic Ping/Pong
- Support TLS/SSL connections with OpenSSL integration
- Add background recv thread with ack synchronization mechanism
Test Procedure: 1. Load Onewifi build.
2. Run Websocket client.
Risks: Low
Priority: P1
Signed-off-by: apatel599@cable.comcast.com
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
source/utils/web_socket/src/ws_client.cpp:33
- pthread_sigmask() is used later in this file, but <pthread.h> is not included. This can fail to compile on toolchains that don’t declare pthread_* APIs via /<signal.h>.
#include <cerrno>
#include <csignal>
#include <cstdio>
#include <cstring>
source/utils/web_socket/src/ws_client.cpp:240
- ssl_write_nosigpipe() blocks SIGPIPE via pthread_sigmask(SIG_BLOCK, …) but never restores the previous thread signal mask. That leaves SIGPIPE blocked for the remainder of the thread and can mask signal handling bugs elsewhere.
(void)pthread_sigmask(SIG_BLOCK, &set, nullptr);
source/utils/web_socket/src/ws_client.cpp:753
- WsClient::connect() starts a new std::thread unconditionally once connect_locked() succeeds. If a previous recv thread exited due to a network error, impl_->socket may be invalid but impl_->recv_thread can still be joinable; assigning a new thread into a joinable std::thread triggers std::terminate.
impl_->recv_thread = std::thread(recv_thread_fn, impl_.get());
source/utils/web_socket/src/ws_client.cpp:392
- The handshake read loop appends whole recv() chunks until it sees "\r\n\r\n". If the server sends any WebSocket frame bytes in the same TCP record after the headers, those bytes are appended to resp and then discarded when do_ws_handshake() returns, so the first frame can be lost.
while (resp.find("\r\n\r\n") == std::string::npos) {
char tmp[256];
int n = impl_read(impl, tmp, sizeof(tmp) - 1);
if (n <= 0) return false;
resp.append(tmp, static_cast<std::size_t>(n));
source/utils/web_socket/src/ws_client.cpp:547
- RFC 6455 requires servers to send unmasked frames to clients. recv_frame() currently accepts masked server frames and silently unmasks them; this is a protocol violation and can mask interoperability issues.
bool is_masked = (hdr[1] & 0x80u) != 0;
std::size_t len = hdr[1] & 0x7Fu;
source/utils/web_socket/src/ws_client.cpp:693
- recv_thread_fn() closes the connection on any opcode it doesn’t explicitly handle. That includes WsOpcode::Continuation (0x0), so fragmented messages (FIN=0 followed by continuation frames) will cause a disconnect even though fragmentation is valid per RFC 6455.
default:
WS_ERR(*impl, "unknown opcode 0x%x — closing\n",
static_cast<uint8_t>(frame.opcode));
source/utils/web_socket/src/ws_client.cpp:262
- When sending over TLS, impl_send_all() logs errno/strerror on failure, but SSL_write failures are not reported via errno. This makes TLS send failures hard to diagnose; use SSL_get_error() / ERR_get_error() for the TLS path.
if (n <= 0) {
WS_ERR(impl, "send failed n:%d errno:%d(%s)\n",
n, errno, strerror(errno));
return false;
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.