Skip to content

fix: escape error JSON responses, check write/OTA return values (#77, #78) #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sjordan0228 merged 2 commits into from
Apr 2, 2026
Merged

fix: escape error JSON responses, check write/OTA return values (#77, #78) #89

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8280268
fix: escape error JSON responses, check write/OTA return values (#77,…
sjordan0228 Apr 2, 2026
c703dc6
fix: escape newlines and control chars in JSON error responses (#77)
sjordan0228 Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions src/WebServerManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,12 @@ void WebServerManager::handleApiUpdateFromUrl() {
_otaError[0] = '\0';

Serial.printf("OTA: Free heap before task: %u\n", ESP.getFreeHeap());
xTaskCreatePinnedToCore(otaDownloadTask, "OTATask", 24576, this, 2, nullptr, 0);
BaseType_t created = xTaskCreatePinnedToCore(otaDownloadTask, "OTATask", 24576, this, 2, nullptr, 0);
if (created != pdPASS) {
_otaState = OtaState::IDLE;
sendError(500, "Failed to start OTA task");
return;
}

_server.send(200, "application/json", "{\"success\":true,\"status\":\"started\"}");
}
Expand Down Expand Up @@ -1312,7 +1317,10 @@ void WebServerManager::handleApiFormatTag() {
req.request_id = millis();
req.type = NFCWriteType::FORMAT_NEW;
strncpy(req.expected_spool_id, uid, sizeof(req.expected_spool_id) - 1);
NFCManager::getInstance().enqueueWrite(req);
if (!NFCManager::getInstance().enqueueWrite(req)) {
sendError(503, "Write queue full");
return;
}

_server.send(200, "application/json", "{\"success\":true}");
}
Expand Down Expand Up @@ -1402,7 +1410,10 @@ void WebServerManager::handleApiWriteTigerTag() {
req.type = NFCWriteType::WRITE_TIGERTAG;
strncpy(req.expected_spool_id, uid, sizeof(req.expected_spool_id) - 1);
memcpy(req.data.tigertag_data, payload, 40);
NFCManager::getInstance().enqueueWrite(req);
if (!NFCManager::getInstance().enqueueWrite(req)) {
sendError(503, "Write queue full");
return;
}

_server.send(200, "application/json", "{\"success\":true}");
}
Expand Down Expand Up @@ -1512,8 +1523,20 @@ void WebServerManager::handleApiWriteOpenTag3D() {

void WebServerManager::sendError(int code, const char* msg) {
_server.sendHeader("Access-Control-Allow-Origin", "*");
char body[96];
snprintf(body, sizeof(body), "{\"success\":false,\"error\":\"%s\"}", msg);
// Escape quotes and backslashes in msg to produce valid JSON
char escaped[128];
size_t j = 0;
for (size_t i = 0; msg[i] && j < sizeof(escaped) - 2; i++) {
if (msg[i] == '"' || msg[i] == '\\') {
escaped[j++] = '\\';
}
if (j < sizeof(escaped) - 1) {
escaped[j++] = msg[i];
}
}
escaped[j] = '\0';
char body[192];
snprintf(body, sizeof(body), "{\"success\":false,\"error\":\"%s\"}", escaped);

@coderabbitai coderabbitai Bot Apr 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Incomplete escaping: newlines and control characters are not handled.

Issue #77 explicitly mentions escaping "quotes, backslashes, newlines" but this implementation only handles quotes and backslashes. If an upstream error contains \n, \r, \t, or other control characters, the JSON will still be malformed.

🛠️ Proposed fix to escape control characters 
 void WebServerManager::sendError(int code, const char* msg) {
     _server.sendHeader("Access-Control-Allow-Origin", "*");
     // Escape quotes and backslashes in msg to produce valid JSON
-    char escaped[128];
+    char escaped[256];
     size_t j = 0;
-    for (size_t i = 0; msg[i] && j < sizeof(escaped) - 2; i++) {
-        if (msg[i] == '"' || msg[i] == '\\') {
-            escaped[j++] = '\\';
-        }
-        if (j < sizeof(escaped) - 1) {
-            escaped[j++] = msg[i];
+    for (size_t i = 0; msg[i] && j < sizeof(escaped) - 6; i++) {
+        char c = msg[i];
+        if (c == '"' || c == '\\') {
+            escaped[j++] = '\\';
+            escaped[j++] = c;
+        } else if (c == '\n') {
+            escaped[j++] = '\\';
+            escaped[j++] = 'n';
+        } else if (c == '\r') {
+            escaped[j++] = '\\';
+            escaped[j++] = 'r';
+        } else if (c == '\t') {
+            escaped[j++] = '\\';
+            escaped[j++] = 't';
+        } else if ((unsigned char)c < 0x20) {
+            // Other control chars: encode as \uXXXX
+            j += snprintf(escaped + j, sizeof(escaped) - j, "\\u%04X", (unsigned char)c);
+        } else {
+            escaped[j++] = c;
         }
     }
     escaped[j] = '\0';
-    char body[192];
+    char body[320];
     snprintf(body, sizeof(body), "{\"success\":false,\"error\":\"%s\"}", escaped);
     _server.send(code, "application/json", body);
 }

Alternatively, consider using ArduinoJson (already included) to build the error response safely:

void WebServerManager::sendError(int code, const char* msg) {
    _server.sendHeader("Access-Control-Allow-Origin", "*");
    StaticJsonDocument<256> doc;
    doc["success"] = false;
    doc["error"] = msg;
    String body;
    serializeJson(doc, body);
    _server.send(code, "application/json", body);
}

This leverages ArduinoJson's built-in escaping and is consistent with other response handlers in this file.

🧰 Tools
🪛 Clang (14.0.6)

[warning] 1527-1527: do not declare C-style arrays, use std::array<> instead

(modernize-avoid-c-arrays)

[warning] 1528-1528: variable 'j' is not initialized

(cppcoreguidelines-init-variables)

[warning] 1528-1528: variable name 'j' is too short, expected at least 3 characters

(readability-identifier-length)

[warning] 1538-1538: do not declare C-style arrays, use std::array<> instead

(modernize-avoid-c-arrays)

[warning] 1539-1539: escaped string literal can be written as a raw string literal

(modernize-raw-string-literal)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/WebServerManager.cpp` around lines 1526 - 1539, The current manual
escaping in WebServerManager::sendError (variables msg, escaped, body) only
handles quotes and backslashes and fails for newlines/control characters;
replace the manual escape+snprintf logic with ArduinoJson: create a
StaticJsonDocument (e.g., 256), set doc["success"]=false and doc["error"]=msg,
serializeJson to a String, call
_server.sendHeader("Access-Control-Allow-Origin","*") and then _server.send with
"application/json" and the serialized body; remove the escaped/body buffers and
loop entirely so ArduinoJson handles all necessary escaping.

_server.send(code, "application/json", body);
}

Expand Down