Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
13 changes: 12 additions & 1 deletion tools/cdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ function filter(str, type) {
}
}

// Generate build timestamp as UNIX timestamp (seconds since epoch)
function generateBuildTime() {
return Math.floor(Date.now() / 1000);
}

function writeHtmlGzipped(sourceFile, resultFile, page) {
console.info("Reading " + sourceFile);
new inliner(sourceFile, function (error, html) {
Expand Down Expand Up @@ -141,7 +146,13 @@ function writeHtmlGzipped(sourceFile, resultFile, page) {
* Please see https://mm.kno.wled.ge/advanced/custom-features/#changing-web-ui
* to find out how to easily modify the web UI source!
*/


// Automatically generated build time for cache busting (UNIX timestamp)
#ifdef WEB_BUILD_TIME // avoid duplicate defintions
#undef WEB_BUILD_TIME
#endif
#define WEB_BUILD_TIME ${generateBuildTime()}

// Autogenerated from ${sourceFile}, do not edit!!
const uint16_t PAGE_${page}_L = ${result.length};
const uint8_t PAGE_${page}[] PROGMEM = {
Expand Down
183 changes: 183 additions & 0 deletions wled00/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2229,6 +2229,7 @@ function requestJson(command=null)
if (json.info) {
let i = json.info;
parseInfo(i);
checkVersionUpgrade(i); // Check for version upgrade
populatePalettes(i);
if (isInfo) populateInfo(i);
}
Comment on lines 2229 to 2235

@coderabbitai coderabbitai Bot Nov 29, 2025

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 | 🔴 Critical

version-info.json is likely never saved (/upload field name mismatch, no loc support)

The version-reporting flow is well thought out overall, but there are two concrete issues that will likely break persistence and file-mode behavior:

  1. Wrong form field name for /upload

Existing code uses uploadFileWithText() to POST files to /upload:

function uploadFileWithText(name, text) {
  ...
  var formData = new FormData();
  var blob = new Blob([text], {type : 'application/text'});
  var fileOfBlob = new File([blob], name);
  formData.append("upload", fileOfBlob);
  req.send(formData);
}

The new updateVersionInfo() instead does:

const blob = new Blob([JSON.stringify(versionInfo)], { type: 'application/json' });
const formData = new FormData();
formData.append('data', blob, 'version-info.json');

fetch('/upload', {
  method: 'POST',
  body: formData
})

The server-side upload handler in WLED expects the field name "upload" (as evidenced by uploadFileWithText); sending "data" means the firmware will not recognize or store the file. As a result:

  • version-info.json will never be created/updated.
  • checkVersionUpgrade() will keep treating this as a first install or as a version mismatch, so users will be re-prompted indefinitely.

You should either:

  • Reuse the existing helper:
-function updateVersionInfo(version, neverAsk) {
-  const versionInfo = { version, neverAsk };
-  // Create a Blob with JSON content and use /upload endpoint
-  const blob = new Blob([JSON.stringify(versionInfo)], { type: 'application/json' });
-  const formData = new FormData();
-  formData.append('data', blob, 'version-info.json');
-
-  fetch('/upload', { method: 'POST', body: formData });
-}
+function updateVersionInfo(version, neverAsk) {
+  const versionInfo = { version, neverAsk };
+  uploadFileWithText('/version-info.json', JSON.stringify(versionInfo));
+}
  • Or at least change the field name to "upload" and ensure the filename path matches what /edit?edit=version-info.json expects.
  1. Hard‑coded device-local URLs ignore loc / locip

Other fetches in this file consistently use (loc ? \http://${locip}` : '')as a prefix so the UI works both when served by the device and when opened fromfile:`. The new logic uses bare paths:

  • fetch('/edit?edit=version-info.json', ...)
  • fetch('/json/info', ...)
  • fetch('/upload', ...)

In file‑mode these will target the local filesystem origin instead of the WLED device, so:

  • checkVersionUpgrade() will fail its fetch, fall into the catch, and attempt to POST /upload to the wrong origin.
  • reportUpgradeEvent() will fail to fetch /json/info at all.

If you care about file‑mode parity, you should follow the same pattern used elsewhere:

-function checkVersionUpgrade(info) {
+function checkVersionUpgrade(info) {
+  const base = loc ? `http://${locip}` : '';
   ...
-  fetch('/edit?edit=version-info.json', { method: 'get' })
+  fetch(base + '/edit?edit=version-info.json', { method: 'get' })
   ...

and similarly for /json/info and /upload.

Once those two points are fixed, the flow (single per-load check, honor neverAsk, prompt on real version changes, and build an upgradeData payload from /json/info) looks structurally sound.

Also applies to: 3690-3871

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@coderabbitai i have tested the code (latest commit of this PR) on several boards, and version-info.json is properly created on the device, and it is also loaded and recognized by the JS function.

please re-check your analysis, I think you made some wrong assumptions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@softhack007, thank you for testing and providing that feedback! I apologize for the incorrect analysis. If version-info.json is being properly created and recognized on the device, then my assumptions about the /upload endpoint were clearly wrong.

I'm curious about one thing for my own learning: I noticed that the new updateVersionInfo() function uses 'data' as the FormData field name:

formData.append('data', blob, 'version-info.json');

While the existing uploadFileWithText() function uses 'upload':

formData.append("upload", fileOfBlob);

Does the WLED firmware /upload endpoint accept both field names, or does it perhaps just look at the filename regardless of the field name? Understanding this would help me avoid similar misassumptions in the future.

In any case, I appreciate you taking the time to test this thoroughly—empirical evidence always beats theoretical analysis!


🧠 Learnings used
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.

Expand Down Expand Up @@ -3686,6 +3687,188 @@ function mergeDeep(target, ...sources)
}
return mergeDeep(target, ...sources);
}
// Version reporting feature
var versionCheckDone = false;

function checkVersionUpgrade(info) {
// Only check once per page load
if (versionCheckDone) return;
versionCheckDone = true;

// Fetch version-info.json using existing /edit endpoint
fetch('/edit?edit=version-info.json', {
method: 'get'
})
.then(res => {
if (res.status === 404) {
// File doesn't exist - first install, show install prompt
showVersionUpgradePrompt(info, null, info.ver);
return null;
}
if (!res.ok) {
throw new Error('Failed to fetch version-info.json');
}
return res.json();
})
.then(versionInfo => {
if (!versionInfo) return; // 404 case already handled

// Check if user opted out
if (versionInfo.neverAsk) return;

// Check if version has changed
const currentVersion = info.ver;
const storedVersion = versionInfo.version || '';

if (storedVersion && storedVersion !== currentVersion) {
// Version has changed, show upgrade prompt
showVersionUpgradePrompt(info, storedVersion, currentVersion);
} else if (!storedVersion) {
// Empty version in file, show install prompt
showVersionUpgradePrompt(info, null, currentVersion);
}
})
.catch(e => {
console.log('Failed to load version-info.json', e);
// On error, save current version for next time
if (info && info.ver) {
updateVersionInfo(info.ver, false);
}
});
}

function showVersionUpgradePrompt(info, oldVersion, newVersion) {
// Determine if this is an install or upgrade
const isInstall = !oldVersion;

// Create overlay and dialog
const overlay = d.createElement('div');
overlay.id = 'versionUpgradeOverlay';
overlay.style.cssText = 'position:fixed;top:0;left:0;width:100%;height:100%;background:rgba(0,0,0,0.7);z-index:10000;display:flex;align-items:center;justify-content:center;';

const dialog = d.createElement('div');
dialog.style.cssText = 'background:var(--c-1);border-radius:10px;padding:25px;max-width:500px;margin:20px;box-shadow:0 4px 6px rgba(0,0,0,0.3);';

// Build contextual message based on install vs upgrade
const title = isInstall
? '🎉 Thank you for installing WLED-MM!'
: '🎉 WLED-MM Upgrade Detected!';

const description = isInstall
? `You are now running WLED-MM <strong>${newVersion}</strong>.`
: `Your WLED-MM has been upgraded from <strong>${oldVersion}</strong> to <strong>${newVersion}</strong>.`;

const question = 'Would you like to help the WLED development team by reporting your installation? This helps us understand what hardware and versions are being used.'

dialog.innerHTML = `
<h2 style="margin-top:0;color:var(--c-f);">${title}</h2>
<p style="color:var(--c-f);">${description}</p>
<p style="color:var(--c-f);">${question}</p>
<div style="margin-top:20px;">
<button id="versionReportYes" class="btn">Yes</button>
<button id="versionReportNo" class="btn">Not Now</button>
<button id="versionReportNever" class="btn">Never Ask</button>
</div>
`;

overlay.appendChild(dialog);
d.body.appendChild(overlay);

// Add event listeners
gId('versionReportYes').addEventListener('click', () => {
reportUpgradeEvent(oldVersion, newVersion);
d.body.removeChild(overlay);
});

gId('versionReportNo').addEventListener('click', () => {
// Don't update version, will ask again on next load
d.body.removeChild(overlay);
});
Comment thread
netmindz marked this conversation as resolved.

gId('versionReportNever').addEventListener('click', () => {
updateVersionInfo(newVersion, true);
d.body.removeChild(overlay);
showToast('You will not be asked again.');
});
}

function reportUpgradeEvent(oldVersion, newVersion) {
showToast('Reporting upgrade...');

// Fetch fresh data from /json/info endpoint as requested
fetch('/json/info', {
method: 'get'
})
.then(res => res.json())
.then(infoData => {
// Map to UpgradeEventRequest structure per OpenAPI spec
// Required fields: deviceId, version, previousVersion, releaseName, chip, ledCount, isMatrix, bootloaderSHA256
const upgradeData = {
deviceId: infoData.deviceId, // Use anonymous unique device ID
version: infoData.ver || '', // Current version string
previousVersion: oldVersion || '', // Previous version from version-info.json
releaseName: infoData.release || '', // Release name (e.g., "WLED 0.15.0")
chip: infoData.arch || '', // Chip architecture (esp32, esp8266, etc)
ledCount: infoData.leds ? infoData.leds.count : 0, // Number of LEDs
isMatrix: !!(infoData.leds && infoData.leds.matrix), // Whether it's a 2D matrix setup
bootloaderSHA256: infoData.bootloaderSHA256 || '', // Bootloader SHA256 hash - not yet availeable in WLEDMM
brand: infoData.brand, // Device brand (always present)
product: infoData.product, // Product name (always present)
flashSize: infoData.flash // Flash size (always present)
};
// Add optional fields if available
if (infoData.tpram !== undefined) upgradeData.psramSize = Math.round(infoData.tpram / (1024 * 1024)); // convert bytes to MB - tpram is MM specific
// Note: partitionSizes not currently available in /json/info endpoint
// it is availeable in WLEDMM => infoData.t = total FS size in bytes

// Make AJAX call to postUpgradeEvent API
return fetch('https://usage.wled.me/api/usage/upgrade', {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(upgradeData)
});
Comment on lines +3821 to +3827

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add timeout and improve error handling for external API call.

The fetch request to https://usage.wled.me/api/usage/upgrade has no timeout and could hang indefinitely. Additionally, the hardcoded external URL may face CORS restrictions depending on the server configuration.

Add a timeout and better error context:

 		// Make AJAX call to postUpgradeEvent API
-		return fetch('https://usage.wled.me/api/usage/upgrade', {
+		const controller = new AbortController();
+		const timeoutId = setTimeout(() => controller.abort(), 10000); // 10 second timeout
+		
+		return fetch('https://usage.wled.me/api/usage/upgrade', {
 			method: 'POST',
 			headers: {
 				'Content-Type': 'application/json'
 			},
-			body: JSON.stringify(upgradeData)
+			body: JSON.stringify(upgradeData),
+			signal: controller.signal
+		}).finally(() => {
+			clearTimeout(timeoutId);
 		});
 	})
 	.then(res => {
 		if (res.ok) {
 			showToast('Thank you for reporting!');
 			updateVersionInfo(newVersion, false);
 		} else {
-			showToast('Report failed. Please try again later.', true);
+			showToast(`Report failed (${res.status}). Please try again later.`, true);
 			// Do NOT update version info on failure - user will be prompted again
 		}
 	})
 	.catch(e => {
-		console.log('Failed to report upgrade', e);
-		showToast('Report failed. Please try again later.', true);
+		const isTimeout = e.name === 'AbortError';
+		console.log('Failed to report upgrade', e);
+		showToast(isTimeout ? 'Report timed out. Please try again later.' : 'Report failed. Please try again later.', true);
 		// Do NOT update version info on error - user will be prompted again
 	});

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In wled00/data/index.js around lines 3825-3831, the fetch to
https://usage.wled.me/api/usage/upgrade has no timeout and lacks robust error
handling and CORS consideration; wrap the call in an async try/catch, use an
AbortController with a configurable timeout (clear the timeout on completion),
check response.ok and parse/attach response body or status to any thrown error,
and surface/log the enriched error context; additionally make the URL
configurable (env/config) or move the request to a server-side endpoint to avoid
client-side CORS issues.

})
.then(res => {
if (res.ok) {
showToast('Thank you for reporting!');
updateVersionInfo(newVersion, false);
} else {
showToast('Report failed. Please try again later.', true);
// Do NOT update version info on failure - user will be prompted again
}
})
.catch(e => {
console.log('Failed to report upgrade', e);
showToast('Report failed. Please try again later.', true);
// Do NOT update version info on error - user will be prompted again
});
}

function updateVersionInfo(version, neverAsk) {
const versionInfo = {
version: version,
neverAsk: neverAsk
};

// Create a Blob with JSON content and use /upload endpoint
const blob = new Blob([JSON.stringify(versionInfo)], { type: 'application/json' });
const formData = new FormData();
formData.append('data', blob, 'version-info.json');

fetch('/upload', {
method: 'POST',
body: formData
})
.then(res => res.text())
.then(data => {
console.log('Version info updated', data);
})
.catch(e => {
console.log('Failed to update version-info.json', e);
});
}

size();
_C.style.setProperty('--n', N);
Expand Down
79 changes: 46 additions & 33 deletions wled00/wled_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,56 @@
#endif
#include "html_cpal.h"

/*
* Integrated HTTP web server page declarations
*/

bool handleIfNoneMatchCacheHeader(AsyncWebServerRequest* request);
void setStaticContentCacheHeaders(AsyncWebServerResponse *response);

// define flash strings once (saves flash memory)
static const char s_redirecting[] PROGMEM = "Redirecting...";
static const char s_content_enc[] PROGMEM = "Content-Encoding";
static const char s_unlock_ota [] PROGMEM = "Please unlock OTA in security settings!";
static const char s_unlock_cfg [] PROGMEM = "Please unlock settings using PIN code!";
static const char s_cache_control[] PROGMEM = "Cache-Control";
static const char s_no_store[] PROGMEM = "no-store";
static const char s_expires[] PROGMEM = "Expires";

/*
* Integrated HTTP web server page declarations
*/

static void generateEtag(char *etag, uint16_t eTagSuffix) {
sprintf_P(etag, PSTR("%u-%02x-%04x"), WEB_BUILD_TIME, cacheInvalidate, eTagSuffix);
}

static void setStaticContentCacheHeaders(AsyncWebServerResponse *response, int code=200, uint16_t eTagSuffix = 0) {
// Only send ETag for 200 (OK) responses
if (code != 200) return;

// https://medium.com/@codebyamir/a-web-developers-guide-to-browser-caching-cc41f3b73e7c
#ifndef WLED_DEBUG
// this header name is misleading, "no-cache" will not disable cache,
// it just revalidates on every load using the "If-None-Match" header with the last ETag value
response->addHeader(FPSTR(s_cache_control), F("no-cache"));
#else
response->addHeader(FPSTR(s_cache_control), F("no-store,max-age=0")); // prevent caching if debug build
#endif
char etag[32] = {'\0'};
generateEtag(etag, eTagSuffix);
response->addHeader(F("ETag"), etag);
}

static bool handleIfNoneMatchCacheHeader(AsyncWebServerRequest *request, int code=200, uint16_t eTagSuffix = 0) {
// Only send 304 (Not Modified) if response code is 200 (OK)
if (code != 200) return false;

AsyncWebHeader *header = request->getHeader(F("If-None-Match"));
char etag[32] = {'\0'};
generateEtag(etag, eTagSuffix);
if (header && header->value() == etag) {
AsyncWebServerResponse *response = request->beginResponse(304);
setStaticContentCacheHeaders(response, code, eTagSuffix);
request->send(response);
return true;
}
return false;
}


//Is this an IP?
bool isIp(String str) {
Expand Down Expand Up @@ -451,7 +489,7 @@ void initServer()
AsyncWebServerResponse *response = request->beginResponse_P(404, "text/html", PAGE_404, PAGE_404_length);
#endif
response->addHeader(FPSTR(s_content_enc),"gzip");
setStaticContentCacheHeaders(response);
setStaticContentCacheHeaders(response, 404);
request->send(response);
//request->send_P(404, "text/html", PAGE_404);
});
Expand All @@ -467,31 +505,6 @@ void serveIndexOrWelcome(AsyncWebServerRequest *request)
}
}

bool handleIfNoneMatchCacheHeader(AsyncWebServerRequest* request)
{
AsyncWebHeader* header = request->getHeader("If-None-Match");
if (header && header->value() == String(VERSION)) {
request->send(304);
return true;
}
return false;
}

void setStaticContentCacheHeaders(AsyncWebServerResponse *response)
{
char tmp[12];
// https://medium.com/@codebyamir/a-web-developers-guide-to-browser-caching-cc41f3b73e7c
#ifndef WLED_DEBUG
//this header name is misleading, "no-cache" will not disable cache,
//it just revalidates on every load using the "If-None-Match" header with the last ETag value
response->addHeader(F("Cache-Control"),"no-cache");
#else
response->addHeader(F("Cache-Control"),"no-store,max-age=0"); // prevent caching if debug build
#endif
sprintf_P(tmp, PSTR("%8d-%02x"), VERSION, cacheInvalidate);
response->addHeader(F("ETag"), tmp);
}

void serveIndex(AsyncWebServerRequest* request)
{
if (handleFileRead(request, "/index.htm")) return;
Expand Down