Skip to content

Commit 6676705

Browse files
Copilotnetmindz
andcommitted
Address final review feedback: revert unnecessary files, use F-strings, add length validation, implement deferred ESP8266 validation
Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com>
1 parent 71ad0d9 commit 6676705

File tree

5 files changed

+113
-8
lines changed

5 files changed

+113
-8
lines changed

pio-scripts/build_ui.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,23 @@
1313
print('\x1b[6;33;42m' + 'Installing node packages' + '\x1b[0m')
1414
env.Execute("npm ci")
1515

16-
print('Building web UI')
16+
# Extract the release name from build flags
17+
release_name = None # Let cdata.js provide the default if not found
18+
build_flags = env.get("BUILD_FLAGS", [])
19+
for flag in build_flags:
20+
if 'WLED_RELEASE_NAME=' in flag:
21+
# Extract the release name, remove quotes and handle different formats
22+
parts = flag.split('WLED_RELEASE_NAME=')
23+
if len(parts) > 1:
24+
release_name = parts[1].split()[0].strip('\"\\')
25+
break
26+
27+
# Set environment variable for cdata.js to use (only if found)
28+
if release_name:
29+
os.environ['WLED_RELEASE_NAME'] = release_name
30+
print(f'Building web UI with release name: {release_name}')
31+
else:
32+
print('Building web UI with default release name (from cdata.js)')
1733

1834
# Call the bundling script
1935
exitCode = env.Execute("npm run build")

tools/cdata.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ function adoptVersionAndRepo(html) {
9696
html = html.replaceAll("##VERSION##", version);
9797
}
9898

99+
// Replace ##RELEASE## with the actual release name from build environment
100+
const releaseName = process.env.WLED_RELEASE_NAME || 'Custom';
101+
html = html.replaceAll("##RELEASE##", releaseName);
102+
99103
return html;
100104
}
101105

wled00/ota_release_check.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessa
9191
if (!hasCustomDesc) {
9292
// No custom description - this could be a legacy binary
9393
if (errorMessage && errorMessageLen > 0) {
94-
const char* msg = "This firmware file is missing compatibility metadata. Enable 'Ignore firmware validation' to proceed anyway.";
95-
strncpy(errorMessage, msg, errorMessageLen - 1);
94+
strncpy_P(errorMessage, PSTR("This firmware file is missing compatibility metadata. Enable 'Ignore firmware validation' to proceed anyway."), errorMessageLen - 1);
9695
errorMessage[errorMessageLen - 1] = '\0';
9796
}
9897
return false;
@@ -101,7 +100,7 @@ bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessa
101100
// Validate compatibility using extracted release name
102101
if (!validateReleaseCompatibility(extractedDesc.release_name)) {
103102
if (errorMessage && errorMessageLen > 0) {
104-
snprintf(errorMessage, errorMessageLen, "Firmware compatibility mismatch: current='%s', uploaded='%s'. Enable 'Ignore firmware validation' to proceed anyway.",
103+
snprintf_P(errorMessage, errorMessageLen, PSTR("Firmware compatibility mismatch: current='%s', uploaded='%s'. Enable 'Ignore firmware validation' to proceed anyway."),
105104
releaseString, extractedDesc.release_name);
106105
errorMessage[errorMessageLen - 1] = '\0'; // Ensure null termination
107106
}

wled00/wled_custom_desc.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ const wled_custom_desc_t __attribute__((section(WLED_CUSTOM_DESC_SECTION))) wled
1616
djb2_hash_constexpr(WLED_RELEASE_NAME) // crc32 - computed at compile time
1717
};
1818

19+
// Compile-time validation that release name doesn't exceed maximum length
20+
static_assert(sizeof(WLED_RELEASE_NAME) <= WLED_RELEASE_NAME_MAX_LEN,
21+
"WLED_RELEASE_NAME exceeds maximum length of WLED_RELEASE_NAME_MAX_LEN characters");
22+
1923
// Single reference to ensure it's not optimized away
2024
const wled_custom_desc_t* __attribute__((used)) wled_custom_desc_ref = &wled_custom_description;
2125

wled00/wled_server.cpp

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,23 +426,42 @@ void initServer()
426426
}
427427
if (!correctPIN || otaLock) return;
428428

429-
// Static variable to track release check status across chunks
429+
// Static variables to track release check status and data accumulation across chunks
430430
static bool releaseCheckPassed = false;
431+
static bool releaseCheckPending = true;
432+
static size_t totalBytesReceived = 0;
433+
static uint8_t* validationBuffer = nullptr;
434+
static const size_t ESP8266_VALIDATION_OFFSET = 0x1000; // 4KB offset for ESP8266
435+
static const size_t VALIDATION_BUFFER_SIZE = 8192; // Buffer size for validation
431436

432437
if(!index){
433438
DEBUG_PRINTLN(F("OTA Update Start"));
434439

440+
// Reset validation state for new update
441+
releaseCheckPassed = false;
442+
releaseCheckPending = true;
443+
totalBytesReceived = 0;
444+
445+
// Free any existing validation buffer
446+
if (validationBuffer) {
447+
free(validationBuffer);
448+
validationBuffer = nullptr;
449+
}
450+
435451
// Check if user wants to skip validation
436452
bool skipValidation = request->hasParam("skipValidation", true);
437453

438454
// If user chose to skip validation, proceed without compatibility check
439455
if (skipValidation) {
440456
DEBUG_PRINTLN(F("OTA validation skipped by user"));
441457
releaseCheckPassed = true;
458+
releaseCheckPending = false;
442459
} else {
443-
// Validate OTA release compatibility using the first chunk data directly
460+
#ifdef ESP32
461+
// ESP32: metadata appears at offset 0, validate immediately with first chunk
444462
char errorMessage[128];
445463
releaseCheckPassed = shouldAllowOTA(data, len, errorMessage, sizeof(errorMessage));
464+
releaseCheckPending = false;
446465

447466
if (!releaseCheckPassed) {
448467
DEBUG_PRINTF_P(PSTR("OTA declined: %s\n"), errorMessage);
@@ -451,6 +470,16 @@ void initServer()
451470
} else {
452471
DEBUG_PRINTLN(F("OTA allowed: Release compatibility check passed"));
453472
}
473+
#elif defined(ESP8266)
474+
// ESP8266: metadata appears around offset 0x1000, need to buffer data until then
475+
validationBuffer = (uint8_t*)malloc(VALIDATION_BUFFER_SIZE);
476+
if (!validationBuffer) {
477+
DEBUG_PRINTLN(F("OTA failed: Could not allocate validation buffer"));
478+
request->send(500, FPSTR(CONTENT_TYPE_PLAIN), F("Out of memory for validation"));
479+
return;
480+
}
481+
DEBUG_PRINTLN(F("ESP8266: Deferring validation until offset 0x1000"));
482+
#endif
454483
}
455484

456485
DEBUG_PRINTLN(F("Release check passed, starting OTA update"));
@@ -487,8 +516,46 @@ void initServer()
487516
}
488517
}
489518

490-
// Write chunk data to OTA update (only if release check passed)
491-
if (releaseCheckPassed && !Update.hasError()) {
519+
// Handle ESP8266 deferred validation - accumulate data until validation offset is reached
520+
#ifdef ESP8266
521+
if (releaseCheckPending && validationBuffer) {
522+
// Copy data to validation buffer
523+
size_t bytesToCopy = min((size_t)len, VALIDATION_BUFFER_SIZE - totalBytesReceived);
524+
if (bytesToCopy > 0) {
525+
memcpy(validationBuffer + totalBytesReceived, data, bytesToCopy);
526+
}
527+
totalBytesReceived += len;
528+
529+
// Check if we've reached the validation offset
530+
if (totalBytesReceived >= ESP8266_VALIDATION_OFFSET + sizeof(wled_custom_desc_t)) {
531+
DEBUG_PRINTLN(F("ESP8266: Performing deferred validation"));
532+
char errorMessage[128];
533+
534+
// Validate using buffered data starting from the expected offset
535+
releaseCheckPassed = shouldAllowOTA(validationBuffer + ESP8266_VALIDATION_OFFSET,
536+
totalBytesReceived - ESP8266_VALIDATION_OFFSET,
537+
errorMessage, sizeof(errorMessage));
538+
releaseCheckPending = false;
539+
540+
if (!releaseCheckPassed) {
541+
DEBUG_PRINTF_P(PSTR("OTA declined (deferred): %s\n"), errorMessage);
542+
free(validationBuffer);
543+
validationBuffer = nullptr;
544+
request->send(400, FPSTR(CONTENT_TYPE_PLAIN), errorMessage);
545+
return;
546+
} else {
547+
DEBUG_PRINTLN(F("OTA allowed: Deferred release compatibility check passed"));
548+
}
549+
550+
// Free validation buffer as it's no longer needed
551+
free(validationBuffer);
552+
validationBuffer = nullptr;
553+
}
554+
}
555+
#endif
556+
557+
// Write chunk data to OTA update (only if release check passed or still pending)
558+
if ((releaseCheckPassed || releaseCheckPending) && !Update.hasError()) {
492559
if (Update.write(data, len) != len) {
493560
DEBUG_PRINTF_P(PSTR("OTA write failed on chunk %zu: %s\n"), index, Update.getErrorString().c_str());
494561
}
@@ -497,6 +564,21 @@ void initServer()
497564
if(isFinal){
498565
DEBUG_PRINTLN(F("OTA Update End"));
499566

567+
// Clean up validation buffer if still allocated
568+
#ifdef ESP8266
569+
if (validationBuffer) {
570+
free(validationBuffer);
571+
validationBuffer = nullptr;
572+
}
573+
#endif
574+
575+
// Check if validation was still pending (shouldn't happen normally)
576+
if (releaseCheckPending) {
577+
DEBUG_PRINTLN(F("OTA failed: Validation never completed"));
578+
request->send(400, FPSTR(CONTENT_TYPE_PLAIN), F("Firmware validation incomplete"));
579+
return;
580+
}
581+
500582
if (releaseCheckPassed) {
501583
if(Update.end(true)){
502584
DEBUG_PRINTLN(F("Update Success"));

0 commit comments

Comments
 (0)