From 30a71c13193bcb5f050543503ef87fe600b16005 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 5 Aug 2026 11:36:18 -0500 Subject: [PATCH] autorun.zip must be STORED and opened with roBrightPackage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A BrightSign consultant ran our v1.9.29-rc2 autorun.zip through BSN.cloud's automated deployment. The archive reached the player and then could not be opened — reported as invalid. Two causes, both ours. 1. COMPRESSION. We built with default deflate. The player bootstrap extracts autozip.brs by itself before any script runs, and roBrightPackage supports a specific set of methods, of which "no compression" is the universally safe one. Both builders now store: scripts/build-autorun-zip.sh passes -0, and the server-side package builder used archiver level 9 — maximum deflate — so EVERY self-update package it produced would have failed the same way, silently and in the field. 2. THE UNPACK API. We used roUnzip; BrightSign's own tooling uses roBrightPackage. Converted in autozip.brs and in the self-update path. This is the failure mode worth naming: a compressed package uploads, downloads and deploys perfectly, then fails to open on the player. It reads as a broken deployment rather than a broken zip, so it gets debugged everywhere except where the bug is. Both builders now ASSERT the property rather than trusting the flag — the build script walks `unzip -v` and refuses a compressed member, and a test walks the local file headers of the server-built package checking method 0. Verified by negative control: re-enabling compression fails the test. Also adopted the shipped volume-discovery pattern in autozip.brs — probe USB1:/SD:/SSD:/FLASH: for the archive instead of guessing two volumes. The unit that drove this port boots from FLASH because its card interface is dead, and extracting to a volume that does not exist fails silently. 1056 pass. Reported by giyokun, who was right about both. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- brightsign/autorun.brs | 8 ++--- brightsign/autozip.brs | 45 +++++++++++++++++--------- scripts/build-autorun-zip.sh | 22 ++++++++++--- server/lib/brightsign-package.js | 8 ++++- server/test/brightsign-package.test.js | 27 ++++++++++++++++ 5 files changed, 85 insertions(+), 25 deletions(-) diff --git a/brightsign/autorun.brs b/brightsign/autorun.brs index abbb232..1b55448 100644 --- a/brightsign/autorun.brs +++ b/brightsign/autorun.brs @@ -202,15 +202,15 @@ Sub ApplyPendingPackage(root As String) print "[st-update] unpacking pending package" - unzip = CreateObject("roUnzip", zipPath$) - if unzip = invalid then - print "[st-update] ERROR: archive unreadable — parking it as .bad" + 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$) return end if - if unzip.DecompressAllFiles(root + "/") <> 0 then + 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$) diff --git a/brightsign/autozip.brs b/brightsign/autozip.brs index 5bc3921..d21c572 100644 --- a/brightsign/autozip.brs +++ b/brightsign/autozip.brs @@ -13,18 +13,32 @@ ' being processed at all. autorun.brs belongs INSIDE the zip, which is where the build script puts ' it (scripts/build-autorun-zip.sh). ' -' Requires BrightSignOS 7.0.60+ (roUnzip). +' 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 +' +' Requires BrightSignOS 7.0.60+. -Function StorageRoot() As String - ' Same reasoning as autorun.brs: a player may be booting from internal flash rather than a - ' card — the only path on a unit whose card interface has failed. Extracting to "SD:/" on such - ' a player writes to a volume that does not exist. - if DoesFileExist("FLASH:/autorun.zip") then return "FLASH:" - return "SD:" +' WHERE the archive is. A player may be fed from USB, a card, an SSD, or internal flash — and the +' unit that drove this port boots from FLASH because its card interface is physically dead. Probing +' 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:"] + for each v in volumes + if DoesFileExist(v + "/autorun.zip") then return v + end for + return "" End Function Sub Main() - root$ = StorageRoot() + root$ = SourceRoot() + if root$ = "" then + print "[st-autozip] no autorun.zip on any volume — nothing to do" + return + end if zipPath$ = root$ + "/autorun.zip" extractPath$ = root$ + "/" donePath$ = root$ + "/autorun.zip.done" @@ -45,17 +59,16 @@ Sub Main() print "[st-autozip] unpacking "; zipPath$ - unzip = CreateObject("roUnzip", zipPath$) - if unzip = invalid then - print "[st-autozip] ERROR: could not open the archive" + package = CreateObject("roBrightPackage", zipPath$) + if package = invalid then + print "[st-autozip] ERROR: could not open the archive — is it STORED (no compression)?" + ' Deliberately NOT marking it done: a corrupt, truncated or wrongly-compressed copy should + ' be retried once someone replaces the file, not silently skipped forever. return end if - result = unzip.DecompressAllFiles(extractPath$) - if result <> 0 then - print "[st-autozip] ERROR: extract failed, code "; result - ' Deliberately NOT marking it done: a corrupt or truncated copy should be retried after - ' someone replaces the file, not silently skipped forever. + if not package.Unpack(extractPath$) then + print "[st-autozip] ERROR: unpack failed" return end if diff --git a/scripts/build-autorun-zip.sh b/scripts/build-autorun-zip.sh index 93ad64a..95edffd 100755 --- a/scripts/build-autorun-zip.sh +++ b/scripts/build-autorun-zip.sh @@ -74,9 +74,16 @@ mkdir -p "$(dirname "$OUT")" rm -f "$OUT" ABS_OUT="$(cd "$(dirname "$OUT")" && pwd)/$(basename "$OUT")" -# -j would flatten any directories we add later; instead cd in and zip '.' so the archive root IS -# the staging root, and future subdirectories keep their structure. -( cd "$STAGE" && zip -q -r -X "$ABS_OUT" . ) +# -0 = STORED, no compression. This is not a size/speed preference, it is a compatibility +# requirement: BrightSign's automated deployment reported our first archive as invalid and could +# not open it. The player bootstrap extracts autozip.brs by itself, before any script runs, and +# roBrightPackage documents a specific set of supported methods — "no compression" is the one that +# is universally safe. A deflated archive copies onto the player perfectly and then fails to open, +# which looks like a broken deployment rather than a broken zip. +# +# -X drops extra attributes; -j would flatten any directories added later, so instead cd in and zip +# '.' so the archive root IS the staging root and future subdirectories keep their structure. +( cd "$STAGE" && zip -q -r -X -0 "$ABS_OUT" . ) echo " built $OUT" unzip -l "$OUT" | sed 's/^/ /' @@ -94,4 +101,11 @@ if ! unzip -l "$OUT" | grep -qE ' autozip\.brs$'; then echo "ERROR: autozip.brs is missing — nothing would unpack this archive." >&2 exit 1 fi -echo " root-level layout verified" +# Prove every entry is STORED. A single deflated member is enough to make the archive unopenable +# on the player, and it is invisible until a deployment fails in the field. +if unzip -v "$OUT" | awk '$1 ~ /^[0-9]+$/ && $2 != "Stored" {print $2}' | grep -q .; then + echo "ERROR: archive contains compressed members; BrightSign needs it stored (zip -0)." >&2 + unzip -v "$OUT" | sed 's/^/ /' >&2 + exit 1 +fi +echo " root-level layout verified, all members stored" diff --git a/server/lib/brightsign-package.js b/server/lib/brightsign-package.js index c4bd647..bfda59c 100644 --- a/server/lib/brightsign-package.js +++ b/server/lib/brightsign-package.js @@ -67,7 +67,13 @@ function buildZip() { return new Promise((resolve, reject) => { const dir = brightsignDir(); const chunks = []; - const archive = archiver('zip', { zlib: { level: 9 } }); + // STORED, no compression — not a size/speed choice. A player could not open our first + // deflated archive: BrightSign's automated deployment copied it across and then reported it + // invalid. The bootstrap extracts autozip.brs before any script runs, and roBrightPackage + // supports a specific set of methods, of which "no compression" is the universally safe one. + // A compressed package deploys perfectly and then fails to open, which reads as a broken + // deployment rather than a broken zip. + const archive = archiver('zip', { store: true }); archive.on('data', (c) => chunks.push(c)); archive.on('error', reject); diff --git a/server/test/brightsign-package.test.js b/server/test/brightsign-package.test.js index 02753d1..126915d 100644 --- a/server/test/brightsign-package.test.js +++ b/server/test/brightsign-package.test.js @@ -81,3 +81,30 @@ test('the version comes from VERSION, so the manifest matches the release it shi const pkg = await pkgLib.getPackage(); assert.equal(pkg.version, expected); }); + +// A BrightSign consultant's automated deployment copied our first autorun.zip onto a player and +// then reported it invalid. Two causes: the archive was DEFLATED, and we opened it with roUnzip +// rather than roBrightPackage. The player bootstrap extracts autozip.brs by itself before any +// script runs, and roBrightPackage supports a specific set of methods — "no compression" is the +// universally safe one. +// +// This is the failure mode that hurts: a compressed package uploads, downloads and deploys +// perfectly, then fails to open on the player. It reads as a broken deployment, not a broken zip, +// so it gets debugged everywhere except where the bug is. +test('THE DEPLOYMENT BUG: every member of the package is STORED, never deflated', async () => { + const pkg = await pkgLib.getPackage(); + const buf = Buffer.isBuffer(pkg) ? pkg : (pkg && (pkg.buffer || pkg.bytes || pkg.zip)); + assert.ok(Buffer.isBuffer(buf), 'getPackage must yield the archive bytes'); + + // Walk the local file headers: signature PK\x03\x04, compression method at offset +8. + let found = 0; + for (let i = 0; i + 30 <= buf.length; i++) { + if (buf.readUInt32LE(i) !== 0x04034b50) continue; + const method = buf.readUInt16LE(i + 8); + const nameLen = buf.readUInt16LE(i + 26); + const name = buf.slice(i + 30, i + 30 + nameLen).toString(); + assert.equal(method, 0, `${name} is compressed (method ${method}); the player cannot open it`); + found++; + } + assert.ok(found > 0, 'no entries found — the walk itself is wrong, not the archive'); +});