Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 25 additions & 1 deletion wled00/cfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,9 +636,32 @@ bool deserializeConfig(JsonObject doc, bool fromFS) {
return (doc["sv"] | true);
}


static const char s_cfg_json[] PROGMEM = "/cfg.json";

bool backupConfig() {
return backupFile(s_cfg_json);
}

bool restoreConfig() {
return restoreFile(s_cfg_json);
}

bool verifyConfig() {
return validateJsonFile(s_cfg_json);
}

// rename config file and reboot
// if the cfg file doesn't exist, such as after a reset, do nothing
void resetConfig() {
if (WLED_FS.exists(s_cfg_json)) {
DEBUG_PRINTLN(F("Reset config"));
char backupname[32];
snprintf_P(backupname, sizeof(backupname), PSTR("/rst.%s"), &s_cfg_json[1]);
WLED_FS.rename(s_cfg_json, backupname);
doReboot = true;
}
}

bool deserializeConfigFromFS() {
[[maybe_unused]] bool success = deserializeConfigSec();
#ifdef WLED_ADD_EEPROM_SUPPORT
Expand Down Expand Up @@ -676,6 +699,7 @@ bool deserializeConfigFromFS() {

void serializeConfig() {
serializeConfigSec();
backupConfig(); // backup before writing new config

DEBUG_PRINTLN(F("Writing settings to /cfg.json..."));

Expand Down
26 changes: 22 additions & 4 deletions wled00/fcn_declare.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ void handleIO();
void IRAM_ATTR touchButtonISR();

//cfg.cpp
bool backupConfig();
bool restoreConfig();
bool verifyConfig();
void resetConfig();
bool deserializeConfig(JsonObject doc, bool fromFS = false);
bool deserializeConfigFromFS();
bool deserializeConfigSec();
Expand Down Expand Up @@ -114,10 +118,15 @@ bool readObjectFromFileUsingId(const char* file, uint16_t id, JsonDocument* dest
bool readObjectFromFile(const char* file, const char* key, JsonDocument* dest);
void updateFSInfo();
void closeFile();
inline bool writeObjectToFileUsingId(const String &file, uint16_t id, JsonDocument* content) { return writeObjectToFileUsingId(file.c_str(), id, content); };
inline bool writeObjectToFile(const String &file, const char* key, JsonDocument* content) { return writeObjectToFile(file.c_str(), key, content); };
inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest) { return readObjectFromFileUsingId(file.c_str(), id, dest); };
inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest) { return readObjectFromFile(file.c_str(), key, dest); };
inline bool writeObjectToFileUsingId(const String &file, uint16_t id, const JsonDocument* content) { return writeObjectToFileUsingId(file.c_str(), id, content); };
inline bool writeObjectToFile(const String &file, const char* key, const JsonDocument* content) { return writeObjectToFile(file.c_str(), key, content); };
inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFileUsingId(file.c_str(), id, dest); };
inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFile(file.c_str(), key, dest); };
Comment on lines +123 to +124
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Filter parameter not forwarded to underlying functions.

The new filter parameter in the inline wrappers is declared but not passed through to the underlying function calls, silently discarding it.

Apply this diff to forward the filter parameter:

-inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFileUsingId(file.c_str(), id, dest); };
-inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFile(file.c_str(), key, dest); };
+inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFileUsingId(file.c_str(), id, dest, filter); };
+inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFile(file.c_str(), key, dest, filter); };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFileUsingId(file.c_str(), id, dest); };
inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) { return readObjectFromFile(file.c_str(), key, dest); };
inline bool readObjectFromFileUsingId(const String &file, uint16_t id, JsonDocument* dest, const JsonDocument* filter = nullptr) {
return readObjectFromFileUsingId(file.c_str(), id, dest, filter);
};
inline bool readObjectFromFile(const String &file, const char* key, JsonDocument* dest, const JsonDocument* filter = nullptr) {
return readObjectFromFile(file.c_str(), key, dest, filter);
};
🤖 Prompt for AI Agents
In wled00/fcn_declare.h around lines 123 to 124, the inline overloads declare a
filter parameter but do not pass it to the underlying function calls; update the
two inline functions to forward the filter argument to their respective
underlying functions (i.e., call readObjectFromFileUsingId(file.c_str(), id,
dest, filter) and readObjectFromFile(file.c_str(), key, dest, filter)) so the
filter is not dropped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is not a change introduced in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the actual implementation of readObjectFromFileUsingId and readObjectFromFile in file.cpp
echo "=== Searching for readObjectFromFileUsingId implementations ==="
ast-grep --pattern 'bool readObjectFromFileUsingId($$$) {
  $$$
}'

echo -e "\n=== Searching for readObjectFromFile implementations ==="
ast-grep --pattern 'bool readObjectFromFile($$$) {
  $$$
}'

echo -e "\n=== Using ripgrep to find all declarations/definitions with filter parameter ==="
rg -n "readObjectFromFile.*filter" --type=cpp -C 2

Length of output: 1145


🏁 Script executed:

#!/bin/bash
# Check if the underlying function declarations have been updated elsewhere in this PR
echo "=== Check all readObjectFromFile declarations in fcn_declare.h ==="
rg -n "bool readObjectFromFile" wled00/fcn_declare.h

echo -e "\n=== Check file.cpp for function signatures ==="
rg -n "^bool readObjectFromFile" wled00/file.cpp -A 1

echo -e "\n=== Search for any changes to these function signatures in git history ==="
git log --oneline --all -20 | head -20

echo -e "\n=== Check current branch ==="
git branch --show-current

Length of output: 1976


🏁 Script executed:

#!/bin/bash
# Check the main branch to see if filter parameter exists there
echo "=== Fetch main branch to compare ==="
git fetch origin main 2>&1 | head -5

echo -e "\n=== Check if filter parameter exists on main branch ==="
git show origin/main:wled00/fcn_declare.h | grep -n -A 2 -B 2 "readObjectFromFile.*filter" || echo "No filter parameter found on main branch"

echo -e "\n=== Check the 0_15_x branch ==="
git show origin/0_15_x:wled00/fcn_declare.h | grep -n -A 2 -B 2 "readObjectFromFile.*filter" || echo "No filter parameter found on 0_15_x branch"

echo -e "\n=== Check what the inline wrappers look like on 0_15_x before this PR ==="
git show origin/0_15_x:wled00/fcn_declare.h | grep -n -A 1 "inline bool readObjectFromFile"

Length of output: 1705


🏁 Script executed:

#!/bin/bash
# Check if the underlying function declarations in THIS branch have filter parameter
echo "=== Current branch underlying function declarations ==="
sed -n '114,118p' wled00/fcn_declare.h

echo -e "\n=== Main branch underlying function declarations ==="
git show origin/main:wled00/fcn_declare.h | sed -n '94,97p'

echo -e "\n=== Check file.cpp implementations on main ==="
git show origin/main:wled00/file.cpp | grep -n "^bool readObjectFromFile" -A 1

Length of output: 1203


@DedeHai, you're correct that the bug of not forwarding the filter parameter exists on the main branch as well. However, there's an important difference:

On main branch:

  • Underlying functions (readObjectFromFileUsingId and readObjectFromFile at lines 96-97) do support the filter parameter
  • The inline wrappers fail to forward it (a bug, but the code compiles)

On this branch (0_15_x_bootloopdetect):

  • Underlying functions (lines 117-118) do not support the filter parameter at all
  • The inline wrappers accept filter but cannot forward it because the underlying functions don't have that parameter

This appears to be an incomplete backport. The diff shows this PR adds the filter parameter to the inline wrappers, but the corresponding updates to the underlying function declarations and implementations in file.cpp weren't backported.

Options to resolve:

  1. Remove the filter parameter from the inline wrappers in this PR (revert to match the underlying functions)
  2. Complete the backport by also updating the underlying function declarations and implementations to support filtering

Which approach would you prefer for this backport?


🧬 Code Graph Analysis Results
  • File: wled00/fcn_declare.h, lines 9-16

    • Full snippet:
      void onAlexaChange(EspalexaDevice* dev);
  • File: wled00/fcn_declare.h, lines 17-24

    • Full snippet:
      void alexaInit();
      void handleAlexa();
      void onAlexaChange(EspalexaDevice* dev);
  • Note: There is a duplicate declaration of onAlexaChange(EspalexaDevice* dev) in this header (appearing twice) under the alexa.cpp section, which aligns with the user’s comment about a change not introduced in this PR.


🧠 Learnings used
Learnt from: DedeHai
Repo: wled/WLED PR: 4793
File: wled00/file.cpp:481-513
Timestamp: 2025-07-31T18:21:49.868Z
Learning: In WLED, essential configuration files that require backup have short, controlled names (like "/cfg.json", "/presets.json") that are well within a 32-character buffer limit. The file naming is controlled by developers, making buffer overflow in backup filename construction highly unlikely.

bool copyFile(const char* src_path, const char* dst_path);
bool backupFile(const char* filename);
bool restoreFile(const char* filename);
bool validateJsonFile(const char* filename);
void dumpFilesToSerial();

//hue.cpp
void handleHue();
Expand Down Expand Up @@ -399,6 +408,15 @@ void enumerateLedmaps();
uint8_t get_random_wheel_index(uint8_t pos);
float mapf(float x, float in_min, float in_max, float out_min, float out_max);

void handleBootLoop(); // detect and handle bootloops
#ifndef ESP8266
void bootloopCheckOTA(); // swap boot image if bootloop is detected instead of restoring config
#endif

void handleBootLoop(); // detect and handle bootloops
#ifndef ESP8266
void bootloopCheckOTA(); // swap boot image if bootloop is detected instead of restoring config
#endif
// RAII guard class for the JSON Buffer lock
// Modeled after std::lock_guard
class JSONBufferGuard {
Expand Down
153 changes: 153 additions & 0 deletions wled00/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -438,3 +438,156 @@ bool handleFileRead(AsyncWebServerRequest* request, String path){
}
return false;
}

// copy a file, delete destination file if incomplete to prevent corrupted files
bool copyFile(const char* src_path, const char* dst_path) {
DEBUG_PRINTF("copyFile from %s to %s\n", src_path, dst_path);
if(!WLED_FS.exists(src_path)) {
DEBUG_PRINTLN(F("file not found"));
return false;
}

bool success = true; // is set to false on error
File src = WLED_FS.open(src_path, "r");
File dst = WLED_FS.open(dst_path, "w");

if (src && dst) {
uint8_t buf[128]; // copy file in 128-byte blocks
while (src.available() > 0) {
size_t bytesRead = src.read(buf, sizeof(buf));
if (bytesRead == 0) {
success = false;
break; // error, no data read
}
size_t bytesWritten = dst.write(buf, bytesRead);
if (bytesWritten != bytesRead) {
success = false;
break; // error, not all data written
}
}
} else {
success = false; // error, could not open files
}
if(src) src.close();
if(dst) dst.close();
if (!success) {
DEBUG_PRINTLN(F("copy failed"));
WLED_FS.remove(dst_path); // delete incomplete file
}
return success;
}

// compare two files, return true if identical
bool compareFiles(const char* path1, const char* path2) {
DEBUG_PRINTF("compareFile %s and %s\n", path1, path2);
if (!WLED_FS.exists(path1) || !WLED_FS.exists(path2)) {
DEBUG_PRINTLN(F("file not found"));
return false;
}

bool identical = true; // set to false on mismatch
File f1 = WLED_FS.open(path1, "r");
File f2 = WLED_FS.open(path2, "r");

if (f1 && f2) {
uint8_t buf1[128], buf2[128];
while (f1.available() > 0 || f2.available() > 0) {
size_t len1 = f1.read(buf1, sizeof(buf1));
size_t len2 = f2.read(buf2, sizeof(buf2));

if (len1 != len2) {
identical = false;
break; // files differ in size or read failed
}

if (memcmp(buf1, buf2, len1) != 0) {
identical = false;
break; // files differ in content
}
}
} else {
identical = false; // error opening files
}

if (f1) f1.close();
if (f2) f2.close();
return identical;
}

static const char s_backup_fmt[] PROGMEM = "/bkp.%s";

bool backupFile(const char* filename) {
DEBUG_PRINTF("backup %s \n", filename);
if (!validateJsonFile(filename)) {
DEBUG_PRINTLN(F("broken file"));
return false;
}
char backupname[32];
snprintf_P(backupname, sizeof(backupname), s_backup_fmt, filename + 1); // skip leading '/' in filename

if (copyFile(filename, backupname)) {
DEBUG_PRINTLN(F("backup ok"));
return true;
}
DEBUG_PRINTLN(F("backup failed"));
return false;
}

bool restoreFile(const char* filename) {
DEBUG_PRINTF("restore %s \n", filename);
char backupname[32];
snprintf_P(backupname, sizeof(backupname), s_backup_fmt, filename + 1); // skip leading '/' in filename

if (!WLED_FS.exists(backupname)) {
DEBUG_PRINTLN(F("no backup found"));
return false;
}

if (!validateJsonFile(backupname)) {
DEBUG_PRINTLN(F("broken backup"));
return false;
}

if (copyFile(backupname, filename)) {
DEBUG_PRINTLN(F("restore ok"));
return true;
}
DEBUG_PRINTLN(F("restore failed"));
return false;
}

bool validateJsonFile(const char* filename) {
if (!WLED_FS.exists(filename)) return false;
File file = WLED_FS.open(filename, "r");
if (!file) return false;
StaticJsonDocument<0> doc, filter; // https://arduinojson.org/v6/how-to/validate-json/
bool result = deserializeJson(doc, file, DeserializationOption::Filter(filter)) == DeserializationError::Ok;
file.close();
if (!result) {
DEBUG_PRINTF_P(PSTR("Invalid JSON file %s\n"), filename);
} else {
DEBUG_PRINTF_P(PSTR("Valid JSON file %s\n"), filename);
}
return result;
}

// print contents of all files in root dir to Serial except wsec files
void dumpFilesToSerial() {
File rootdir = WLED_FS.open("/", "r");
File rootfile = rootdir.openNextFile();
while (rootfile) {
size_t len = strlen(rootfile.name());
// skip files starting with "wsec" and dont end in .json
if (strncmp(rootfile.name(), "wsec", 4) != 0 && len >= 6 && strcmp(rootfile.name() + len - 5, ".json") == 0) {
Serial.println(rootfile.name());
while (rootfile.available()) {
Serial.write(rootfile.read());
}
Serial.println();
Serial.println();
}
rootfile.close();
rootfile = rootdir.openNextFile();
}
}

4 changes: 4 additions & 0 deletions wled00/ota_update.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ static void endOTA(AsyncWebServerRequest *request) {
// If the upload is incomplete, Update.end(false) should error out.
if (Update.end(context->uploadComplete)) {
// Update successful!
#ifndef ESP8266
bootloopCheckOTA(); // let the bootloop-checker know there was an OTA update
#endif
doReboot = true;
context->needsRestart = false;
}
Expand Down Expand Up @@ -109,6 +112,7 @@ static bool beginOTA(AsyncWebServerRequest *request, UpdateContext* context)
strip.suspend();
strip.resetSegments(); // free as much memory as you can
context->needsRestart = true;
backupConfig(); // backup current config in case the update ends badly

DEBUG_PRINTF_P(PSTR("OTA Update Start, %x --> %x\n"), (uintptr_t)request,(uintptr_t) context);

Expand Down
Loading