From 374a971743c8ea8be1278340c34f2e87f8a4a945 Mon Sep 17 00:00:00 2001 From: Hansel Ip Date: Fri, 23 Jan 2026 12:54:40 -0800 Subject: [PATCH 1/4] Fix crash when HttpResponseDecoder receives non-JSON response Add validation before JSON parsing in processBody to prevent crashes when receiving non-JSON HTTP responses (e.g., HTML from captive portals). Uses nlohmann::json::accept() to validate response body before parsing. Fixes #1338 Co-Authored-By: Claude Sonnet 4.5 --- lib/http/HttpResponseDecoder.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/http/HttpResponseDecoder.cpp b/lib/http/HttpResponseDecoder.cpp index 9e3fd786a..72184c7db 100644 --- a/lib/http/HttpResponseDecoder.cpp +++ b/lib/http/HttpResponseDecoder.cpp @@ -173,6 +173,14 @@ namespace MAT_NS_BEGIN { try { std::string body(response.GetBody().begin(), response.GetBody().end()); + + // Validate that the body is valid JSON before attempting to parse + if (body.empty() || !nlohmann::json::accept(body)) + { + LOG_ERROR("HTTP response: body is not valid JSON, skipping processing"); + return; + } + responseBody = nlohmann::json::parse(body.c_str()); int accepted = 0; auto acc = responseBody.find("acc"); From ca99f1ced2e43f3ead9a876eb9ad1bbf7c748ee0 Mon Sep 17 00:00:00 2001 From: Hansel Ip Date: Wed, 28 Jan 2026 12:02:31 -0800 Subject: [PATCH 2/4] Address PR #1405 review: use parse with exceptions disabled Replace double-parsing approach (accept() then parse()) with single parse() call using allow_exceptions=false and is_discarded() check. This eliminates redundant parsing and removes the need for try/catch. Addresses review comments from lalitb and ThomsonTan. Co-Authored-By: Claude Sonnet 4.5 --- lib/http/HttpResponseDecoder.cpp | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/http/HttpResponseDecoder.cpp b/lib/http/HttpResponseDecoder.cpp index 72184c7db..c414351ba 100644 --- a/lib/http/HttpResponseDecoder.cpp +++ b/lib/http/HttpResponseDecoder.cpp @@ -169,19 +169,26 @@ namespace MAT_NS_BEGIN { { #ifdef HAVE_MAT_JSONHPP // TODO: [MG] - parse HTTP response without json.hpp library - nlohmann::json responseBody; - try + std::string body(response.GetBody().begin(), response.GetBody().end()); + + // Handle empty body + if (body.empty()) { - std::string body(response.GetBody().begin(), response.GetBody().end()); + LOG_ERROR("HTTP response: body is empty, skipping processing"); + return; + } - // Validate that the body is valid JSON before attempting to parse - if (body.empty() || !nlohmann::json::accept(body)) - { - LOG_ERROR("HTTP response: body is not valid JSON, skipping processing"); - return; - } + // Parse JSON with exceptions disabled + nlohmann::json responseBody = nlohmann::json::parse(body, nullptr, false); + + // Check if parsing failed (returns discarded value for invalid JSON) + if (responseBody.is_discarded()) + { + LOG_ERROR("HTTP response: body is not valid JSON, skipping processing"); + return; + } - responseBody = nlohmann::json::parse(body.c_str()); + { int accepted = 0; auto acc = responseBody.find("acc"); if (responseBody.end() != acc) @@ -240,10 +247,6 @@ namespace MAT_NS_BEGIN { LOG_TRACE("HTTP response: all rejected"); } } - catch (...) - { - LOG_ERROR("HTTP response: JSON parsing failed"); - } #else UNREFERENCED_PARAMETER(response); UNREFERENCED_PARAMETER(result); From f88a6415bd063572ed2264daad81cd913321b1f8 Mon Sep 17 00:00:00 2001 From: Hansel Ip Date: Wed, 28 Jan 2026 14:35:11 -0800 Subject: [PATCH 3/4] Use iterators directly in parse() to avoid copying body Pass body iterators directly to nlohmann::json::parse() instead of copying into std::string first. This improves performance by avoiding unnecessary memory allocation and copy. Co-Authored-By: Claude Sonnet 4.5 --- lib/http/HttpResponseDecoder.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/http/HttpResponseDecoder.cpp b/lib/http/HttpResponseDecoder.cpp index c414351ba..7699a32ae 100644 --- a/lib/http/HttpResponseDecoder.cpp +++ b/lib/http/HttpResponseDecoder.cpp @@ -169,17 +169,18 @@ namespace MAT_NS_BEGIN { { #ifdef HAVE_MAT_JSONHPP // TODO: [MG] - parse HTTP response without json.hpp library - std::string body(response.GetBody().begin(), response.GetBody().end()); + auto bodyBegin = response.GetBody().begin(); + auto bodyEnd = response.GetBody().end(); // Handle empty body - if (body.empty()) + if (bodyBegin == bodyEnd) { LOG_ERROR("HTTP response: body is empty, skipping processing"); return; } - // Parse JSON with exceptions disabled - nlohmann::json responseBody = nlohmann::json::parse(body, nullptr, false); + // Parse JSON with exceptions disabled (pass iterators directly to avoid copy) + nlohmann::json responseBody = nlohmann::json::parse(bodyBegin, bodyEnd, nullptr, false); // Check if parsing failed (returns discarded value for invalid JSON) if (responseBody.is_discarded()) From 54de0177eec12df05b19632427b3f8fe78f86c5d Mon Sep 17 00:00:00 2001 From: Hansel Ip Date: Fri, 30 Jan 2026 14:52:17 -0800 Subject: [PATCH 4/4] Remove unnecessary inner scope block Remove the leftover scope braces from the try/catch removal and fix indentation for cleaner code structure. Co-Authored-By: Claude Sonnet 4.5 --- lib/http/HttpResponseDecoder.cpp | 84 ++++++++++++++++---------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/lib/http/HttpResponseDecoder.cpp b/lib/http/HttpResponseDecoder.cpp index 7699a32ae..11e9d4096 100644 --- a/lib/http/HttpResponseDecoder.cpp +++ b/lib/http/HttpResponseDecoder.cpp @@ -189,64 +189,62 @@ namespace MAT_NS_BEGIN { return; } + int accepted = 0; + auto acc = responseBody.find("acc"); + if (responseBody.end() != acc) { - int accepted = 0; - auto acc = responseBody.find("acc"); - if (responseBody.end() != acc) + if (acc.value().is_number()) { - if (acc.value().is_number()) - { - accepted = acc.value().get(); - } + accepted = acc.value().get(); } + } - int rejected = 0; - auto rej = responseBody.find("rej"); - if (responseBody.end() != rej) + int rejected = 0; + auto rej = responseBody.find("rej"); + if (responseBody.end() != rej) + { + if (rej.value().is_number()) { - if (rej.value().is_number()) - { - rejected = rej.value().get(); - } + rejected = rej.value().get(); } + } - auto efi = responseBody.find("efi"); - if (responseBody.end() != efi) + auto efi = responseBody.find("efi"); + if (responseBody.end() != efi) + { + for (auto it = responseBody["efi"].begin(); it != responseBody["efi"].end(); ++it) { - for (auto it = responseBody["efi"].begin(); it != responseBody["efi"].end(); ++it) + std::string efiKey(it.key()); + nlohmann::json val = it.value(); + if (val.is_array()) { - std::string efiKey(it.key()); - nlohmann::json val = it.value(); - if (val.is_array()) - { - //std::vector failureVector = val.get>(); - // eventsRejected(ctx); with only the ids in the vector above - } - if (val.is_string()) + //std::vector failureVector = val.get>(); + // eventsRejected(ctx); with only the ids in the vector above + } + if (val.is_string()) + { + if ("all" == val.get()) { - if ("all" == val.get()) - { - result = Rejected; - } + result = Rejected; } } } + } - auto ticket = responseBody.find("TokenCrackingFailure"); - if (responseBody.end() != ticket) - { - DebugEvent evt; - evt.type = DebugEventType::EVT_TICKET_EXPIRED; - DispatchEvent(evt); - } + auto ticket = responseBody.find("TokenCrackingFailure"); + if (responseBody.end() != ticket) + { + DebugEvent evt; + evt.type = DebugEventType::EVT_TICKET_EXPIRED; + DispatchEvent(evt); + } - if (result != Rejected) - { - LOG_TRACE("HTTP response: accepted=%d rejected=%d", accepted, rejected); - } else - { - LOG_TRACE("HTTP response: all rejected"); - } + if (result != Rejected) + { + LOG_TRACE("HTTP response: accepted=%d rejected=%d", accepted, rejected); + } else + { + LOG_TRACE("HTTP response: all rejected"); } #else UNREFERENCED_PARAMETER(response);