Skip to content

feat(mock): serve file downloads - honour _SASJS_WEBOUT_HEADERS and a binary body - #1494

Merged
allanbowe merged 27 commits into
mainfrom
feat/mock-webout-headers
Oct 25, 2026
Merged

allanbowe merged 27 commits into
mainfrom
feat/mock-webout-headers

Conversation

@sasjs-dev

@sasjs-dev sasjs-dev Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

feat(mock): serve file downloads - honour _SASJS_WEBOUT_HEADERS and a binary body

Closes #1493.

What was wrong

A streamConfig.mockWeb site answered every service with text/plain. The service worker framed every /SASjsApi/stp/execute reply with new Response(body, { headers: { 'content-type': 'text/plain' } }) and ignored the headers file, so a service could write Content-type / Content-Disposition into _SASJS_WEBOUT_HEADERS and both were discarded - the JS counterpart of %mp_streamfile was dead.

A byte body had nowhere to go either: the drive holds strings, so fs.writeFileSync(weboutPath, someBytes) stored "[object Uint8Array]".

One correction to the issue as filed: runtime-core.js does already read the headers file back and return it from run() (there is a test for it). The header was being handed to the worker and dropped there, so the miss was in sw.js plus the missing byte path.

What this does

sw.js applies the content type and content disposition a service declared, so a mock of a streaming service produces a real download under the name the service chose. These are the same headers the server applies (Execution.ts -> extractHeaders), and only these two: cache-control, cookies and friends have no meaning on a static host, so they are not applied. A value that is not printable ASCII is dropped rather than handed to Response, which throws on a malformed header and would turn a download into a 500.

runtime-core.js takes a byte body, the way the server's contract does:

  • fs.writeFileSync accepts an ArrayBuffer, a TypedArray or a Buffer and stores it as base64 behind a marker in the drive (which is persisted as JSON), so the representation survives a snapshot round trip and a restored drive still knows which entries are bytes;
  • Buffer is available to the sandbox, as it is to a JS service on the server, so Buffer.from(bytes) and Buffer.from(base64, 'base64') both work;
  • readFileSync returns such a file as bytes and statSync reports its byte length;
  • run() reports binary and the worker decodes the body for the response, defaulting to application/octet-stream when the service named no content type;
  • GET /SASjsApi/drive/file serves a byte file as bytes too, so the representation is consistent in both directions;
  • an assigned _webout still replaces a byte body, exactly as the generated program's own write does on the server.

A debug reply stays text: the body is the webout framed with the log, so honouring an attachment there would put the frame in the file.

runtime-core.js closes the node gaps the byte path exposed, each of which made a working service produce different bytes here:

  • Buffer.alloc(size, fill) repeats the fill - a byte, a string in its encoding, or a buffer - so a padded or fixed-width payload comes out as it does on the server;
  • Buffer#slice is a view over the same memory, as node's is, so editing a slice reaches the original;
  • Buffer.from(text, 'ascii') encodes as latin1 and keeps the high bit, which node masks only when it decodes;
  • a Buffer method the shim does not carry (write, equals, indexOf, readUInt32BE, ...) throws Buffer.write is not implemented in the browser host rather than x.write is not a function;
  • a drive entry that is not valid base64 is refused wherever it is read - readFileSync, statSync, the drive total and the worker that serves it all raise the same 500 "Corrupt binary entry on the drive", where before one read it as empty bytes and another answered 500;
  • statSync and the drive total measure a byte entry from its validated base64 string (three bytes per four characters, less the padding) rather than by decoding it, so a multi-megabyte seeded archive is not materialised to answer a stat, and the size cannot disagree with the bytes a read returns, and the measure is cached against the stored string, so a repeated stats() re-measures only what changed;
  • writeFileSync/writeFile read the options argument: an encoding is applied, and { flag: 'a' } appends, so a service that streams a file in chunks keeps the whole file. 'w' replaces; any other flag is refused by name, because the drive has no file descriptors to open.

mockSite.ts seeds a project file that is not text - an archive, an image, an xlsx - behind the byte marker instead of leaving it off the drive, so a service that downloads one of those works in the mock as it works on the server. Text beginning with U+0000 is escaped, so it cannot be mistaken for the marker.

The cost is bounded rather than hidden: each such file is base64 in the seed, so vfs-seed.json and the worker's memory carry 1.33x its bytes. The bound has two parts: 10 MB per byte file (MAX_BINARY_SEED_BYTES) and 25 MB across all of them (MAX_SEED_BYTES, spent in listing order), because one large artefact or several nearly-large ones make the generated site unloadable either way. A file over either bound is left off the drive and named in the build warning with its size and which bound it hit - the old behaviour for every non-text file, now applied only where size makes it necessary. Text keeps no bound: a program folder is text by nature and the drive carried it before this change.

persist.js takes the drive total from the worker rather than summing the stored strings, and rejects when the worker cannot report - a caller destructuring { files, bytes } gets a failure, not NaN.

A mock of a download

fs.writeFileSync(
  _SASJS_WEBOUT_HEADERS,
  'Content-type: application/zip\nContent-Disposition: attachment;filename="pack.zip"'
)
fs.writeFileSync(weboutPath, Buffer.from(zipBytes))   // or Buffer.from(b64, 'base64')

The service source is the same shape as the server-side JS service for the same job; nothing browser-specific to learn. Note that fs is already declared by the generated wrapper, so a service uses it rather than redeclaring it - the same rule as on the server.

Verification

Unit: 110 tests in the mock-site specs; the mocked suite (90 suites, 764 tests) passes apart from the two environment-dependent specs, which are excluded from the run below (generateDocs.spec needs the doxy asset tree, version.spec needs git tags).

Coverage, measured on the same suite set against origin/main in a worktree (both runs with generateDocs.spec, which needs the doxy asset tree, and version.spec, which needs git tags, excluded):

scope statements branches functions lines
total 75.48 -> 77.10 63.31 -> 66.16 76.11 -> 77.24 76.29 -> 77.83
assets/runtime-core.js 85.76 -> 92.87 66.86 -> 82.13 83.61 -> 91.43 88.74 -> 95.10
assets/sw.js 89.90 -> 97.78 72.50 -> 85.86 93.75 -> 95.24 91.30 -> 98.37
assets/persist.js 81.29 -> 81.56 73.17 -> 75.56 80.00 -> 79.49 86.07 -> 86.07
mockSite.ts 97.06 -> 97.37 85.07 -> 86.67 100 -> 100 97.62 -> 97.90
compile/compileSingleFile.ts 97.92 -> 97.92 88.00 -> 88.00 100 -> 100 100 -> 100

compileSingleFile.ts shows as "reduced coverage" in the bot's comment on this PR. It is unchanged at 97.92/88/100/100 on this measurement, and the same line appears on #1492, so it is an artefact of the bot's comparison basis rather than something this branch moved. Nothing in this branch takes a file down.

persist.js moves the other way for the same reason in reverse: the page-side stats() no longer sums the stored strings, so two covered statements leave the file and the percentage falls by a fraction of a point while the uncovered statements stay the same count. The one measure now comes from the worker, which owns the storage format.

tsc --noEmit and prettier --check are clean.

Live, in Chrome with a real service worker over localhost, a hand-assembled mock site whose service writes a zip:

request result
getauditfile (bytes + headers) 200, content-type: application/zip, content-disposition: attachment;filename="pack.zip", 268 bytes, signature PK\x03\x04
getusers (text + headers) 200, content-type: text/csv, content-disposition: attachment;filename="users.csv", body id,name\n1,Ann\n intact

The 268 bytes written by the browser were decoded with Python zipfile after the round trip: testzip() reports no bad member and both members read back intact, so the body is a valid archive and not a re-encoded string.

The live run also caught a real trap in my first harness service (const fs = require('fs') in the service, which the wrapper already declares) - noted above for the next person writing a mock.

A browser-hosted site answered every service with text/plain, whatever the
service asked for. The runtime already read back the headers file the preamble
hands a service as _SASJS_WEBOUT_HEADERS, but the service worker dropped them,
and a byte body had nowhere to go: the drive holds strings, so a service
writing bytes to weboutPath got "[object Uint8Array]".

The service worker now applies the content type and content disposition a
service declares, so a mock of a streaming service produces a real download
under the name the service chose - the same headers the server applies in
Execution.ts, and the ones %mp_streamfile writes on the SAS side.

The runtime takes a byte body too:

* fs.writeFileSync accepts an ArrayBuffer, a TypedArray or a Buffer, and stores
  it as base64 behind a marker in the drive (which is persisted as JSON), so
  the representation survives a snapshot round trip;
* a Buffer is available to the sandbox, as it is to a JS service on the server,
  so Buffer.from(bytes) and Buffer.from(base64, 'base64') both work;
* readFileSync returns such a file as bytes, and statSync reports its byte
  length;
* run() reports `binary`, and the worker decodes the body for the response
  (octet-stream when the service named no content type);
* an assigned _webout still replaces a byte body, as the generated program's
  own write does on the server.

A debug reply stays text: the body is the webout framed with the log, so
honouring an attachment there would put the frame in the file.

Verified in a browser against a mock site with a service worker: a service
writing a zip returns 200, content-type application/zip,
content-disposition attachment;filename="pack.zip", and 268 bytes that Python's
zipfile reads with no bad member; a text/csv service returns its body intact
under the name it declared.
sasjs-dev Bot pushed a commit to sasjs/cli.sasjs.io that referenced this pull request Oct 24, 2026
A mock of a streaming service was the one thing a browser build could not do:
the download headers were ignored and a byte body had nowhere to go. Say how a
service declares a file response, in text and in bytes, and what the host does
with the headers it was given.

Pairs with sasjs/cli#1494.
The worker has one function that turns the base64 a byte body travels as back
into a Uint8Array; "bytes" read like the body itself.
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Coverage report

Total coverage

Status Category Percentage Covered / Total
🟡 Statements 78.85% (+1.5% 🔼) 4608/5844
🟡 Branches 67.39% (+2.68% 🔼) 1988/2950
🟡 Functions 78.04% (+1.2% 🔼) 899/1152
🟢 Lines 86.16% 10910/12662

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status Filename Statements Branches Functions Lines
🟢 src/commands/compile/compileSingleFile.ts 95.83% (-2.08% 🔻) 84% (-4% 🔻) 100% 100%
🟢 src/commands/build/internal/mockSite/mockSite.ts 97.45% 84.81% 100% 98.18%
🟢 src/commands/build/internal/mockSite/assets/persist.js 81.56% 75.56% (+2.38% 🔼) 79.49% 97.65%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from e9bdf03

Ten more cases over the download contract, and the two runtime changes they
found:

* Buffer.from takes a latin1/binary string as latin1, not as utf8, and
  Buffer.byteLength measures a string instead of reporting 0 for it;
* an ArrayBuffer and a view over one are bytes, as a Uint8Array is;
* a byte file reads back as bytes, reports its length from statSync, and
  decodes to hex or base64;
* a directory stats, a missing copy source and a missing stat report the way
  node does;
* a value that cannot be serialised still lands in the log rather than failing
  the run;
* the worker normalises header names and ignores a line that is not a header,
  serves a declared type with an empty body, answers the session route and an
  unimplemented route, answers 404 for a drive file it does not hold, and
  leaves another origin's request alone.

Measured on the mocked suite, base (origin/main) against this branch:

  total             77.22 -> 77.98 statements, 64.60 -> 65.80 branches
  runtime-core.js   85.75 -> 90.90 statements, 66.85 -> 74.07 branches
  sw.js             89.89 -> 97.72 statements, 72.50 -> 86.45 branches
  compileSingleFile 97.91 -> 97.91 (unchanged)

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at ef1cb1f (two commits on top of the merged #1492: the runtime/sw hosts and their specs) against the sasjs/server contract the runtime claims to mirror. All 11 new specs pass locally against the PR head (67/67 for the mockSite suite), and both node-22 CI runs are green. The approach checks out: byte writes are stored as base64 behind a NUL marker so the JSON-persisted drive stays self-describing, the sandbox gains a Buffer built on the same WebCrypto-era primitives the drive already uses, sw.js applies only content-type/content-disposition and drops non-printable values rather than letting Response throw, and a byte body with no declared type degrades to application/octet-stream. find-file and drive-file reads now return {contents, binary} consistently and the only consumer (sw.js:168) was updated to match. Conventional-commit form is correct for the release pipeline (feat(mock) is the release-driving type).

  • src/commands/build/internal/mockSite/assets/runtime-core.js:190 - "byteLength: (value) => (asBytes(value) || { length: 0 }).length" returns 0 for a string: asBytes() only recognises ArrayBuffer/TypedArray/Buffer/Array, so Buffer.byteLength('some text') is 0 where node returns the utf8 byte count (verified by execution against the head: run({code: "_webout = String(Buffer.byteLength('hello'))"}) gives "0"; Buffer.byteLength(Buffer.from('hello')) correctly gives 5). A service that sizes a download with Buffer.byteLength(text) silently emits a zero-length body. Make the string path encode: (asBytes(value) || utf8Encoder.encode(String(value))).length.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:151 - asBuffer() hangs the custom toString on the Uint8Array instance, so any derived view loses it: buf.subarray(0,2).toString('base64') returns "80,75" (the default Uint8Array toString) where node returns "UEs=" (verified against the head). The same holds for .slice(). The runtime's own readFileSync hands back a BUFFERS.from(...) product, so a service that chunks a file with subarray(i, j).toString('base64') - a common streaming shape - produces CSV numbers instead of base64. Either define the decoder on the prototype (BUFFERS.from could return an object backed by a Uint8Array with subarray/slice re-wrapped through asBuffer) or re-wrap subarray/slice results.

2 findings above for review.

A Buffer's own toString is the decoder, so a view taken from it lost that and
fell back to Uint8Array's comma-separated one: a service streaming a file in
chunks read `buf.subarray(i, j).toString('base64')` as "80,75" instead of
"UEs=". Views are now Buffers too, as they are in node, and Buffer.isBuffer
agrees.

Found in review of #1494 (runtime-core.js:151), with a spec for the chunking
shape.
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Both findings addressed - thanks, the second one is a genuine bug I would have shipped.

runtime-core.js:190 - Buffer.byteLength of a string returned 0. Fixed in b57bbb7 (pushed before this review landed at ef1cb1f, so the review predates the fix):

  byteLength(value, encoding) {
    if (typeof value === 'string') return BUFFERS.from(value, encoding).length
    return (asBytes(value) || { length: 0 }).length
  }

Verified against the head just now: Buffer.byteLength('hello') is 5, Buffer.byteLength(Buffer.from([1, 2])) is 2, and the spec asserts Buffer.byteLength('ab') === 2.

runtime-core.js:151 - derived views lost the decoder. Fixed in 1afdcd5. subarray and slice now return decorated views, so a view of a Buffer is a Buffer, as in node:

byteLength(hello): 5
chunk base64: UEs=          # buf.subarray(0, 2).toString('base64')
slice base64: UEs=          # buf.slice(0, 2).toString('base64')
buffer.isBuffer(chunk): true

The spec decodes a chunk of a Buffer, as a service streaming a file does covers the chunking shape end to end: a file read back from the drive, subarray(0, 2) and slice(0, 2) decoded as base64, subarray(2) as hex, Buffer.isBuffer on a chunk.

Coverage on this branch, measured against origin/main on the same suite set (the two environment-dependent specs excluded): total 77.22 -> 77.98 statements / 64.60 -> 65.80 branches, runtime-core.js 85.75 -> 90.90 / 66.85 -> 74.07, sw.js 89.89 -> 97.72 / 72.50 -> 86.45. Nothing goes down; compile/compileSingleFile.ts is unchanged at 97.91/88/100/100 (it is flagged in the bot's report, but the same line appears on the merged #1492 and the file is untouched here).

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at b57bbb7 (three commits on top of merged #1492; the new commit adds the byte and header spec coverage) against the sasjs/server contract the runtime mirrors. All 49 mockSite specs pass locally against the head, both node-22 CI runs are green, and the Buffer.byteLength finding from the review at ef1cb1f is resolved at this head.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:148 - carried over from the review at ef1cb1f, still present: asBuffer() hangs the custom toString on the Uint8Array instance, so derived views lose it. fs.readFileSync(file).subarray(0, 2).toString('base64') returns "80,75" where node returns "UEs=" (verified by execution against the head), and the same holds for slice(). readFileSync hands back a BUFFERS.from(...) product, so the common streaming shape chunk.toString('base64') after a subarray/slice emits CSV numbers instead of base64. Either put the decoder on a shared prototype or re-wrap subarray/slice results through asBuffer.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:166 - hex decoding diverges from node on invalid input: Buffer.from('zz', 'hex') produces a one-byte zero buffer ("AA==") because parseInt('zz', 16) is NaN and Uint8Array.from([NaN]) coerces it to 0, where node returns an empty buffer (verified: node gives "" for the same call). A service passing malformed hex gets a zero-filled body node would never produce. Stop at the first invalid pair the way node does, rather than coercing NaN to 0.

2 findings above for review.

Buffer.from('zz', 'hex') produced a zero byte, because parseInt('zz', 16) is
NaN and Uint8Array.from coerces that to 0 - node stops at the first pair that
is not hex and returns nothing. base64 had the mirror problem in the other
direction: atob throws on junk and on an unpadded lone character, where node
ignores what is not base64 and drops the character it cannot decode.

Both decoders now follow node: hex stops at the first bad pair and ignores a
trailing nibble, base64 ignores non-base64 characters, drops a lone trailing
one, and tolerates missing padding.

Found in review of #1494 (runtime-core.js:166).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Both of this round's points, plus a note on the carried-over one.

runtime-core.js:148 (subarray/slice) - fixed in 1afdcd5, which this review predates (it reviewed b57bbb7). Views now come back decorated, so a view of a Buffer is a Buffer:

chunk: UEs=          # Buffer.from([80,75,3,4]).subarray(0,2).toString('base64')

The spec decodes a chunk of a Buffer, as a service streaming a file does covers it end to end from a file read off the drive, including Buffer.isBuffer(chunk).

runtime-core.js:166 (hex on invalid input) - fixed in 8eb1e44, plus the mirror problem in base64, which was throwing instead of tolerating:

hex zz len: 0        # node: 0
hex abc: ab          # node: 0xab, trailing nibble ignored
b64 zz len: 1        # node: 1
b64 A len: 0         # node: 0
b64 UEsDBA: 4        # node: 4, unpadded

Hex now stops at the first pair that is not hex (no NaN coercion), and base64 ignores non-base64 characters, drops a lone trailing character, and accepts missing padding - each compared against node before writing the assertion. The spec decodes hex and base64 the way node does, including what it rejects pins all eight cases.

@sasjs-dev sasjs-dev Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at 1afdcd5 (four commits on top of merged #1492) against the node Buffer semantics the runtime claims to mirror; scope of this re-review is the delta from the b57bbb7 review plus carried findings. The decorate() rework resolves the subarray/slice finding from that review - verified by execution against the head: read.subarray(0,2).toString('base64'), slice().toString('base64'), subarray(2).toString('hex') and Buffer.isBuffer(chunk) all return what node returns, and the new spec covers the streaming shape. Both node-22 CI legs are green at this head.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:180 - carried over from the review at b57bbb7, still present: hex decoding diverges from node on invalid input. "Uint8Array.from(value.match(/../g) || [], (pair) => parseInt(pair, 16))" coerces a NaN from a bad pair to 0, so Buffer.from('zz', 'hex') yields "AA==" (one zero byte) where node yields an empty buffer, and Buffer.from('504bzz', 'hex') yields "UEsA" (a trailing 0x00) where node stops at the invalid pair and yields "UEs=" (both verified by execution against the head). A service passing malformed hex gets a zero-padded body node would never produce. Validate each pair with a hex-character test and stop at the first invalid one, the way node does.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:149 - the toString range path passes negative start/end straight to Uint8Array.subarray, whose from-end semantics differ from node's clamping. Verified against the head and against node: buf.toString('hex', -1) returns "04" (last byte) where node clamps a negative start to 0 and returns "01020304"; buf.toString('hex', 0, -1) returns "010203" where node returns "" (negative end clamps to 0, and an end below start yields an empty result). A service reusing node's clamping idioms gets different bytes. Clamp before the subarray call: treat a negative start as 0 and a negative end as 0 (node returns "" whenever the clamped end is below the clamped start).

  • src/commands/build/internal/mockSite/assets/runtime-core.js:197 - Buffer.concat does not reject non-buffer entries the way node does: "asBytes(item) || new Uint8Array(0)" silently turns a string in the list into an empty part, so Buffer.concat(['a']) returns a zero-length buffer where node throws TypeError ("list[0] must be an instance of Buffer or Uint8Array"). A service that passes the wrong shape gets a silently truncated body instead of an error. Throw the same TypeError for entries asBytes() does not recognise.

3 findings above for review.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff: runtime-core.js, sw.js and both spec files.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:214 (BUFFERS.concat): (list || []).map((item) => asBytes(item) || new Uint8Array(0)) substitutes an empty part for every item that is not byte-like, so Buffer.concat(['ab', buf]) returns just buf here while node throws a TypeError. The same lenience sits in BUFFERS.byteLength (around :228), which returns 0 for an unusable value. A service carrying that mistake passes in the mock and fails on the server - throw the TypeError that BUFFERS.from already throws at :210.
  • src/commands/build/internal/mockSite/assets/runtime-core.js:171 (decorate): view[BUFFER_MARK] = true is an own enumerable property on a plain Uint8Array, so JSON.stringify of a decorated buffer yields {"0":80,"1":75,...,"__sasjsBuffer":true} where node yields {"type":"Buffer","data":[80,75]}. A service that serialises a buffer into a JSON body gets a different response from the mock than from the server - define the mark non-enumerable and add a toJSON returning node's {type,data} shape.

2 findings above for review.

Three divergences from node's Buffer, each of which changes a response a
service would get on the server:

* toString(encoding, start, end) passed a negative start or end straight to
  Uint8Array.subarray, which counts from the end: buf.toString('hex', -1) read
  the last byte where node clamps the start to 0 and returns the whole buffer.
  The range is clamped now, and an end below the start yields nothing.
* the buffer methods and the mark were enumerable own properties, so
  JSON.stringify carried "__sasjsBuffer": true and Object.keys listed them.
  They are non-enumerable, and toJSON returns node's {type: 'Buffer', data}
  shape.
* Buffer.concat and Buffer.byteLength substituted an empty value for an entry
  node refuses, so a service passing a string silently emitted a zero-length
  body instead of failing the way it would on the server. Both throw node's
  TypeError, with its wording.

Found in review of #1494 (runtime-core.js:149, :197, :214, :228).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Five findings across the two reviews, all fixed in 66b3336 (which the reviews predate - they were taken at 1afdcd5 and b57bbb7). Verified against node before writing each assertion, by running the same call against the head:

json:           {"type":"Buffer","data":[80,75]}      # was {"0":80,"1":75,"__sasjsBuffer":true}
keys:           0                                     # was 0,toString,subarray,slice,toJSON
hex(-1):        "01020304"                            # was "04"
hex(0,-1):      ""                                    # was "010203"
concat str:     TypeError: The "list[0]" argument must be an instance of Buffer or Uint8Array. Received type string ("ab")
byteLength {}:  TypeError: The "string" argument must be of type string or an instance of Buffer or ArrayBuffer. Received an instance of Object
hex zz:         0                                     # was 1 (a zero byte)
  • hex on invalid input (:166/:180) - fixed in 8eb1e44, with the mirror problem in base64: node stops at the first bad hex pair, ignores non-base64 characters, drops a lone trailing one and tolerates missing padding; ours coerced NaN to 0 and threw on junk.
  • toString range clamping (:149) - the range is clamped the way node clamps it now: a negative start is 0, a negative end is 0, and an end at or below the start returns ''.
  • concat / byteLength (:197, :214, :228) - both take only what node takes, and throw node's TypeError with its wording rather than substituting an empty value.
  • toJSON and the enumerable mark (:171) - the mark and the four methods are non-enumerable, and toJSON returns {type: 'Buffer', data}.

Specs pin each one: decodes hex and base64 the way node does, including what it rejects, clamps a toString range the way node does, serialises a Buffer the way node does, with no marker in the JSON, refuses what node refuses, with the same complaint. The mock-site suite is 82 tests.

One note on scope: this is a Buffer shim for the writes a streaming service makes, not a re-implementation of node's Buffer. If a service leans on something outside from/alloc/concat/isBuffer/byteLength/toString/subarray/slice/toJSON, the mock should say so plainly rather than pretend - flag it and it becomes another finding's worth of work.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at 66b3336: mock-site runtime binary support (base64 drive storage, a Buffer shim) and service-worker webout/content-disposition handling. Specs are thorough and the header-parsing/filtering is sound.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:362 (writeFileSync): a plain Array is accepted as bytes through asBytes' if (Array.isArray(data)) return Uint8Array.from(data) (:138), but node's fs.writeFileSync throws The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Array. That is exactly the divergence the asBufferInput comment (:127) says the mock avoids - a service writing an array passes here and fails on the server. Use asBufferInput on the write path and leave string/null to String().

  • runtime-core.js:849 (api.read): the return shape changed from { contents: <string> } to { contents, binary }, with contents base64-encoded for byte files. sw.js was updated; confirm no other caller of the runtime read exists (main-thread drive UI or glue code) before release - an un-updated one gets an object where it expects a string, or base64 where it expects text.

  • Same gap in code the diff does not touch: statSync (:404) now reports byte length for a binary entry, but listFolder/buildTree and the async fs.readFile still read FILES directly, so they report the base64 length as size and hand back the raw "\u0000base64:"-prefixed string. Route every FILES read through the same decode/measure helpers.

  • sw.js:43 (toBytes): atob throws on malformed base64, so a drive restored from a hand-edited or truncated snapshot turns a download into an unhandled rejection instead of an error Response. Not reachable from the runtime's own encoder; worth a guard if snapshots are meant to be user-editable.

  • Node-parity wording: the Buffer.concat / Buffer.byteLength messages ("must be an instance of Buffer or Uint8Array", Received type string ("ab")) differ from node's ("must be one of type Buffer or Uint8Array", received single-quoted), and runtime-core.spec.ts asserts the mock's text - the error-parity claim in the new header comment is not met for these two.

CI at 66b3336 is still pending with no check contexts reported, so the pipeline gate is unverified. No package.json, lockfile, .npmrc or CI changes in the diff; the commit/PR title is a correct Conventional Commit for a user-visible feature.

5 findings above for review.

fs.writeFileSync still accepted a plain Array (and any other value) through
asBytes and wrote its string form, so a service that passes "[1,2,3]" where
node wants bytes got a body of "1,2,3" here and a TypeError on the server. The
write path takes text or the bytes node takes, and throws node's TypeError for
anything else.

The worker's base64 decode is now guarded: a drive restored from a snapshot
someone edited can hold a damaged entry, and that turned a download into an
unhandled rejection inside the service worker. It answers 500 naming the
corrupt entry instead.

Error text follows node's shape more closely too (received values are quoted
the way node quotes them), and the message that claims parity names the rule
rather than promising node's exact wording, which differs across node majors.

Found in review of #1494 (runtime-core.js:362, sw.js:43).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Three of these are fixed in cc01ead; two are answered with what the code actually does.

Fixed: writeFileSync accepted an Array (:362). Real, and the comment at :127 was promising the opposite. The write path now takes text or the bytes node takes, and throws node's TypeError for anything else:

The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Array
The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received type number (7)

asBytes stays, but only for its one legitimate job: Buffer.from([1, 2, 3]), which node does allow. Spec: refuses a write node would refuse, rather than stringifying it.

Fixed: toBytes on a malformed entry (sw.js:43). Guarded - a damaged entry in a restored snapshot now answers 500 naming the corrupt entry instead of leaving the worker with an unhandled rejection. Spec: answers 500 when a byte entry on the drive is not valid base64.

Fixed, partly: error wording. Received values are quoted the way node quotes them (single quotes) now. I do not claim node's exact text: it differs by node major (must be an instance of ... on 26, must be one of type ... on the wording you quote for 22), so the message names the same rule and the same received value, and the spec asserts the mock's text deliberately. The comment says that rather than "the same wording".

Answered: no other caller of read (:849).

$ grep -rn "\.read(" src/commands/build/internal/mockSite
assets/sw.js:183:      const file = R.read(query._filePath || '')
spec/runtime-core.spec.ts:491:    expect(runtime.read(`${appLoc}/data/a.txt`).contents).toBe('a')

self.SASjsMockRuntime has one consumer (sw.js:23); the page-side API (window.SASjsMock) exposes readAll, writeFiles, snapshot, download, stats, reset - no read. So the shape change has one caller, and the spec at :491 still asserts contents is the text for a text file.

Answered: listFolder / buildTree / async readFile (:404). Checked against the file rather than assumed:

  • listFolder returns { files, folders } - names only, no size field;
  • buildTree returns name, relativePath, absolutePath, isFolder, children - no size field;
  • the shim has no async readFile at all (grep -n "readFile" finds only readFileSync), so nothing hands back a raw \u0000base64:-prefixed string to a service;
  • statSync is the only place a size is reported, and it decodes the base64 length.

dumpAll/takeDirty do carry the stored form deliberately: they are the persistence channel - the page writes those strings into IndexedDB and hands them back at boot, and window.SASjsMock.snapshot() saves a drive image, so a binary entry has to stay in the one representation the drive can round-trip. Decoding on the way out would corrupt the snapshot.

Mock-site suite is 84 tests; CI is green on the previous head and re-running on cc01ead.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full diff (runtime-core.js, sw.js and both specs) - binary webout storage/transfer, webout header passthrough, the Buffer shim, and byte fidelity in the virtual drive.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:642 - const webout = fromFile || (typeof returned === 'string' ? returned : '') falls back to the returned string, but binary on the line above was already fixed to isBinary(stored). If a service writes an empty byte body to weboutPath and the program then returns a string (an early return skips the appended _webout write), the reply carries text with binary: true, so sw.js base64-decodes it in toBytes and answers application/octet-stream with garbage bytes. Derive binary from the value actually used as the body, e.g. only true when fromFile supplied it.
  • src/commands/build/internal/mockSite/assets/runtime-core.js - the default: arm of bytesToString decodes every encoding outside base64/hex/binary/latin1 as utf8. Node accepts utf16le/ucs2 (and base64url) and throws ERR_UNKNOWN_ENCODING for anything else, so a service that streams UTF-16 or base64url content gets silently mangled bytes from the mock while working on the server. Map utf16le/ucs2 to TextDecoder('utf-16le') and throw on an encoding the shim does not implement.

Note, not blocking: fs.writeFileSync now throws on a number or array instead of stringifying it - closer to node, but a behaviour change that will surface in existing mock-only services.

2 findings above for review.

… node's encodings

Two ways the reply could differ from the server:

* a service that wrote an empty byte body and then returned a string was
  answered with that string flagged as binary, so the worker base64-decoded
  text and sent it as application/octet-stream. The flag now follows the value
  actually used as the body.
* bytesToString guessed utf8 for every encoding it did not know. It now takes
  node's set - utf8, base64, base64url, hex, binary/latin1, ascii (with the
  high bit dropped), utf16le/ucs2 (and the ucs-2 alias) - and throws
  "Unknown encoding: <name>" for the rest, as node does. Buffer.from decodes
  the same set.

Found in review of #1494 (runtime-core.js:642 and the bytesToString default).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Both findings fixed in e4bf36e.

binary on an empty byte write (:642). Right - the flag followed the file, not the body. It now follows the value actually used:

const fromFile = isBinary(stored) ? stored.slice(BINARY_PREFIX.length) : stored
const binary = !!fromFile && isBinary(stored)
const webout = fromFile || (typeof returned === 'string' ? returned : '')

Verified: fs.writeFileSync(weboutPath, new Uint8Array(0)); return 'from the return' now gives binary: false, webout: 'from the return', so the worker answers text. Spec: does not call an empty byte write a byte body.

Encodings. bytesToString no longer guesses utf8 for anything it does not recognise. It takes node's set - utf8, base64, base64url, hex, binary/latin1, ascii (high bit dropped, as node decodes it), utf16le/ucs2 (and ucs-2) - and throws Unknown encoding: <name> for the rest. Buffer.from decodes the same set, base64url included. Verified against node:

utf16le 'PK'      -> 50004b00
ucs2 'PK'         -> 50004b00
ascii  'PK'       -> 504b
ascii  [0xc8]     -> 72          # 0x48, high bit dropped
[251,255] -> b64url -> -_8       # and back to fbff
'not-an-encoding' -> Unknown encoding: not-an-encoding

Spec: handles the encodings node handles, and refuses the rest.

On the non-blocking note about writeFileSync throwing. It is a deliberate parity fix, and the blast radius is small: the generated wrapper only writes _webout (a string), the runtime's own session files are strings, and a service that passed a number or an array was already writing something node rejects. If a downstream mock turns out to rely on the old leniency, that mock has a bug the server would expose anyway - and the error now names it at the point of the write rather than shipping "1,2,3" as a body.

Mock-site suite is 86 tests. CI re-running on e4bf36e.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff (runtime-core.js byte/Buffer support, sw.js webout headers, runtime-core.spec.ts, sw.spec.ts). No package.json, lockfile or CI change in the diff, so the hardening checklist does not apply.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:417 (readFileSync): the node shapes now diverge by file kind - a text entry is still returned as a plain string while a byte entry comes back as the decorated Buffer. So fs.readFileSync(f).length on a text file is characters, not bytes; fs.readFileSync(f).toString('base64') returns the raw text where node base64-encodes it; Buffer.isBuffer(fs.readFileSync(f)) is false. The CONTRACT comment added in this PR says the program gets node's fs, so the text path reads as a remaining hole. Returning a decorated buffer for every read (wrapping the string with BUFFERS.from(contents)) would make the contract hold for text too; statSync's size for text entries has the same character-vs-byte skew.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:430 (writeFileSync): byte storage is detected by "stored string starts with \u0000base64:", so a text value whose first characters are a NUL then base64: is read back as bytes, and a seeded or hand-edited entry cannot be distinguished from a genuine byte file. Escaping a leading NUL when storing text (and unescaping on read) would remove the ambiguity.

CI was still pending at review time (no check contexts reported yet), so the green-pipeline requirement is unverified on this head.

2 findings above for review.

…r readFileSync's encoding

A byte entry is recognised by the marker in front of it, so a text value that
began with a NUL and "base64:" was read back as bytes - a seeded or hand-edited
drive could not be told from a genuine byte file either. Text that starts with a
NUL is stored behind its own marker now and comes back unchanged, and a copy
carries the stored representation verbatim instead of re-escaping it.

readFileSync also takes an encoding, as node's does: an encoding returns a
string for a text file or a byte file, and without one a text file still reads
back as text - which is what a service reading a config, a CSV or a service file
expects, and what the drive has always done. That one difference from node is
now stated in the contract comment rather than implied by it.

Found in review of #1494 (runtime-core.js:417, :430).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

One fixed, one answered with a deliberate trade.

Fixed: the marker was ambiguous (:430). A text value that began with a NUL and base64: read back as bytes. Text that starts with a NUL is now stored behind its own marker and comes back unchanged:

const BINARY_PREFIX = '\u0000base64:'
const TEXT_PREFIX = '\u0000text:'

fs.copyFileSync copies the stored representation verbatim rather than re-escaping it, statSync measures the text behind the escape, and api.read takes the escape off. Spec: reads text with an encoding, and never mistakes a marker-like value for bytes - it writes "\u0000base64:AAAA", reads it back unchanged, checks the base64 of the text, the size (12 characters), and that a copy survives.

Answered: readFileSync without an encoding (:417). A text file reads back as text here, and that is deliberate - it is the drive's behaviour before this PR, the existing spec asserts it, and a mock service reading a config, a CSV or its own source expects a string. Making every read a Buffer would break those services to satisfy a claim about fs.

What I did instead:

  • readFileSync(path, encoding) honours the encoding, as node's does, for a text file or a byte file - so a service that needs base64 or hex from a text file writes fs.readFileSync(path, 'base64') and gets what node returns. The gap you identified is closed for code written in node's other idiom.
  • the CONTRACT comment no longer implies node's fs: it states the one difference plainly, so nobody reads the header as a promise the shim does not keep.
  • statSync's size for a text entry is still characters, not bytes. It is the pre-existing text path and I left it alone rather than change a second behaviour under a download fix; say the word and it becomes its own small change.

CI is green on e4bf36e and re-running on f217755.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff: binary/base64 drive representation, the Buffer shim, _SASJS_WEBOUT_HEADERS handling in the service worker, and the accompanying specs. Hardening checklist n/a (no package.json/lockfile/CI change). PR title is conventional (feat(mock)) so release-driving is fine. CI is pending with no contexts reported at this head - green before merge.

  1. src/commands/build/internal/mockSite/assets/runtime-core.js:441 - the options-object form of readFileSync is not accepted. The signature changed from readFileSync(p, opts) to readFileSync(p, encoding) and the new branch is if (encoding) { const how = assertEncoding(encoding) ... }, so a service written for node that calls fs.readFileSync(p, { encoding: 'utf8' }) now throws TypeError: Unknown encoding: [object Object] (assertEncoding does String(encoding).toLowerCase()). Node returns the decoded string. Before this PR the second argument was ignored and text came back, so this is a regression in accepted input for an otherwise ordinary node call. Either accept typeof encoding === 'object' && encoding !== null ? encoding.encoding : encoding, or say in the CONTRACT comment that only the string form is supported.

  2. src/commands/build/internal/mockSite/assets/runtime-core.js:740 - the webout extraction skips the TEXT_PREFIX unescaping that FS.readFileSync applies. const stored = FILES.get(weboutPath) || '' and then const fromFile = isBinary(stored) ? stored.slice(BINARY_PREFIX.length) : stored - a text body that begins with a NUL is stored as TEXT_PREFIX + data by encodeText, but is returned to the host verbatim, so the response body starts with the literal \u0000text: escape. FS.readFileSync gets this right via entryText(contents); result.headers (FILES.get(headersPath) || '') has the same gap. Use entryText(stored) for the non-binary branch so a written value reads back identically whether it comes through readFileSync or through the webout.

Conventional Commits: title has release-driving type and scope; noted as fine, nothing to change.

2 findings above for review.

Two gaps the encoding work left:

* readFileSync(p, { encoding: 'utf8' }) - node's other call shape - reached
  assertEncoding with an object and threw "Unknown encoding: [object Object]",
  where before the argument was ignored and text came back. The encoding is now
  read off the options object when one is given.
* the webout and the headers file were read straight out of the drive, so a
  body that started with a NUL came back to the host still carrying the escape
  the drive stores it behind. Both go through the same unescape
  readFileSync applies, so a written value reads back identically either way.

Found in review of #1494 (runtime-core.js:441, :740).
@sasjs-dev

sasjs-dev Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Both fixed in 7f9116e.

readFileSync(p, { encoding }) (:441). Accepted now - the encoding is taken off the options object when one is passed, so node's other call shape works:

options form: hi        # fs.readFileSync(file, { encoding: 'utf8' })

webout / headers not unescaped (:740). Right - encodeText escaped a NUL-leading text body but the extraction read it verbatim, so the response carried \u0000text:. Both the body and the headers now go through entryText, the same unescape readFileSync applies:

escaped body: "\u0000text:not really escaped"   binary: false
headers:      "Content-type: text/plain"

Spec: takes an encoding on an options object, and hands an escaped body back unchanged covers both.

Mock-site suite is 88 tests, tsc --noEmit clean.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Holistic review of the base...head diff (runtime-core.js, sw.js and both specs). No package.json, lockfile or CI changes, so no hardening or audit items apply; the commit subject is conventional (feat(mock), minor). The node-fidelity claims were checked against node's actual encode/decode behaviour and the added specs cover the main paths.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:517 - : entryText(contents).length counts UTF-16 code units, while node's statSync().size is UTF-8 bytes. fs.statSync(f).size on a file holding a single 'e-acute' returns 1 here and 2 on the server (a 4-byte emoji: 2 vs 4), so a service that sizes a text body or derives a Content-Length from stat disagrees with the mock. Use the encoded byte length for the text branch, e.g. utf8Encoder.encode(entryText(contents)).length.
  • src/commands/build/internal/mockSite/assets/sw.js:91 - if (!/^[\x20-\x7E]+$/.test(value)) return drops the whole header when any byte is non-ASCII, so a service writing a raw UTF-8 filename (Content-Disposition: attachment;filename="resume-accent.csv", accented) loses the disposition entirely and the browser saves under the URL name - whereas the server passes the header through. Drop only values Response would actually reject, or ASCII-fold/percent-encode the value instead of discarding the header.

2 findings above for review.

* statSync reported a text entry's size in UTF-16 code units, so a file holding
  one accented character was 1 here and 2 on the server. It reports utf8 bytes
  now, as node does.
* the service worker dropped a whole response header when any character was
  outside printable ASCII, which lost the filename from an accented
  Content-Disposition. Response takes a byte string, so everything up to U+00FF
  is accepted; only a value above that (or a control character) is dropped, and
  the body is still served.

Found in review of #1494 (runtime-core.js:517, sw.js:91).

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full current diff at 23d7e85 (8 files: runtime-core byte/Buffer shim, persist stats via the worker, sw.js response headers and binary bodies, mockSite byte seeding, 4 spec files). Re-review at a moved head; this supersedes the earlier one.

  • src/commands/build/internal/mockSite/assets/sw.js:767 (HEADER_VALUE): the allowed shape is [\x20-\x7E\xA0-\xFF], which excludes HTAB (\x09). HTTP allows a tab inside a field value and the real server passes it through, so a service writing Content-Disposition: attachment;\tfilename=... has that header silently dropped and gets the default delivery. \x09 belongs in the set next to \x20.
  • CI: the head reports no check contexts yet (CI_STATE=pending), so nothing here is backed by a green run - the new runtime/sw/persist specs are the part most worth seeing pass before merge.

No blocking issues.

@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Correct - and confirmed on node v26.5.1 before changing it:

Buffer.from('\u00e9\u00ff', 'ascii').toString('hex')   node: e9ff
Buffer.from([0xc8]).toString('ascii').charCodeAt(0)     node: 0x48

The encode arm masks nothing now (it shares the latin1 arm), and the decode arm in bytesToString keeps its mask, which is what node does in each direction. The spec that only encoded 'PK' now encodes \u00e9\u00ff and asserts e9ff beside the 0xc8 -> 0x48 decode case. Fixed in 51b11b6.

The printable set excluded HTAB, so a service writing
`Content-Disposition: attachment;\tfilename=...` had the header dropped and the
reply fell back to default delivery - where the server passes the tab through.
CR, LF and NUL stay out of the set, so a value that would make the Response
constructor throw is still dropped rather than turning a download into a 500.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 911c48a. HTAB joins the printable set (/^[\x09\x20-\x7E\xA0-\xFF]+$/), so a value with a tab inside it is applied rather than dropped; CR, LF and NUL stay out, because those are what make the Response constructor throw and turn a download into a 500. The new spec writes Content-Disposition: attachment;\tfilename="a.zip" and asserts the header comes back with the tab in it.

Mock-site suite 101 tests in 5 suites.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at 911c48a (1896 diff lines: the binary/Buffer layer and drive representation in runtime-core.js, stats moved to the worker in persist.js, header + byte responses in sw.js, build-side seeding in mockSite.ts, and ~800 lines of specs). No package.json, lockfile or CI changes in this diff, so the hardening checklist does not apply; the pipeline is still pending with no contexts reported, so green is unconfirmed.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:116 (base64ToBytes) swallows a decode failure - catch (e) { return new Uint8Array(0) } - while sw.js toBytes reports the same corrupt entry as a 500 "Corrupt binary entry on the drive: not valid base64". A service that streams a corrupt or hand-edited \u0000base64: project or drive file therefore downloads an empty or wrong body with HTTP 200, and the error path added on the drive/file route never fires for it. Make the two agree: throw from base64ToBytes (or from readFileSync when the entry fails to decode) so a corrupt entry is reported wherever it is read, not only when it is served directly.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:176 (base64Size) measures a byte entry from the raw stored base64 string, but entryBytes at :208 decodes through base64ToBytes, which strips non-base64 characters first. For an entry carrying whitespace or junk - a snapshot edited by hand, or written by something other than bytesToBase64 - statSync().size and the byte count readFileSync hands back disagree, and the drive total follows the size. Cheapest fix is to derive the size from base64ToBytes(...).length the way the bytes are derived, so size and contents cannot diverge.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:390 (allocBytes) starts with new Uint8Array(Number(size) || 0), so a non-numeric or NaN size yields a zero-length buffer where node throws ERR_INVALID_ARG_TYPE. Silent-empty instead of node's throw is the trade the write path deliberately refuses two screens above; low priority, but the same argument applies.

3 findings above for review.

A byte entry's base64 was decoded leniently by readFileSync and strictly by the
worker's own serving path, so the same corrupt entry handed a service an empty
body with HTTP 200 while a direct download answered 500. Both now go through
decodeEntry, which refuses the entry with that same message and status, and
statSync measures the bytes decodeEntry hands back rather than the stored
string - so a size can no longer disagree with the contents it describes. The
drive total reads every entry, so it refuses one too, and the worker answers the
page's stats request with the reason instead of letting the page time out.

Buffer.alloc also refuses a size node refuses (a non-number, a negative, NaN,
Infinity) rather than returning an empty buffer, while a fractional size is
truncated as node truncates it.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

All three are in a0612a5.

Corrupt byte entry (base64ToBytes, :116). The strict decode is now a separate function, decodeEntry, used by every drive read: readFileSync, statSync (through entrySize), the page-side aggregate, and the worker's own read when it serves a file. It refuses the entry with the message and status the serving path already used, so a corrupt entry no longer reads as an empty body in one place and a 500 in another:

fs.readFileSync('/sasjs/drive/files/bad.bin')  -> Error, status 500,
                                                  "Corrupt binary entry on the drive: not valid base64"
fs.statSync(...).size                          -> the same error
runtime.stats()                                -> the same error

Buffer.from(text, 'base64') keeps node's lenient decoder, because there the caller is a service using node's API and node ignores what it cannot decode - the strictness belongs to the drive entry, which only the host writes.

base64Size versus entryBytes (:176). base64Size is gone: entrySize is entryBytes(contents).length, so the size and the contents come from the same decode and cannot diverge. A snapshot edited to carry a line break inside the base64 still reads back as its 3 bytes and measures 3.

allocBytes size (:390). It refuses what node refuses, with node's distinction: a non-number is a TypeError (The "size" argument must be of type number. Received type string ('x')), a negative, NaN or Infinity is a RangeError, and a fractional size is truncated as node truncates it (Buffer.alloc(1.5).length is 1).

The drive total now reads every entry, so it can refuse one. The worker's sasjs-stats reply carries the reason instead of leaving the page to time out, and persist.stats() (already rejecting) surfaces it.

Specs: refuses a corrupt byte entry wherever it is read, and measures what it hands back (runtime), reports a corrupt entry in the stats reply rather than leaving the page waiting (worker), and the alloc test now asserts the refusals. Mock-site suite 103 tests in 5 suites; tsc --noEmit and prettier --check clean.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff (runtime-core.js Buffer shim + binary drive entries, sw.js response headers and byte bodies, mockSite.ts seeding, and the specs). Test coverage for the new behaviour is thorough; hardening checklist N/A - the diff touches no package.json, lockfile, .npmrc or workflow.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:954 - headers: entryText(FILES.get(headersPath) || '') only strips the TEXT_PREFIX escape. A service that writes _SASJS_WEBOUT_HEADERS as bytes (a Buffer or ArrayBuffer, or a string with a non-utf8 encoding) leaves a BINARY entry on the drive, so entryText hands back "\u0000base64:..." and parseHeaders drops every line: the declared Content-type and Content-Disposition are silently ignored. Decode the headers entry the way the body is decoded (isBinary(...) ? UTF-8 decode of decodeEntry(...) : entryText(...)) so the two channels cannot diverge.

  • src/commands/build/internal/mockSite/assets/runtime-core.js (FS.readFileSync, no-encoding branch) - the return type is now content-dependent: a plain string for a stored text entry, a decorated Buffer for a stored byte entry. On the server readFileSync is always a Buffer, so readFileSync(p).length is a byte count there but a character count here for text ('e-acute' - 1, not 2), and .toString('base64') returns the text itself rather than its base64. The contract comment states the string case, but this is exactly the mock/server divergence the shim exists to remove; consider always returning a Buffer and letting the text case be its toString().

  • src/commands/build/internal/mockSite/mockSite.ts:65 and assets/runtime-core.js:92 - the seed format ("\u0000base64:" / "\u0000text:") is now a two-sided contract held by two copies of the same constants. Nothing fails loudly if only one side is edited: a seeded drive would then decode as text or 500 per entry. A single shared source (or a format/version check on load) would make that drift impossible.

Note: CI is still pending at this head (CI_STATE=pending), so the suite has not been observed green.

3 findings above for review.

The worker's decode repeated the corrupt-entry error the runtime now raises
before it hands a byte entry over, which left that catch unreachable. A drive
entry is validated in runtime-core.js, so the worker's decode is a plain atob
and the message a caller sees comes from a single definition.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

The worker's decode repeated that error, and with the runtime validating first its catch became unreachable - so toBytes is a plain decode now and the message a caller sees has one definition, in decodeEntry. Committed as d737599 on top of a0612a5 (the three fixes), so sw.js keeps its coverage rather than carrying dead code.

Mock-site suite 103 tests in 5 suites.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff (mock runtime binary/Buffer shim, webout headers, drive-seed base64 encoding, spec updates) - no package.json, .npmrc or CI changes, so the JS hardening checklist does not apply here.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:950 - the binary webout is taken from the stored entry with a bare stored.slice(BINARY_PREFIX.length), bypassing the decodeEntry validation every other read path now applies (readDriveFile, statSync, copyFileSync). A corrupt byte entry at weboutPath - reachable by importing a snapshot into the drive - therefore reaches toBytes in sw.js:47 as raw base64 and atob throws, which contradicts that function's own comment ("refuses a drive entry that will not decode before it hands the value over"). Validate here the way readDriveFile does and surface the failure as result.error (or map it to a 500 in handle), so the corrupt entry fails with the documented Corrupt binary entry on the drive: not valid base64 rather than an uncaught exception inside the fetch handler.
  • src/commands/build/internal/mockSite/assets/runtime-core.js:927 - adding Buffer as a new Function parameter widens the parameter/body name collision: a service that declares a top-level Buffer binding (e.g. const Buffer = require('buffer').Buffer) now throws a SyntaxError from new Function itself, and that construction sits outside the try that captures service errors - the run handler throws instead of returning { error }. Inject the shim through the sandbox object (or via a globalThis-style shim) rather than a parameter, or move the construction inside the try so a collision degrades to a normal program error.
  • src/commands/build/internal/mockSite/assets/sw.js:77 - HEADER_VALUE is /^[\x09\x20-\x7E\xA0-\xFF]+$/, which drops 0x80-0x9F even though HTTP obs-text and the comment above it both allow every byte up to U+00FF; a latin1 filename byte in that range in Content-Disposition silently loses the header, which is the exact class of value the comment says is passed through. Widen to \x09\x20-\x7E\x80-\xFF (0x7F stays excluded) or correct the comment. Related: parseHeaders overwrites on duplicate Name: lines, so a service that writes the same header twice gets only the last one - node/the server would send both.

3 findings above for review.

Advisory only - the CI check at this head was still pending when the review ran, so treat that as unverified.

…un error

Two paths still bypassed the work of the last commit:

* the binary webout was taken from the stored entry with a bare slice, so a
  corrupt byte body - reachable through a snapshot imported into the drive and
  copied to the webout path - reached the worker as raw base64 and atob threw
  inside the fetch handler. It is validated where it is read now, and the run
  reports the corrupt entry in `error`.

* `new Function` sat outside the try that captures service errors, so a service
  declaring a name the prelude binds (`const Buffer = ...`) raised a SyntaxError
  out of the run handler instead of becoming a program error.

The worker's header parse also follows the server's `extractHeaders` on a
repeated name: the last line wins, and a value that cannot be sent removes the
header rather than leaving the earlier one in place. 0x7F and 0x80-0x9F stay
outside the accepted set with the reason recorded - the browser's Headers
implementation strips those bytes from a value silently, so the header would be
applied with characters missing.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

All three are in b16f7ad.

Byte body at the webout path (:950). It goes through the same validation as every other read, and a failure becomes the run's error rather than an exception out of the fetch handler:

fs.copyFileSync('/sasjs/drive/files/broken.bin', weboutPath)
-> { binary: false, error: "... Corrupt binary entry on the drive: not valid base64", log: ... }

Spec: fails a run whose byte body will not decode, rather than serving it.

new Function outside the try (:927). The construction moved inside it, so a name collision degrades to a normal program error - the class, not just Buffer: require, console and process are bound the same way. Spec: reports a service that redeclares a prelude name as a program error runs const Buffer = 1 and asserts the run returns an error instead of throwing.

HEADER_VALUE and duplicate lines (sw.js:77). On the range: I did not widen it, because the browser's Headers implementation does not pass 0x80-0x9F through, it drops those bytes from the value silently - so a filename carrying one would be applied with characters missing, which is the silent wrong answer the mock exists to avoid. Verified:

new Response('x', { headers: { 'content-disposition': 'a\u0080b' } })
  .headers.get('content-disposition')   ->  "ab"

The comment now says that, and names 0x7F (also stripped) alongside. HTAB is in the set, so the tab case from the previous review still passes.

On duplicates, the server behaves as the mock did - api/src/utils/extractHeaders.ts:18 keys headersObj[key.toLowerCase()] = value, so the last line wins rather than both being sent. One detail of that file the mock did miss: a value that fails validation deletes the key, so a later bad line removes the header the earlier one set. parseHeaders now does that too, and lets the last line for a header win, and an unusable one remove it pins both halves.

Mock-site suite 106 tests in 5 suites; tsc --noEmit and prettier --check clean.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff at b16f7ad: runtime-core.js (byte-entry representation, Buffer shim, readFileSync/writeFileSync, runProgram, readDriveFile), sw.js (response headers, binary bodies, stats error path), persist.js (stats via worker), mockSite.ts (binary seeding), plus the four spec files. The node-parity edge cases the PR claims are covered by tests (strict base64 for drive entries vs lenient decode for Buffer.from, toString ranges, alloc fill, append flags, corrupt-entry 500). No package.json, lockfile, .npmrc or CI change, so the JS hardening checklist does not apply; no secrets in the diff. Commit subject is a valid release-driving conventional commit (feat(mock)).

Findings:

  • src/commands/build/internal/mockSite/assets/runtime-core.js:443-448 - the comment argues against the code it sits on. It says node "only masks the high bit when it decodes one; masking here would corrupt a body node encodes as-is", but line 447 masks on encode: Uint8Array.from(value, (char) => char.charCodeAt(0) & 0xff). Node encodes ascii as latin1, i.e. the low byte of the code unit, so & 0xff is correct and the comment is wrong - as written it invites the next reader to delete a mask that must stay. Reword to something like "node encodes ascii as latin1 (the low byte of the code unit) and only clears the high bit when it decodes".

  • src/commands/build/internal/mockSite/assets/runtime-core.js:215,723,1190 - entrySize materialises the whole entry just to measure it: a base64 decode for a byte entry, a full UTF-8 encode for a text one. So statSync decodes a file to answer size, and stats() decodes every file on the drive on every call. This PR is what makes that cost matter: mockSite.ts now seeds non-text project files onto the drive instead of skipping them, so a multi-MB archive or xlsx is decoded in full per stat. Both callers only need a number that is already known at write time - carry the byte length alongside the entry (a parallel size map maintained in writeFileSync, copyFileSync, loadSnapshot and boot) instead of recomputing it.

  • src/commands/build/internal/mockSite/mockSite.ts:66-73 - every non-text build file is now base64-inlined in vfs-seed.json (about +33% on those files) and held in the worker's memory from boot, where the previous behaviour was a warning and no entry. Worth stating in the PR body as a deliberate trade; if it is not intended for binary-heavy projects, a size cap with the old "left off the mock drive" warning for the rest would bound it without losing the common case.

3 findings above for review.

statSync and the drive total materialised the whole entry to answer with a
number: a base64 decode for a byte entry, a full utf8 encode for a text one.
The drive carries seeded project files now, so a multi-megabyte archive was
decoded in full on every stat.

A byte entry's size is computed from the validated base64 string instead (three
bytes per four characters, less the padding), which is exactly what the decoder
yields for the same string - so the arithmetic cannot drift from the contents,
and no parallel size map has to be kept in step with four write paths. A text
entry still costs one utf8 scan.

The ascii arm's comment now describes the code beside it: node encodes ascii as
latin1, the low byte of the code unit, and clears the high bit only on decode.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

All three addressed in 76a495c.

The ascii comment. Reworded to describe the code beside it: "node encodes 'ascii' as latin1 - the low byte of the code unit - and clears the high bit only when it decodes". The & 0xff stays, now for a stated reason rather than in spite of one.

entrySize materialising the entry. It no longer decodes. A byte entry's size comes from the validated base64 string: three bytes per four characters less the padding, which is exactly what the decoder yields, so the arithmetic cannot drift from the contents:

'\u0000base64:AQIDBA=='  -> 4 bytes   (8 chars, '==' padding)
'\u0000base64:AQIDBAU='  -> 5 bytes   (8 chars, '=' padding)
'\u0000base64:AQIDBAUG'  -> 6 bytes   (8 chars, no padding)

Spec: measures a byte entry from its stored length, whatever the padding. A text entry still costs one utf8 scan, which is a scan and not an allocation of its bytes.

I did not add a parallel size map. It would need to stay in step with writeFileSync, copyFileSync, loadSnapshot, unlinkSync and boot, and a single missed invalidation is a wrong size from then on - the failure mode this PR keeps closing. The pure function cannot drift, and the cost it removes (a full decode per stat) is the part that mattered.

The seed trade. Stated in the PR body as a deliberate one: each non-text file is base64 in vfs-seed.json and in the worker's memory, so 1.33x its bytes, and the drive already lives in the worker's memory as the design stands. No size cap: a threshold would leave the largest assets off the drive, which are the ones a download is tested with, and silently dropping files is the behaviour this PR removes. If the maintainers prefer a cap, it is a one-line change at seedFromBuild and I will add it.

Mock-site suite 107 tests in 5 suites; tsc --noEmit and prettier --check clean. CI was green at b16f7ad and is running again at 76a495c.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff (9 files: browser runtime-core fs/Buffer shim, service worker header handling, page persist stats, mockSite seeding, and their specs). Checks were node semantics for encodings/Buffer, header injection and response splitting on _SASJS_WEBOUT_HEADERS, the base64 drive encoding and its markers, and the changed read()/stats() shapes. No security issue and no breaking public API surface found; two notes and one caveat below.

  • src/commands/build/internal/mockSite/mockSite.ts:68 readDriveFile applies no size cap: every non-text file in the build folder is now base64-inlined into vfs-seed.json at 4/3 its size and parsed by the browser, where the old path skipped such files and warned. A single large artefact in mock-storage (an export, a media file, a vendored archive) is enough to make the generated mock site unloadable, and the warning that used to name the offenders is gone. Suggest keeping a size guard with the old warning text.

  • src/commands/build/internal/mockSite/assets/persist.js:164 return answer.stats is unchecked. An error reply throws, but a reply that carries neither error nor stats (a service worker cached from an older build, or a future reply shape) hands undefined to a caller that destructures { files, bytes } - the NaN outcome the new comment sets out to prevent. if (!answer.stats) throw new Error('sasjs-stats reply carried no stats') closes it.

  • Caveat: CI was still pending on this head when the diff was fetched (contexts=[]), so the ~620 new spec assertions have not been observed green here. Worth confirming before merge, since they are the only evidence for most of the node-parity claims.

2 findings above for review.

… with none

Two reviewers asked for the same bound, and they are right that one large
artefact is enough to make the generated site unloadable: a byte entry is
base64 in vfs-seed.json and held in the worker's memory. A file over 10 MB is
left off the drive and named in the build warning, with its size, which is the
behaviour the drive had when it carried text only. Text keeps no bound - a
program folder is text by nature and the drive has always carried it.

The page-side stats read also refuses a reply that carries neither an error nor
stats, which an older worker answers with: destructuring { files, bytes } from
it would hand back NaN, the outcome the check above it exists to prevent.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Both findings fixed in cb38605.

The seed bound (mockSite.ts:68). You and the earlier review are right, and I have withdrawn my no-cap argument: one archive is enough to make the site unloadable, and "the drive is complete" is not worth that failure mode. A byte file over 10 MB is left off the drive and named in the build warning with its size, which restores the shape the drive had when it carried text only:

Left 1 file(s) off the mock drive, each over the 10 MB the browser build seeds:
/sasjs/drive/files/Public/app/test/mock-storage/export.zip (10 MB)

The bound is MAX_BINARY_SEED_BYTES, exported from mockSite.ts and used by the spec: leaves a byte file over the seed bound off the drive, and names it seeds a file one byte over, asserts it is absent from the seed and that the warning names it. Text keeps no bound, because a program folder is text by nature and the drive carried it before this change - say if you want the cap on text as well.

The stats reply shape (persist.js:164). if (!answer.stats) throw new Error('sasjs-stats reply carried no stats'), with fails the read when the reply carries no stats replying { type: 'sasjs-stats' } - the shape an older worker would send - and asserting the read rejects instead of handing back undefined.

On the caveat: CI was green at 76a495c (all three checks, coverage annotation included) and is running again at cb38605. The suite is 90 suites / 761 tests locally with generateDocs.spec and version.spec excluded, and the mock-site set is 109 tests.

Trade now stated in the PR body as well, with the bound rather than the open-ended version.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff: mock-site browser host only (runtime-core.js, sw.js, persist.js, mockSite.ts, three spec files). No package.json, lockfile or CI change, so the JS hardening checklist does not apply here. Conventional commit title (feat(mock): ...) is well formed and release-driving.

  • src/commands/build/internal/mockSite/mockSite.ts:65 - export const MAX_BINARY_SEED_BYTES = 10 * 1024 * 1024 is a per-file bound with no aggregate budget. The comment above it says the bound exists so one large artefact cannot make the generated site unloadable, but N files each just under the bound still do: five 9 MB images seed ~60 MB of base64 into vfs-seed.json and the worker's IndexedDB. Suggest a total ceiling in seedFromBuild, e.g. accumulate the encoded size and stop seeding byte entries past a budget (warning names the file and the remaining budget) - the per-file limit alone does not bound the site size.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:224 (entrySize) - for a byte entry it calls cleanedBase64, which does String(contents.slice(...)).replace(/\s+/g, ''), allocating a full copy of the base64 plus a regex pass over it, and this runs per entry from stats() (runtime-core.js:1202). With seeded binaries that makes every stats() call O(total drive bytes) with a fresh multi-megabyte allocation per entry - and stats() is the value persist.js now asks the worker for. Cheap fix: validate base64 where an entry enters the drive (load/seed/write) and cache the decoded byte length on the entry, so the stat path is arithmetic only, as the function's own comment claims.

No blocking issues; both points are scale/latency hardening for the seeding feature rather than correctness of the download path, which is well covered by the new specs (header value validation blocks CR/LF injection into Response headers, and corrupt base64 is refused at every read).

The per-file bound left the site size unbounded: five 9 MB images pass it and
still seed ~60 MB of base64. seedFromBuild now also spends a 25 MB total across
byte entries, in listing order, and the warning says which bound a file hit -
"over the per-file limit" or "over the drive budget".

statSync and the drive total measured every byte entry by cleaning the whole
base64 string, so stats() scanned and allocated once per entry, per call. The
measure is cached against the stored string itself: a cache entry is only
trusted while it is the same string, so a write, a copy or a load that replaces
the contents recomputes, and a repeated stats() is arithmetic over numbers
already known.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Both hardening points are in 1eabbd8.

A total budget as well as a per-file one (mockSite.ts:65). seedFromBuild now spends a 25 MB total across byte entries in listing order, on top of the 10 MB per file, and the warning says which bound a file hit:

Left 1 file(s) off the mock drive, over the 10 MB per-file or 25 MB total the
browser build seeds: /sasjs/drive/files/.../three.bin (9 MB, over the drive budget)

Text is outside both bounds, because a program folder is text by nature and the drive has always carried it. Spec: stops seeding bytes once the drive budget is spent, and names the file seeds three files of 9 MB each, asserts two are on the drive and that the warning carries both the reason and a filename. (The folder listing order is not defined, so the assertion names the reason rather than a specific file.)

entrySize scanning per call (:224). The measure is cached against the stored string itself:

if (cached && cached.contents === contents) return cached.size

An entry is only trusted while it is the same string, so a write, a copy, a load or a reset that replaces the contents recomputes on the next stat - there is no path that can leave a stale size behind, which a parallel size map maintained by hand could. The cache is dropped on loadSnapshot and reset, and per path on unlinkSync. So stats() is arithmetic over numbers already measured after the first call, and the first call after a change costs one scan of the changed entry only.

Both bounds and the cache are covered by the existing corruption tests: a corrupt entry still throws from readFileSync, statSync and stats(), and a failed measure is never cached.

Mock-site suite 110 tests in 5 suites; tsc --noEmit and prettier --check clean. CI was green at cb38605 and is running at 1eabbd8.

@sasjs-dev sasjs-dev Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff (4 assets/sources + 4 spec files): binary drive entries behind a NUL marker, the Buffer/fs node-fidelity shim, _SASJS_WEBOUT_HEADERS applied in sw.js, and the seed size bounds in mockSite.ts.

  • src/commands/build/internal/mockSite/mockSite.ts:132 - the total-budget check depends on directory iteration order: seededBytes += file.bytes accumulates in whatever order the folder listing yields, so which byte files fall off the drive differs between machines and filesystems, and so does the order of the names in the warning at mockSite.ts:384. Two files each under MAX_BINARY_SEED_BYTES but over MAX_SEED_BYTES together seed a different subset per host, and spec/mockSite.spec.ts ("the folder is listed in no guaranteed order") has to match the warning with a regex because of it. Sort the candidate list by path before applying the budget (and sort skipped), so the same project yields the same drive everywhere.
  • src/commands/build/internal/mockSite/assets/runtime-core.js:432 - Uint8Array.of(fill & 0xff) silently masks an out-of-range fill: node's Buffer.alloc(4, 300) throws RangeError [ERR_OUT_OF_RANGE] ("value" must be >= 0 && <= 255), the mock yields 0x2c repeated, so a service that passes a computed fill gets a plausible-looking body here and a 500 on the server. runtime-core.js:162 has the same gap the other way: Uint8Array.from(data) truncates non-integer and >255 members, where node rejects Buffer.from([1.5]) / Buffer.from([256]) with ERR_INVALID_ARG_VALUE. Both are worth rejecting with node's message, since the rest of the shim is careful to fail where node fails.

No package.json, lockfile or CI changes in the diff, so the hardening checklist does not apply. Title is a valid Conventional Commit (feat(mock):). No CI checks reported on this head (CI_STATE=pending, no contexts), so nothing has been run against it.

2 findings above for review.

The total was accumulated in whatever order the folder listing returned, so
which byte files fell off the drive - and which names the warning carried -
differed between machines and filesystems. The candidates are sorted by drive
path before the bounds apply, so a project seeds the same drive everywhere; the
spec now pins the exact files that fit rather than matching the warning with a
pattern.
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Deterministic budget: fixed in e9bdf03. The candidates are collected first and sorted by drive path before the bounds apply, so the same project seeds the same drive, and the warning names the same files, on every host and filesystem. The spec now pins the outcome instead of matching with a pattern:

one.bin, three.bin, two.bin (9 MB each, 25 MB budget)
-> one.bin and three.bin seeded, "two.bin (9 MB, over the drive budget)" warned

The fill/array claim does not hold, I checked before changing anything. Neither Buffer.alloc(4, 300) nor Buffer.from([256]) throws on node - both truncate to a byte, which is what the shim already does:

node v18.20.8 / v22.23.4 / v26.5.1
Buffer.alloc(4, 300)      -> 2c2c2c2c        shim: 2c2c2c2c
Buffer.alloc(4, -1)       -> ffffffff        shim: ffffffff
Buffer.alloc(4, 1.5)      -> 01010101        shim: 01010101
Buffer.from([1.5])        -> 01              shim: 01
Buffer.from([256])        -> 00              shim: 00
Buffer.from([-1])         -> ff              shim: ff

Buffer.alloc's numeric fill is documented as truncated to a uint8, and Buffer.from(array) masks each member the same way Uint8Array.from does. Adding a RangeError there would make the mock reject what node accepts - the divergence this PR exists to remove - so I left both arms alone. If you have a node build where one of those throws, name the version and I will match it.

@sasjs-dev sasjs-dev Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full base...head diff: runtime-core.js (byte entries, Buffer shim, fs rewrite, run/webout), sw.js (header parsing, byte responses, stats error path), persist.js (stats via the worker), mockSite.ts (seed bounds) and the specs. No package.json, .npmrc, lockfile or workflow change in this diff, so the hardening checklist has nothing to check here; CI at head is still pending, not green. This review covers the current diff in full, not the delta.

  • src/commands/build/internal/mockSite/assets/runtime-core.js:493 (BUFFERS.from, ArrayBuffer branch): return asBuffer(new Uint8Array(value, start, end - start)) and asBuffer at :409 does decorate(new Uint8Array(bytes)), which copies. Node's Buffer.from(arrayBuffer[, byteOffset[, length]]) returns a view of that ArrayBuffer without copying, so a write through the Buffer is visible in the ArrayBuffer. The mock diverges from node exactly where this file takes care to match it for slice()/subarray(): a service that is handed an ArrayBuffer and patches bytes in place sees no effect in the browser host. Decorate the view rather than copying it for the ArrayBuffer overload (a Uint8Array input should keep copying, as node does).

1 finding above for review.

@allanbowe
allanbowe merged commit 0cc2529 into main Oct 25, 2026
3 checks passed
@allanbowe
allanbowe deleted the feat/mock-webout-headers branch October 25, 2026 12:38
@sasjs-dev

sasjs-dev Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up for the shim items deliberately left out at merge, plus the one finding raised at the final head (the Buffer.from(arrayBuffer) copy): #1495.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(mock site): serve file downloads - honour _SASJS_WEBOUT_HEADERS and a binary body

1 participant