diff --git a/brightsign/autorun.brs b/brightsign/autorun.brs index ebd3595..8e4f544 100644 --- a/brightsign/autorun.brs +++ b/brightsign/autorun.brs @@ -193,7 +193,13 @@ Sub TakeSnapshot(widget As Object, req As Object) if req <> invalid and req.width <> invalid then w% = req.width if req <> invalid and req.height <> invalid then h% = req.height - body$ = "{""width"":" + Stri(w%).Trim() + ",""height"":" + Stri(h%).Trim() + "}" + ' BrightScript has NO escape sequences in string literals: "" does not mean an escaped quote, + ' it ends one string and begins another, so `"{""width"":"` is three literals with no operator + ' between them — a compile error that stops the WHOLE SCRIPT loading, not just this function. + ' A quote has to come from Chr(34). This line is why the player booted to nothing: + ' ScriptLoadError: Syntax Error. (compile error &h02) in SSD:/autorun.brs(196) + q$ = Chr(34) + body$ = "{" + q$ + "width" + q$ + ":" + Stri(w%).Trim() + "," + q$ + "height" + q$ + ":" + Stri(h%).Trim() + "}" ut = CreateObject("roUrlTransfer") if ut = invalid then @@ -205,8 +211,28 @@ Sub TakeSnapshot(widget As Object, req As Object) ut.SetUserAndPassword("admin", serial$) ut.AddHeader("Content-Type", "application/json") - resp$ = ut.PostFromStringWithRetry(body$, 1) - if resp$ = invalid or resp$ = "" then + ' PostFromStringWithRetry does not exist — calling it raised "Member function not found" from + ' inside the event loop, i.e. a snapshot request took the whole player down. And the synchronous + ' PostFromString() is no use either: it returns only a response CODE and discards the body, which + ' is where the thumbnail is. The documented way to read a POST response is asynchronous, on a + ' message port. + port = CreateObject("roMessagePort") + ut.SetPort(port) + if not ut.AsyncPostFromString(body$) then + widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "could not reach the local DWS" }) + return + end if + + ' Bounded: a capture that never answers must not wedge the event loop that drives playback. + ev = Wait(20000, port) + if type(ev) <> "roUrlEvent" then + ut.AsyncCancel() + widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "the local DWS did not answer" }) + return + end if + + resp$ = ev.GetString() + if resp$ = "" then widget.PostJSMessage({ type: "snapshot-result", ok: false, error: "no response from the local DWS" }) return end if @@ -258,18 +284,48 @@ Sub SetOrientation(widget As Object, o As String) return end if - mode$ = vm.GetMode() - if mode$ = invalid or mode$ = "" then mode$ = "1920x1080x60p" - - ok = vm.SetMode(mode$, transform$) - if ok = invalid then ok = false - - if ok then - print "[st] orientation "; o; " -> transform "; transform$ - else - print "[st] orientation "; o; ": SetMode refused transform "; transform$ + ' SetMode() takes ONE argument — a mode string. Passing a transform as a second argument was a + ' "wrong number of function parameters" abort, so this Sub never reached its own reply and the + ' page never learned to fall back. Rotation lives on SetScreenModes(), whose per-screen config + ' carries a `transform` of normal|90|180|270 and rotates EVERYTHING including the video plane. + ' Implemented in BOS 9.0.15+; an older player simply has no method here and is told so. + ' FindMemberFunction is the guard BrightSign's own scripts use for a method that may not exist + ' on this OS version — safer than naming a member directly, which would attempt the call. + if FindMemberFunction(vm, "GetScreenModes") = invalid or FindMemberFunction(vm, "SetScreenModes") = invalid then + print "[st] orientation: this OS has no SetScreenModes — the page keeps its CSS fallback" + widget.PostJSMessage({ type: "orientation-result", ok: false, error: "SetScreenModes unavailable" }) + return end if - widget.PostJSMessage({ type: "orientation-result", ok: ok, transform: transform$ }) + + configs = vm.GetScreenModes() + if configs = invalid or configs.Count() = 0 then + widget.PostJSMessage({ type: "orientation-result", ok: false, error: "no screen configuration" }) + return + end if + + ' ⚠️ SetScreenModes REBOOTS the player when it changes the screen configuration. A playlist push + ' repeats the current orientation on every update, so applying it unconditionally would reboot + ' the display every time the server spoke to it. Only a real change is worth a reboot. + changed = false + for each c in configs + if c.transform <> transform$ then + c.transform = transform$ + changed = true + end if + end for + + if not changed then + print "[st] orientation already "; transform$; " — nothing to do" + widget.PostJSMessage({ type: "orientation-result", ok: true, transform: transform$ }) + return + end if + + ' Tell the page BEFORE the call: the reboot may take the player out mid-sentence, and a display + ' that rotates without ever confirming looks like the command was ignored. + widget.PostJSMessage({ type: "orientation-result", ok: true, transform: transform$, rebooting: true }) + print "[st] orientation "; o; " -> transform "; transform$; " (the player will now reboot)" + sleep(1000) + vm.SetScreenModes(configs) End Sub '=== capability probe ======================================================================= @@ -289,10 +345,19 @@ End Sub Function StorageProbe() As Object result = { present: false, volume: "", free_mb: 0, total_mb: 0 } - volumes = ["SSD:", "SD:", "USB1:"] + ' "USB:" not "USB1:" — the docs warn that GetStorageStatus() results are UNRELIABLE when called + ' with a "USBn:" parameter, and list "USB:", "SD:", "SSD:", "SD2:/", "Flash:" as the drive + ' strings it understands. One roStorageHotplug for the whole loop rather than one per volume. + hp = CreateObject("roStorageHotplug") + ' Ask the platform which volumes exist rather than guessing; the static list is the fallback for + ' an OS without the enumerator. Same shape BrightSign's own boilerplate uses. + volumes = ["SSD:", "SD:", "SD2:", "USB:"] + if hp <> invalid and FindMemberFunction(hp, "GetStorages") <> invalid then + found = hp.GetStorages() + if found <> invalid and found.Count() > 0 then volumes = found + end if for each v in volumes mounted = false - hp = CreateObject("roStorageHotplug") if hp <> invalid then st = hp.GetStorageStatus(v) if st <> invalid and st.mounted then mounted = true @@ -372,8 +437,16 @@ Function PackageVersion() As String return "0.0.0-dev" ' ST_PACKAGE_VERSION (stamped at build time — do not edit by hand) End Function -Function DoesFileExist(filePath$ As String) As Boolean - files = MatchFiles(filePath$, filePath$) +' Does [name] exist in directory [dir]? +' +' MatchFiles takes a DIRECTORY plus a pattern, and returns nothing when the pattern contains a +' separator. The previous version passed a full path as both arguments, so it answered "no" for +' every file on every player — which silently disabled this entire self-update path: the pending +' package was never seen, the .part file was never cleaned up, and the .done marker was never +' noticed. Two arguments, so there is no path-splitting to get wrong. +Function FileExists(dir As String, name As String) As Boolean + files = MatchFiles(dir, name) + if files = invalid then return false return files.Count() > 0 End Function @@ -385,33 +458,55 @@ End Function ' storage root the OS no longer auto-processes the archive — so from then on the host has to do it ' itself, or self-update would work exactly once. Sub ApplyPendingPackage(root As String) + dir$ = root + "/" zipPath$ = root + "/autorun.zip" donePath$ = root + "/autorun.zip.done" badPath$ = root + "/autorun.zip.bad" + stage$ = root + "/st-staging" - if not DoesFileExist(zipPath$) then return - if DoesFileExist(donePath$) then return ' already unpacked; extracting again is the boot loop + if not FileExists(dir$, "autorun.zip") then return + if FileExists(dir$, "autorun.zip.done") then return ' already unpacked; again is the boot loop print "[st-update] unpacking pending package" package = CreateObject("roBrightPackage", zipPath$) if package = invalid then - print "[st-update] ERROR: archive unreadable (is it STORED?) — parking it as .bad" - fs = CreateObject("roFileSystem") - if fs <> invalid then fs.Rename(zipPath$, badPath$) + print "[st-update] ERROR: archive unreadable — parking it as .bad" + MoveFile(zipPath$, badPath$) return end if - if not package.Unpack(root + "/") then - print "[st-update] ERROR: extract failed — parking it as .bad so we do not retry forever" - fs = CreateObject("roFileSystem") - if fs <> invalid then fs.Rename(zipPath$, badPath$) + ' ⚠️ Unpack() DELETES everything already in its target directory: "Providing a destination path + ' of SD:/ will wipe all preexisting files from the card". Unpacking straight to the volume root + ' would therefore erase this player's provisioning and its entire content pool on every update — + ' the update would work and the display would come back empty and unpaired. + ' + ' So it goes to a staging directory of its own, and the files are moved into place afterwards. + ' The wipe is then a FEATURE: it clears any half-extracted remains of a previous attempt. + CreateDirectory(stage$) + package.Unpack(stage$ + "/") + + ' Unpack() returns Void, so success is proven by looking for what should now exist rather than + ' by testing a return value that was never there. + if not FileExists(stage$ + "/", "autorun.brs") then + print "[st-update] ERROR: extract produced no autorun.brs — parking it as .bad" + MoveFile(zipPath$, badPath$) return end if - fs = CreateObject("roFileSystem") - if fs = invalid then return - if not fs.Rename(zipPath$, donePath$) then + ' screentinker.json is deliberately NOT copied over: it carries THIS player's provisioning + ' (server URL, device id), and the copy inside a package carries the build's defaults. Letting + ' an update overwrite it would re-point or unpair the display as a side effect of a routine + ' upgrade — silently, and on every player at once. + moved% = 0 + for each name in MatchFiles(stage$, "*") + if name <> "screentinker.json" then + if MoveFile(stage$ + "/" + name, root + "/" + name) then moved% = moved% + 1 + end if + end for + print "[st-update] installed "; moved%; " file(s)" + + if not MoveFile(zipPath$, donePath$) then ' Refusing to reboot without the marker: we would extract and reboot forever. print "[st-update] ERROR: could not mark done — not rebooting" return @@ -442,7 +537,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String) xfer.SetUrl(url$) xfer.EnablePeerVerification(true) body$ = xfer.GetToString() - if body$ = "" then return ' unreachable: keep running what works + if body$ = "" then return ' unreachable server: keep running what works manifest = ParseJson(body$) if manifest = invalid then return @@ -456,8 +551,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String) ' Any earlier partial is deleted first: resuming into an existing file would concatenate two ' downloads into something that hashes to neither. - fs = CreateObject("roFileSystem") - if fs <> invalid and DoesFileExist(partPath$) then fs.Delete(partPath$) + if FileExists(root + "/", "autorun.zip.part") then DeleteFile(partPath$) dl = CreateObject("roUrlTransfer") if dl = invalid then return @@ -466,7 +560,7 @@ Sub CheckPackageUpdate(cfg As Object, root As String) if dl.GetToFile(partPath$) <> 200 then print "[st-update] download failed" RecordPackageAttempt(reg, attempts% + 1) - if fs <> invalid then fs.Delete(partPath$) + DeleteFile(partPath$) return end if @@ -474,15 +568,14 @@ Sub CheckPackageUpdate(cfg As Object, root As String) if not VerifyPackage(partPath$, manifest.sha256, manifest.size) then print "[st-update] VERIFICATION FAILED — discarding, staying on "; PackageVersion() RecordPackageAttempt(reg, attempts% + 1) - if fs <> invalid then fs.Delete(partPath$) + DeleteFile(partPath$) return end if ' Promote. Marker first — see the ordering note above. - if fs = invalid then return - if DoesFileExist(root + "/autorun.zip.done") then fs.Delete(root + "/autorun.zip.done") - if DoesFileExist(root + "/autorun.zip") then fs.Delete(root + "/autorun.zip") - if not fs.Rename(partPath$, root + "/autorun.zip") then + if FileExists(root + "/", "autorun.zip.done") then DeleteFile(root + "/autorun.zip.done") + if FileExists(root + "/", "autorun.zip") then DeleteFile(root + "/autorun.zip") + if not MoveFile(partPath$, root + "/autorun.zip") then print "[st-update] ERROR: could not stage the package — staying put" RecordPackageAttempt(reg, attempts% + 1) return @@ -505,35 +598,40 @@ End Sub ' sha256 + size. Both matter: the hash proves the bytes are the ones we were promised, the size ' floor catches an error page or captive-portal login saved under the package's name. Function VerifyPackage(path As String, expected As String, expectedSize As Integer) As Boolean - if expected = invalid or expected = "" then return false + ' Guard the arguments before the type declarations do it for us: a manifest missing sha256 or + ' size passes `invalid` into an `As String`/`As Integer` parameter, which is a runtime error at + ' the CALL — before any check inside the function could help. + if expected = "" then return false - fs = CreateObject("roFileSystem") - if fs = invalid then return false - - info = fs.Stat(path) - if info = invalid then return false - if info.size < 1024 then - print "[st-update] package is implausibly small ("; info.size; " bytes)" - return false - end if - if expectedSize > 0 and info.size <> expectedSize then - print "[st-update] size mismatch: got "; info.size; " expected "; expectedSize + ' roByteArray + roHashGenerator. The previous version used roFileSystem.Stat/OpenInputFile and + ' roMessageDigest — all three are Roku objects that do not exist on BrightSign, so this function + ' returned false unconditionally and every self-update failed verification and burned an + ' attempt. The package is tens of kilobytes, so reading it whole is cheaper than the streaming + ' loop it replaces. + ba = CreateObject("roByteArray") + if ba = invalid then return false + if not ba.ReadFile(path) then + print "[st-update] package unreadable at "; path return false end if - digest = CreateObject("roMessageDigest") + size% = ba.Count() + if size% < 1024 then + print "[st-update] package is implausibly small ("; size%; " bytes)" + return false + end if + if expectedSize > 0 and size% <> expectedSize then + print "[st-update] size mismatch: got "; size%; " expected "; expectedSize + return false + end if + + hg = CreateObject("roHashGenerator", "sha256") + if hg = invalid then return false + digest = hg.Hash(ba) if digest = invalid then return false - digest.SetAlgorithm("sha256") - file = fs.OpenInputFile(path) - if file = invalid then return false - while true - chunk = file.Read(65536) - if chunk.Count() = 0 then exit while - digest.Update(chunk) - end while - - return LCase(digest.Final()) = LCase(expected) + ' Hash() answers with an roByteArray, not a string. + return LCase(digest.ToHexString()) = LCase(expected) End Function '=== main =================================================================================== @@ -613,7 +711,10 @@ Sub Main() ' Back off, then fall back to the local page so the screen says something ' truthful instead of showing white. The local page keeps retrying the server. retries = retries + 1 - print "[st] load-error ("; retries; "): "; data.url + ' The key is `uri` on a load-error; `url` belongs to download-request. Printing + ' the wrong one meant the single diagnostic that names the failing resource always + ' printed "invalid". + print "[st] load-error ("; retries; "): "; data.uri sleep(ChooseBackoff(retries)) if retries >= 3 then ' The server URL rides along so the fallback page can name it on screen and diff --git a/brightsign/autozip.brs b/brightsign/autozip.brs index d21c572..0e09259 100644 --- a/brightsign/autozip.brs +++ b/brightsign/autozip.brs @@ -13,11 +13,13 @@ ' being processed at all. autorun.brs belongs INSIDE the zip, which is where the build script puts ' it (scripts/build-autorun-zip.sh). ' -' Unpacks with roBrightPackage, which is what BrightSign's own tooling uses — NOT roUnzip. -' A BrightSign consultant flagged this after our first archive failed his automated deployment: -' the zip reached the player and then could not be opened. Two causes, both fixed: -' - the archive must be STORED, no compression (scripts/build-autorun-zip.sh now asserts it) -' - roBrightPackage is the supported reader for a player package +' Unpacks with roBrightPackage, which is what BrightSign's own tooling uses. +' +' On compression: roBrightPackage supports deflate32 with default options, PPMd, and "no +' compression". What it does NOT support is bzip2, LZMA, Deflate64 and Zip64 (the last being what +' Windows Explorer's built-in zipper produces). We build STORED, which is stricter than required and +' costs nothing at this size — but note that compression was NOT the cause of the deployment failure +' this script was blamed for. That was the MatchFiles bug below. ' ' Requires BrightSignOS 7.0.60+. @@ -26,9 +28,9 @@ ' for the file beats assuming a volume: extracting to "SD:/" on a player with no card writes to a ' volume that does not exist, and the deployment silently does nothing. Function SourceRoot() As String - volumes = ["USB1:", "SD:", "SSD:", "FLASH:"] + volumes = ["USB1:", "SD:", "SD2:", "SSD:", "FLASH:"] for each v in volumes - if DoesFileExist(v + "/autorun.zip") then return v + if FileExists(v + "/", "autorun.zip") then return v end for return "" End Function @@ -45,14 +47,14 @@ Sub Main() print "[st-autozip] volume "; root$ - if not DoesFileExist(zipPath$) then + if not FileExists(extractPath$, "autorun.zip") then print "[st-autozip] no autorun.zip at "; zipPath$; " — nothing to do" return end if ' Idempotence. Without this the player extracts, reboots, extracts again, reboots again — ' a boot loop that looks like a hardware fault. - if DoesFileExist(donePath$) then + if FileExists(extractPath$, "autorun.zip.done") then print "[st-autozip] already unpacked (autorun.zip.done present) — leaving it alone" return end if @@ -67,20 +69,22 @@ Sub Main() return end if - if not package.Unpack(extractPath$) then - print "[st-autozip] ERROR: unpack failed" + ' Unpack() returns VOID — there is no boolean to test, and `if not package.Unpack(...)` was a + ' type error rather than an error check. Success is proven the only way that actually means + ' anything: the file we came here to install is now on the card. + package.Unpack(extractPath$) + + if not FileExists(extractPath$, "autorun.brs") then + print "[st-autozip] ERROR: unpack produced no autorun.brs — leaving the archive for a retry" return end if print "[st-autozip] extracted" - fs = CreateObject("roFileSystem") - if fs = invalid then - print "[st-autozip] ERROR: no roFileSystem — cannot mark the archive done" - return - end if - - if not fs.Rename(zipPath$, donePath$) then + ' MoveFile/DeleteFile are GLOBAL functions on BrightSign. roFileSystem is a Roku object and does + ' not exist here, so every one of these calls used to return invalid — which meant the archive + ' was never marked done and the player never rebooted into the player it had just installed. + if not MoveFile(zipPath$, donePath$) then print "[st-autozip] ERROR: could not rename the archive; refusing to reboot into a loop" return end if @@ -90,7 +94,17 @@ Sub Main() RebootSystem() End Sub -Function DoesFileExist(filePath$ As String) As Boolean - files = MatchFiles(filePath$, filePath$) +' Does [name] exist in directory [dir]? +' +' THE BUG THIS FIXES. The previous version passed a full path as BOTH arguments of MatchFiles. +' MatchFiles takes a DIRECTORY plus a pattern, and the documentation is explicit: "you will get no +' results if the pattern contains a directory separator". So it returned an empty list every time, +' for every file, on every player — and this script reported "no autorun.zip on any volume" while a +' `dir SD:` sat there listing autorun.zip. Reported from a real deployment on an HD1026. +' +' Two arguments rather than one path, so there is no string-splitting to get wrong. +Function FileExists(dir As String, name As String) As Boolean + files = MatchFiles(dir, name) + if files = invalid then return false return files.Count() > 0 End Function diff --git a/server/test/brightscript-api-surface.test.js b/server/test/brightscript-api-surface.test.js new file mode 100644 index 0000000..ab00421 --- /dev/null +++ b/server/test/brightscript-api-surface.test.js @@ -0,0 +1,200 @@ +'use strict'; + +// BrightScript cannot be run, linted or type-checked here — the only interpreter is a player. So a +// call to an object that does not exist looks exactly like a call to one that does, right up until +// a display in the field stops working. +// +// That is not hypothetical. `brightsign/*.brs` shipped with a whole family of ROKU APIs in it — +// roFileSystem, roMessageDigest, PostFromStringWithRetry — because BrightScript is Roku's language +// and the two references read almost identically. Each one silently disabled a feature: the +// self-update path could never mark a package applied, verification returned false unconditionally +// and burned an attempt counter, and a snapshot request raised "Member function not found" from +// inside the event loop, taking the player down. None of it was visible from here. +// +// This is the cheapest thing that would have caught all of it: a deny-list of APIs that exist on +// Roku and not on BrightSign, plus the argument-shape mistakes that made calls compile and then do +// nothing. It cannot prove the scripts are right. It does stop these specific, expensive mistakes +// coming back — and every entry below was paid for once already. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const DIR = path.join(__dirname, '..', '..', 'brightsign'); +const FILES = fs.readdirSync(DIR).filter((f) => f.endsWith('.brs')); + +/** Source with comment lines stripped, so prose about a bug is not mistaken for the bug. */ +function code(file) { + return fs.readFileSync(path.join(DIR, file), 'utf8') + .split('\n') + .filter((l) => !/^\s*'/.test(l)) + .join('\n'); +} + +test('there are BrightScript files to check', () => { + assert.ok(FILES.length >= 2, `expected the host scripts, found ${FILES.join(', ')}`); +}); + +// Objects that exist on Roku and NOT on BrightSign. Verified against BrightSign's Object Reference, +// which lists every ro* object the platform has. +const ROKU_ONLY = [ + ['roFileSystem', 'use the global MoveFile / DeleteFile / CopyFile, or roByteArray to read'], + ['roMessageDigest', 'use roHashGenerator("sha256"); Hash() answers with an roByteArray'], + ['roUnzip', 'use roBrightPackage — roUnzip is not the reader for a player package'], + ['roRegistryKey', 'use roRegistrySection'], + ['roAssociativeArrayEx', 'plain roAssociativeArray'], +]; + +for (const [obj, advice] of ROKU_ONLY) { + test(`no ${obj} — it does not exist on BrightSign (${advice})`, () => { + for (const f of FILES) { + assert.ok(!code(f).includes(obj), `${f} calls ${obj}, which is a Roku object. ${advice}`); + } + }); +} + +// Methods that do not exist on the object they are called on. +const BAD_METHODS = [ + ['PostFromStringWithRetry', 'roUrlTransfer has no retry variant; AsyncPostFromString + roUrlEvent is how a POST body is read'], + ['.Final(', 'roMessageDigest-era API; roHashGenerator returns the digest from Hash()'], + ['OpenInputFile', 'no roFileSystem here; read with roByteArray.ReadFile'], +]; + +for (const [needle, advice] of BAD_METHODS) { + test(`no ${needle.replace(/[.(]/g, '')} — ${advice}`, () => { + for (const f of FILES) { + assert.ok(!code(f).includes(needle), `${f} calls ${needle}. ${advice}`); + } + }); +} + +test('MatchFiles is called with a DIRECTORY and a bare pattern', () => { + // The bug that made a deployment report an empty card while `dir SD:` listed the file. MatchFiles + // takes a directory plus a pattern and is documented to return nothing when the pattern contains + // a separator — so passing a path as the second argument answers "no" for every file that exists. + for (const f of FILES) { + for (const m of code(f).matchAll(/MatchFiles\(([^)]*)\)/g)) { + const args = m[1].split(',').map((a) => a.trim()); + assert.equal(args.length, 2, `${f}: MatchFiles needs exactly two arguments, got ${m[0]}`); + assert.ok(!/["'].*\/.*["']/.test(args[1]) && !/\+/.test(args[1]), + `${f}: the MatchFiles PATTERN must not contain a path separator — ${m[0]}`); + } + } +}); + +test('Unpack() is never used as if it returned a boolean', () => { + // Unpack(path) is declared As Void. `if not package.Unpack(...)` is a type error, not an error + // check — and reads exactly like one. Success is proven by looking for an expected file instead. + for (const f of FILES) { + const src = code(f); + assert.ok(!/if\s+not\s+\w*\.?Unpack\s*\(/i.test(src), + `${f}: Unpack() returns Void — test for an extracted file rather than its return value`); + assert.ok(!/=\s*\w*\.?Unpack\s*\(/i.test(src), `${f}: Unpack() returns nothing to assign`); + } +}); + +test('Unpack() never targets a volume root', () => { + // "Providing a destination path of SD:/ will wipe all preexisting files from the card." Unpacking + // an update straight to the root would erase the player's provisioning and its whole content pool + // as a side effect of a routine upgrade. + for (const f of FILES) { + for (const m of code(f).matchAll(/\.Unpack\(([^)]*)\)/g)) { + const arg = m[1].trim(); + assert.ok(!/^(root|root\$)\s*\+\s*"\/"$/.test(arg) && !/^"[A-Z0-9]+:\/"$/.test(arg), + `${f}: ${m[0]} unpacks to a volume root, which DELETES everything already there. Stage it.`); + } + } +}); + +test('roVideoMode.SetMode() is called with exactly one argument', () => { + // SetMode(mode As String). A second argument is a "wrong number of function parameters" abort. + // Rotation belongs to SetScreenModes(), whose config carries the transform. + for (const f of FILES) { + for (const m of code(f).matchAll(/\.SetMode\(([^)]*)\)/g)) { + assert.equal(m[1].split(',').length, 1, `${f}: ${m[0]} — SetMode takes one argument`); + } + } +}); + +test('GetStorageStatus is not called with a USBn: drive string', () => { + // Documented: "The results of the GetStorageStatus() method are unreliable when called with a + // USBn: parameter." The drive strings it understands are "USB:", "SD:", "SSD:", "SD2:/", "Flash:". + for (const f of FILES) { + assert.ok(!/GetStorageStatus\(\s*"USB\d/i.test(code(f)), + `${f}: GetStorageStatus is unreliable with USBn: — use "USB:"`); + } +}); + +test('a load-error is reported with its uri, not a url', () => { + // `url` is a key of download-request; a load-error carries `uri`. Reading the wrong one made the + // only diagnostic that names the failing resource print "invalid" every time. + for (const f of FILES) { + const src = code(f); + if (!src.includes('load-error')) continue; + assert.ok(!/\bdata\.url\b/.test(src), `${f}: a load-error names its resource in data.uri`); + } +}); + +test('parameters do not carry BOTH a type suffix and an As clause', () => { + // `filePath$ As String` is a shape the reference never sanctions, and a parse error would stop + // the script loading at all — the worst possible failure, since nothing would run to report it. + for (const f of FILES) { + for (const m of code(f).matchAll(/(?:Function|Sub)\s+\w+\s*\(([^)]*)\)/g)) { + for (const param of m[1].split(',')) { + assert.ok(!/[$%!#&]\s+As\s+/i.test(param), + `${f}: parameter "${param.trim()}" has a type suffix and an As clause`); + } + } + } +}); + +test('the storage root is resolved by probing, not assumed', () => { + // Knowing only FLASH and SD meant that fitting real storage to a flash-booting player and moving + // the deployment onto it resolved every derived path to a volume that was not there. + const src = code('autorun.brs'); + const fn = src.slice(src.indexOf('Function StorageRoot')); + for (const vol of ['SSD:', 'USB1:', 'FLASH:']) { + assert.ok(fn.slice(0, 900).includes(vol), `StorageRoot() must consider ${vol}`); + } +}); + +test('the widget storage path is absolute, on a real volume', () => { + // "/cache" carries no drive specifier, so the widget's local storage has nowhere to persist. + const src = code('autorun.brs'); + assert.ok(!/storage_path:\s*"\/[^"]*"/.test(src), + 'storage_path must name a volume — a bare "/path" is outside the writable volumes'); + assert.match(src, /storage_path:\s*StorageRoot\(\)/); +}); + +test('no doubled quotes inside a string literal — BrightScript has no escape sequences', () => { + // The one that cost a player its boot. `"{""width"":"` is not an escaped quote; it is three + // adjacent string literals with no operator between them, and the compiler rejects the WHOLE + // FILE: "ScriptLoadError: Syntax Error. (compile error &h02) in SSD:/autorun.brs(196)". The + // display came up with nothing at all — not a broken feature, no player. A quote in a literal has + // to come from Chr(34). + for (const f of FILES) { + const lines = fs.readFileSync(path.join(DIR, f), 'utf8').split('\n'); + lines.forEach((line, i) => { + if (/^\s*'/.test(line)) return; + // `""` hugged by non-delimiters is a literal trying to contain a quote; `, ""` or `("")` is + // simply an empty string argument and is fine. + assert.ok(!/[^\s(,=]""[^\s),]/.test(line), + `${f}:${i + 1}: a string literal cannot contain a quote — build it with Chr(34)\n ${line.trim()}`); + }); + } +}); + +test('every string literal on a line is closed', () => { + // An odd number of quotes is the same class of failure: it takes the whole script down, and + // nothing here can run it to find out. + for (const f of FILES) { + const lines = fs.readFileSync(path.join(DIR, f), 'utf8').split('\n'); + lines.forEach((line, i) => { + if (/^\s*'/.test(line)) return; + const beforeComment = line.split(/\s'(?=(?:[^"]*"[^"]*")*[^"]*$)/)[0]; + const quotes = (beforeComment.match(/"/g) || []).length; + assert.equal(quotes % 2, 0, `${f}:${i + 1}: unbalanced quotes\n ${line.trim()}`); + }); + } +});