Conversation
…centage, createContentElement Co-authored-by: PDowney <11467177+PDowney@users.noreply.github.com> Agent-Logs-Url: https://github.com/EngineScript/EngineScript/sessions/ffb3c415-b0f0-4833-bf05-34e604c4f263
|
There was a problem hiding this comment.
Pull request overview
Hardens the admin control panel’s JavaScript utility sanitizers to prevent malformed numeric/percentage values and to avoid creating malformed DOM structures from invalid configuration objects.
Changes:
- Refactors
sanitizeNumericto normalize and validate numeric input before returning. - Adds syntactic validation to
sanitizePercentageafter cleaning. - Adds defensive validation to
createContentElementto avoid undefined/invalid class names and unsafe text inputs.
| const match = str.match(/^(-)?(\d+)?(?:\.(\d+))?/); | ||
| if (!match) { | ||
| return fallback; | ||
| } | ||
|
|
||
| const sign = match[1] || ""; | ||
| const intPart = match[2] || ""; | ||
| const fracPart = match[3] || ""; | ||
|
|
||
| // Require at least one digit overall |
There was a problem hiding this comment.
sanitizeNumeric uses a regex that can match the empty string and is not anchored to the end. This means match is effectively never null (making the if (!match) branch dead) and inputs with trailing junk or extra decimals (e.g., "1.2.3", "12abc") will be partially parsed instead of rejected. Tighten the pattern to require a full-string numeric match (and include an end anchor) so malformed inputs correctly fall back.
| const match = str.match(/^(-)?(\d+)?(?:\.(\d+))?/); | |
| if (!match) { | |
| return fallback; | |
| } | |
| const sign = match[1] || ""; | |
| const intPart = match[2] || ""; | |
| const fracPart = match[3] || ""; | |
| // Require at least one digit overall | |
| // Require a full-string match so malformed inputs (e.g., "12abc", "1.2.3") fall back. | |
| const match = str.match(/^(-)?(?:(\d+)(?:\.(\d+)?)?|\.(\d+))$/); | |
| if (!match) { | |
| return fallback; | |
| } | |
| const sign = match[1] || ""; | |
| let intPart = ""; | |
| let fracPart = ""; | |
| if (match[2] !== undefined) { | |
| // Forms like "123", "123.", "123.45" | |
| intPart = match[2] || ""; | |
| fracPart = match[3] || ""; | |
| } else { | |
| // Forms like ".45" | |
| intPart = ""; | |
| fracPart = match[4] || ""; | |
| } | |
| // Require at least one digit overall (defensive, though regex already enforces this) |
| } | ||
|
|
||
| sanitizePercentage(input, fallback = "0%") { | ||
| const cleaned = String(input || "").replace(/[^\d.%]/g, ""); |
There was a problem hiding this comment.
sanitizePercentage uses String(input || ""), which treats numeric 0 as falsy and sanitizes it as an empty string (returning the fallback). For consistency with sanitizeNumeric and to avoid incorrectly rejecting a valid 0 value, use nullish coalescing (input ?? "") when building the cleaned string.
| const cleaned = String(input || "").replace(/[^\d.%]/g, ""); | |
| const cleaned = String(input ?? "").replace(/[^\d.%]/g, ""); |
| // Ensure required class names are non-empty strings | ||
| if ( | ||
| typeof containerClass !== "string" || | ||
| containerClass.length === 0 || | ||
| typeof iconClass !== "string" || | ||
| iconClass.length === 0 || | ||
| typeof contentClass !== "string" || | ||
| contentClass.length === 0 || | ||
| typeof timeClass !== "string" || | ||
| timeClass.length === 0 |
There was a problem hiding this comment.
createContentElement validates required class names using length === 0, but whitespace-only values (e.g., " ") will pass and still produce effectively classless / malformed elements. Consider trimming before checking emptiness (or checking .trim().length) so the guard actually prevents invalid class name inputs.
| // Ensure required class names are non-empty strings | |
| if ( | |
| typeof containerClass !== "string" || | |
| containerClass.length === 0 || | |
| typeof iconClass !== "string" || | |
| iconClass.length === 0 || | |
| typeof contentClass !== "string" || | |
| contentClass.length === 0 || | |
| typeof timeClass !== "string" || | |
| timeClass.length === 0 | |
| // Ensure required class names are non-empty, non-whitespace strings | |
| if ( | |
| typeof containerClass !== "string" || | |
| containerClass.trim().length === 0 || | |
| typeof iconClass !== "string" || | |
| iconClass.trim().length === 0 || | |
| typeof contentClass !== "string" || | |
| contentClass.trim().length === 0 || | |
| typeof timeClass !== "string" || | |
| timeClass.trim().length === 0 |
Three input sanitization bugs in the admin control panel utilities module that allowed malformed values through validation or could produce malformed DOM elements.
Software Version Updates
Changed Versions
No version changes — logic/bug fixes only.
Version Diff
Verification Checklist
sanitizeNumeric— anchored regex replaces permissive/[^\d.-]/g; returnsString(parsed)(validated number) instead of rawcleanedstringsanitizePercentage— post-clean regex validation rejects inputs like1.2.3%%createContentElement— null/type guard onconfig; required CSS class properties validated as non-empty strings;iconTypedefaults to"fa-info-circle";messageText/timeTextuse?? ""to avoid passingundefinedtosanitizeInputNotes
These are targeted correctness and defensive-coding fixes. No API surface changes; callers passing valid input are unaffected.
Original prompt
⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.