From 9d0d1f221df6537df86dce15f56eb5b60e613252 Mon Sep 17 00:00:00 2001 From: Bryan Biedenkapp Date: Fri, 27 Mar 2026 08:52:00 -0400 Subject: [PATCH] fix issue with HTTP ClientConnection and SecureClientConnection not properly handling responses >65K bytes; add missing backward compat flags (for now, after R05A06 these will be removed); update copyright headers properly in modified files; --- src/bridge/network/PeerNetwork.cpp | 2 +- src/common/restapi/http/ClientConnection.h | 51 ++++++++++-------- .../restapi/http/SecureClientConnection.h | 54 +++++++++++-------- src/fne/network/PeerNetwork.cpp | 14 ++++- src/patch/network/PeerNetwork.cpp | 2 +- src/remote/RESTClient.cpp | 4 +- src/sysview/network/PeerNetwork.cpp | 19 ++++++- 7 files changed, 95 insertions(+), 51 deletions(-) diff --git a/src/bridge/network/PeerNetwork.cpp b/src/bridge/network/PeerNetwork.cpp index 9d31b36f..8332483f 100644 --- a/src/bridge/network/PeerNetwork.cpp +++ b/src/bridge/network/PeerNetwork.cpp @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2024 Bryan Biedenkapp, N2PLL + * Copyright (C) 2024,2026 Bryan Biedenkapp, N2PLL * */ #include "bridge/Defines.h" diff --git a/src/common/restapi/http/ClientConnection.h b/src/common/restapi/http/ClientConnection.h index 0b370cdf..9f472a7c 100644 --- a/src/common/restapi/http/ClientConnection.h +++ b/src/common/restapi/http/ClientConnection.h @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2023,2024 Bryan Biedenkapp, N2PLL + * Copyright (C) 2023,2024,2026 Bryan Biedenkapp, N2PLL * */ /** @@ -20,6 +20,7 @@ #include "common/Log.h" #include +#include #include #include #include @@ -54,6 +55,9 @@ namespace restapi explicit ClientConnection(asio::ip::tcp::socket socket, RequestHandlerType& handler) : m_socket(std::move(socket)), m_requestHandler(handler), + m_sizeToTransfer(0U), + m_bytesTransferred(0U), + m_fullBuffer(65536), m_lexer(HTTPLexer(true)) { /* stub */ @@ -121,39 +125,42 @@ namespace restapi try { - if (m_sizeToTransfer > 0U && (m_bytesTransferred + bytes_transferred) < m_sizeToTransfer) { - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; + // grow accumulation buffer as needed + if (m_bytesTransferred + bytes_transferred > m_fullBuffer.size()) { + m_fullBuffer.resize(m_bytesTransferred + bytes_transferred); + } + + // always accumulate into m_fullBuffer + ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); + m_bytesTransferred += bytes_transferred; + if (m_sizeToTransfer > 0U && m_bytesTransferred < m_sizeToTransfer) { + // still waiting for more data read(); } else { - if (m_sizeToTransfer > 0U) { - // final copy - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; - - m_sizeToTransfer = 0U; - bytes_transferred = m_bytesTransferred; + bool wasMultiRead = (m_sizeToTransfer > 0U); + m_sizeToTransfer = 0U; - // reset lexer and re-parse the full content + if (wasMultiRead) { + // re-parse the full reassembled response from the beginning m_lexer.reset(); - std::tie(result, content) = m_lexer.parse(m_request, m_fullBuffer.data(), m_fullBuffer.data() + bytes_transferred); - } else { - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; - - std::tie(result, content) = m_lexer.parse(m_request, m_buffer.data(), m_buffer.data() + bytes_transferred); } + // always parse from m_fullBuffer so content pointer is always valid + std::tie(result, content) = m_lexer.parse(m_request, m_fullBuffer.data(), m_fullBuffer.data() + m_bytesTransferred); + // determine content length std::string contentLength = m_request.headers.find("Content-Length"); if (contentLength != "") { size_t length = (size_t)::strtoul(contentLength.c_str(), NULL, 10); - // setup a full read if necessary - if (length > bytes_transferred && m_sizeToTransfer == 0U) { - m_sizeToTransfer = length; + // body bytes available: distance from parsed body start to end of accumulated data + // content and m_fullBuffer.data() are in the same allocation, so this arithmetic is valid + size_t bodyBytesAvailable = (size_t)((m_fullBuffer.data() + m_bytesTransferred) - content); + if (length > bodyBytesAvailable) { + // total bytes we need = current accumulated + remaining body bytes + m_sizeToTransfer = m_bytesTransferred + (length - bodyBytesAvailable); } if (m_sizeToTransfer > 0U) { @@ -225,7 +232,7 @@ namespace restapi std::size_t m_sizeToTransfer; std::size_t m_bytesTransferred; - std::array m_fullBuffer; + std::vector m_fullBuffer; std::array m_buffer; diff --git a/src/common/restapi/http/SecureClientConnection.h b/src/common/restapi/http/SecureClientConnection.h index 16d24341..17067d9b 100644 --- a/src/common/restapi/http/SecureClientConnection.h +++ b/src/common/restapi/http/SecureClientConnection.h @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2024 Bryan Biedenkapp, N2PLL + * Copyright (C) 2024,2026 Bryan Biedenkapp, N2PLL * */ /** @@ -22,6 +22,7 @@ #include "common/Log.h" #include +#include #include #include #include @@ -58,6 +59,9 @@ namespace restapi explicit SecureClientConnection(asio::ip::tcp::socket socket, asio::ssl::context& context, RequestHandlerType& handler) : m_socket(std::move(socket), context), m_requestHandler(handler), + m_sizeToTransfer(0U), + m_bytesTransferred(0U), + m_fullBuffer(65536), m_lexer(HTTPLexer(true)) { m_socket.set_verify_mode(asio::ssl::verify_none); @@ -69,7 +73,7 @@ namespace restapi */ void start() { - m_socket.handshake(asio::ssl::stream_base::client); + m_socket.handshake(asio::ssl::stream_base::client); read(); } /** @@ -140,39 +144,42 @@ namespace restapi try { - if (m_sizeToTransfer > 0U && (m_bytesTransferred + bytes_transferred) < m_sizeToTransfer) { - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; + // grow accumulation buffer as needed + if (m_bytesTransferred + bytes_transferred > m_fullBuffer.size()) { + m_fullBuffer.resize(m_bytesTransferred + bytes_transferred); + } + + // always accumulate into m_fullBuffer + ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); + m_bytesTransferred += bytes_transferred; + if (m_sizeToTransfer > 0U && m_bytesTransferred < m_sizeToTransfer) { + // still waiting for more data read(); } else { - if (m_sizeToTransfer > 0U) { - // final copy - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; - - m_sizeToTransfer = 0U; - bytes_transferred = m_bytesTransferred; + bool wasMultiRead = (m_sizeToTransfer > 0U); + m_sizeToTransfer = 0U; - // reset lexer and re-parse the full content + if (wasMultiRead) { + // re-parse the full reassembled response from the beginning m_lexer.reset(); - std::tie(result, content) = m_lexer.parse(m_request, m_fullBuffer.data(), m_fullBuffer.data() + bytes_transferred); - } else { - ::memcpy(m_fullBuffer.data() + m_bytesTransferred, m_buffer.data(), bytes_transferred); - m_bytesTransferred += bytes_transferred; - - std::tie(result, content) = m_lexer.parse(m_request, m_buffer.data(), m_buffer.data() + bytes_transferred); } + // always parse from m_fullBuffer so content pointer is always valid + std::tie(result, content) = m_lexer.parse(m_request, m_fullBuffer.data(), m_fullBuffer.data() + m_bytesTransferred); + // determine content length std::string contentLength = m_request.headers.find("Content-Length"); if (contentLength != "") { size_t length = (size_t)::strtoul(contentLength.c_str(), NULL, 10); - // setup a full read if necessary - if (length > bytes_transferred && m_sizeToTransfer == 0U) { - m_sizeToTransfer = length; + // body bytes available: distance from parsed body start to end of accumulated data + // content and m_fullBuffer.data() are in the same allocation, so this arithmetic is valid + size_t bodyBytesAvailable = (size_t)((m_fullBuffer.data() + m_bytesTransferred) - content); + if (length > bodyBytesAvailable) { + // total bytes we need = current accumulated + remaining body bytes + m_sizeToTransfer = m_bytesTransferred + (length - bodyBytesAvailable); } if (m_sizeToTransfer > 0U) { @@ -183,6 +190,7 @@ namespace restapi } m_request.headers.add("RemoteHost", m_socket.lowest_layer().remote_endpoint().address().to_string()); + if (result == HTTPLexer::GOOD) { m_sizeToTransfer = m_bytesTransferred = 0U; m_requestHandler.handleRequest(m_request, m_reply); @@ -245,7 +253,7 @@ namespace restapi std::size_t m_sizeToTransfer; std::size_t m_bytesTransferred; - std::array m_fullBuffer; + std::vector m_fullBuffer; std::array m_buffer; diff --git a/src/fne/network/PeerNetwork.cpp b/src/fne/network/PeerNetwork.cpp index 710c528f..58984a57 100644 --- a/src/fne/network/PeerNetwork.cpp +++ b/src/fne/network/PeerNetwork.cpp @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2024-2025 Bryan Biedenkapp, N2PLL + * Copyright (C) 2024-2026 Bryan Biedenkapp, N2PLL * */ #include "fne/Defines.h" @@ -523,9 +523,19 @@ bool PeerNetwork::writeConfig() uint32_t peerClass = (uint32_t)PEER_CONN_CLASS::PEER_CONN_CLASS_NEIGHBOR; config["peerClass"].set(peerClass); // Peer Connection Class - config["masterPeerId"].set(m_masterPeerId); // Master Peer ID + /* + ** bryanb: this is deprecated -- it remains here for backwards compatibility with older master versions, + ** but is no longer used by the master and have no effect on R05A06 systems, and may be removed in a future release + ** { + */ + bool external = true; + config["externalPeer"].set(external); // External Peer Marker + /* + ** } + */ + config["software"].set(std::string(software)); // Software ID json::value v = json::value(config); diff --git a/src/patch/network/PeerNetwork.cpp b/src/patch/network/PeerNetwork.cpp index 03f2d912..37f32d9c 100644 --- a/src/patch/network/PeerNetwork.cpp +++ b/src/patch/network/PeerNetwork.cpp @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2025 Bryan Biedenkapp, N2PLL + * Copyright (C) 2025-2026 Bryan Biedenkapp, N2PLL * */ #include "patch/Defines.h" diff --git a/src/remote/RESTClient.cpp b/src/remote/RESTClient.cpp index 4ec31f0c..393fbbc5 100644 --- a/src/remote/RESTClient.cpp +++ b/src/remote/RESTClient.cpp @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2023,2024 Bryan Biedenkapp, N2PLL + * Copyright (C) 2023,2024,2026 Bryan Biedenkapp, N2PLL * */ #include "Defines.h" @@ -68,11 +68,13 @@ bool parseResponseBody(const HTTPPayload& response, json::object& obj) json::value v; std::string err = json::parse(v, response.content); if (!err.empty()) { + LogError(LOG_REST, "Failed to parse REST API response body: %s", err.c_str()); return false; } // ensure parsed JSON is an object if (!v.is()) { + LogError(LOG_REST, "Failed to parse REST API response body: expected JSON object"); return false; } diff --git a/src/sysview/network/PeerNetwork.cpp b/src/sysview/network/PeerNetwork.cpp index 1092bf5a..af9472ae 100644 --- a/src/sysview/network/PeerNetwork.cpp +++ b/src/sysview/network/PeerNetwork.cpp @@ -4,7 +4,7 @@ * GPLv2 Open Source. Use is subject to license terms. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * - * Copyright (C) 2024-2025 Bryan Biedenkapp, N2PLL + * Copyright (C) 2024-2026 Bryan Biedenkapp, N2PLL * */ #include "sysview/Defines.h" @@ -279,6 +279,23 @@ bool PeerNetwork::writeConfig() uint32_t peerClass = PEER_CONN_CLASS::PEER_CONN_CLASS_SYSVIEW; config["peerClass"].set(peerClass); // Peer Connection Class + config["masterPeerId"].set(m_peerId); // Master Peer ID + + bool convPeer = true; + config["conventionalPeer"].set(convPeer); // Conventional Peer Marker + + /* + ** bryanb: these are all deprecated -- they remain here for backwards compatibility with older master versions, + ** but they are no longer used by the master and have no effect on R05A06 systems, and may be removed in a future release + ** { + */ + bool external = true; + config["externalPeer"].set(external); // External Peer Marker + bool sysView = true; + config["sysView"].set(sysView); // SysView Peer Marker + /* + ** } + */ config["software"].set(std::string(software)); // Software ID