Address final review feedback: improve error handling, remove debug duplication, use HTTP API for release name

Co-authored-by: willmmiles <6540455+willmmiles@users.noreply.github.com>

Author: Copilot

pio-scripts/build_ui.py

@@ -13,23 +13,7 @@
     print('\x1b[6;33;42m' + 'Installing node packages' + '\x1b[0m')
     env.Execute("npm ci")
 
-    # Extract the release name from build flags
-    release_name = None  # Let cdata.js provide the default if not found
-    build_flags = env.get("BUILD_FLAGS", [])
-    for flag in build_flags:
-        if 'WLED_RELEASE_NAME=' in flag:
-            # Extract the release name, remove quotes and handle different formats
-            parts = flag.split('WLED_RELEASE_NAME=')
-            if len(parts) > 1:
-                release_name = parts[1].split()[0].strip('\"\\')
-                break
-    
-    # Set environment variable for cdata.js to use (only if found)
-    if release_name:
-        os.environ['WLED_RELEASE_NAME'] = release_name
-        print(f'Building web UI with release name: {release_name}')
-    else:
-        print('Building web UI with default release name (from cdata.js)')
+    print('Building web UI')
 
     # Call the bundling script
     exitCode = env.Execute("npm run build")

tools/cdata.js

@@ -96,10 +96,6 @@ function adoptVersionAndRepo(html) {
     html = html.replaceAll("##VERSION##", version);
   }
 
-  // Replace ##RELEASE## with the actual release name from build environment
-  const releaseName = process.env.WLED_RELEASE_NAME || 'Custom';
-  html = html.replaceAll("##RELEASE##", releaseName);
-  
   return html;
 }
 

wled00/data/update.htm

@@ -17,7 +17,27 @@
 			}
 			window.open(getURL("/update?revert"),"_self");
 		}
-		function GetV() {/*injected values here*/}
+		function GetV() {
+			// Fetch device info via JSON API instead of compiling it in
+			fetch('/json/info')
+				.then(response => response.json())
+				.then(data => {
+					if (data.release) {
+						var releaseSpan = document.querySelector('.release-name');
+						if (releaseSpan) {
+							releaseSpan.textContent = data.release;
+						}
+					}
+				})
+				.catch(error => {
+					console.log('Could not fetch device info:', error);
+					// Fallback to compiled-in value if API call fails
+					var releaseSpan = document.querySelector('.release-name');
+					if (releaseSpan && releaseSpan.textContent === 'Loading...') {
+						releaseSpan.textContent = 'Unknown';
+					}
+				});
+		}
 	</script>
 	<style>
 		@import url("style.css");
@@ -28,7 +48,7 @@
 	<h2>WLED Software Update</h2>
 	<form method='POST' action='./update' id='upd' enctype='multipart/form-data' onsubmit="toggle('upd')">
 		Installed version: <span class="sip">WLED ##VERSION##</span><br>
-		Release: <span class="sip">##RELEASE##</span><br>
+		Release: <span class="sip release-name">Loading...</span><br>
 		Download the latest binary: <a href="https://github.com/wled-dev/WLED/releases" target="_blank" 
 		style="vertical-align: text-bottom; display: inline-flex;">
 		<img src="https://img.shields.io/github/release/wled-dev/WLED.svg?style=flat-square"></a><br>

wled00/ota_release_check.cpp

@@ -13,10 +13,10 @@ bool extractWledCustomDesc(const uint8_t* binaryData, size_t dataSize, wled_cust
 
     // Search in first 8KB only. This range was chosen because:
     // - ESP32 .rodata.wled_desc sections appear early in the binary (typically within first 2-4KB)
-    // - ESP8266 .ver_number sections also appear early (typically within first 1-2KB)
+    // - ESP8266 .ver_number sections also appear early (typically within first 1-2KB)  
     // - 8KB provides ample coverage for metadata discovery while minimizing processing time
     // - Larger firmware files (>1MB) would take significantly longer to process with full search
-    // - Real-world testing shows all valid metadata appears well within this range
+    // - Analysis of typical WLED binary layouts shows metadata appears well within this range
     const size_t search_limit = min(dataSize, (size_t)8192);
 
     for (size_t offset = 0; offset <= search_limit - sizeof(wled_custom_desc_t); offset++) {
@@ -41,13 +41,8 @@ bool extractWledCustomDesc(const uint8_t* binaryData, size_t dataSize, wled_cust
             // Valid structure found - copy entire structure
             memcpy(extractedDesc, custom_desc, sizeof(wled_custom_desc_t));
 
-            #ifdef ESP32
-            DEBUG_PRINTF_P(PSTR("Extracted ESP32 WLED structure from .rodata.wled_desc section at offset %u: '%s'\n"), 
+            DEBUG_PRINTF_P(PSTR("Extracted WLED structure at offset %u: '%s'\n"), 
                           offset, extractedDesc->release_name);
-            #else
-            DEBUG_PRINTF_P(PSTR("Extracted ESP8266 WLED structure from .ver_number section at offset %u: '%s'\n"), 
-                          offset, extractedDesc->release_name);
-            #endif
             return true;
         }
     }
@@ -57,59 +52,61 @@ bool extractWledCustomDesc(const uint8_t* binaryData, size_t dataSize, wled_cust
 }
 
 bool validateReleaseCompatibility(const char* extractedRelease) {
-  if (!extractedRelease || strlen(extractedRelease) == 0) {
+  if (!extractedRelease) {
+    return false;
+  }
+  
+  // Ensure extractedRelease is properly null terminated (guard against fixed-length buffer issues)
+  char safeRelease[WLED_RELEASE_NAME_MAX_LEN];
+  strncpy(safeRelease, extractedRelease, WLED_RELEASE_NAME_MAX_LEN - 1);
+  safeRelease[WLED_RELEASE_NAME_MAX_LEN - 1] = '\0';
+  
+  if (strlen(safeRelease) == 0) {
     return false;
   }
 
   // Simple string comparison - releases must match exactly
-  bool match = strcmp(releaseString, extractedRelease) == 0;
+  bool match = strcmp(releaseString, safeRelease) == 0;
 
   DEBUG_PRINTF_P(PSTR("Release compatibility check: current='%s', uploaded='%s', match=%s\n"), 
-                 releaseString, extractedRelease, match ? "YES" : "NO");
+                 releaseString, safeRelease, match ? "YES" : "NO");
 
   return match;
 }
 
-bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, bool skipValidation, char* errorMessage) {
+bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessage, size_t errorMessageLen) {
   // Clear error message
-  if (errorMessage) {
+  if (errorMessage && errorMessageLen > 0) {
     errorMessage[0] = '\0';
   }
 
   // Ensure our custom description structure is referenced (prevents optimization)
   const wled_custom_desc_t* local_desc = getWledCustomDesc();
   (void)local_desc; // Suppress unused variable warning
 
-  // If user chose to skip validation, allow OTA immediately
-  if (skipValidation) {
-    DEBUG_PRINTLN(F("OTA release check bypassed by user"));
-    return true;
-  }
-
   // Try to extract WLED structure directly from binary data
   wled_custom_desc_t extractedDesc;
   bool hasCustomDesc = extractWledCustomDesc(binaryData, dataSize, &extractedDesc);
 
   if (!hasCustomDesc) {
     // No custom description - this could be a legacy binary
-    if (errorMessage) {
-      strcpy(errorMessage, "This firmware file is missing compatibility metadata. Enable 'Ignore firmware validation' to proceed anyway.");
+    if (errorMessage && errorMessageLen > 0) {
+      const char* msg = "This firmware file is missing compatibility metadata. Enable 'Ignore firmware validation' to proceed anyway.";
+      strncpy(errorMessage, msg, errorMessageLen - 1);
+      errorMessage[errorMessageLen - 1] = '\0';
     }
-    DEBUG_PRINTLN(F("OTA declined: No custom description found"));
     return false;
   }
 
   // Validate compatibility using extracted release name
   if (!validateReleaseCompatibility(extractedDesc.release_name)) {
-    if (errorMessage) {
-      snprintf(errorMessage, 127, "Firmware compatibility mismatch: current='%s', uploaded='%s'. Enable 'Ignore firmware validation' to proceed anyway.", 
+    if (errorMessage && errorMessageLen > 0) {
+      snprintf(errorMessage, errorMessageLen, "Firmware compatibility mismatch: current='%s', uploaded='%s'. Enable 'Ignore firmware validation' to proceed anyway.", 
                releaseString, extractedDesc.release_name);
+      errorMessage[errorMessageLen - 1] = '\0'; // Ensure null termination
     }
-    DEBUG_PRINTF_P(PSTR("OTA declined: Release mismatch current='%s', uploaded='%s'\n"), 
-                   releaseString, extractedDesc.release_name);
     return false;
   }
 
-  DEBUG_PRINTLN(F("OTA allowed: Release names match"));
   return true;
 }

wled00/ota_release_check.h

@@ -67,11 +67,11 @@ bool validateReleaseCompatibility(const char* extractedRelease);
  * Check if OTA should be allowed based on release compatibility using custom description
  * @param binaryData Pointer to binary file data (not modified)
  * @param dataSize Size of binary data in bytes
- * @param skipValidation If true, skip release validation
- * @param errorMessage Buffer to store error message if validation fails (should be at least 128 bytes)
+ * @param errorMessage Buffer to store error message if validation fails 
+ * @param errorMessageLen Maximum length of error message buffer
  * @return true if OTA should proceed, false if it should be blocked
  */
-bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, bool skipValidation, char* errorMessage);
+bool shouldAllowOTA(const uint8_t* binaryData, size_t dataSize, char* errorMessage, size_t errorMessageLen);
 
 /**
  * Get pointer to the embedded custom description structure

wled00/wled_server.cpp

@@ -442,12 +442,14 @@ void initServer()
       } else {
         // Validate OTA release compatibility using the first chunk data directly
         char errorMessage[128];
-        releaseCheckPassed = shouldAllowOTA(data, len, false, errorMessage);
+        releaseCheckPassed = shouldAllowOTA(data, len, errorMessage, sizeof(errorMessage));
 
         if (!releaseCheckPassed) {
           DEBUG_PRINTF_P(PSTR("OTA declined: %s\n"), errorMessage);
           request->send(400, FPSTR(CONTENT_TYPE_PLAIN), errorMessage);
           return;
+        } else {
+          DEBUG_PRINTLN(F("OTA allowed: Release compatibility check passed"));
         }
       }