Skip to content

Commit 2a3e519

Browse files
JasonGrossclaude
andcommitted
Make the web page's argv string encoding a faithful round trip
Fixes scrutineer finding #2519. In fiat-html/main.js, splitUnescapedSpaces and joinWithEscaping convert between the "Input String" text box and the argv array passed to the worker. The decoder used U+0000 and U+0001 as internal placeholders and dropped empty strings, so `splitUnescapedSpaces(joinWithEscaping(a))` was not `a` for arguments containing those characters or for empty arguments. When the page was opened via a crafted `?argv=...&interactive` link, `updateInputType` re-encoded the text box lossily while `parseAndRun` synthesised from the original argv, so the visitor saw one command line and ran another. With `&inputType=json` the text box was additionally mangled by running the string decoder over JSON text. Fix: * Move the pair to a new file fiat-html/argv-string.js and reimplement the decoder as a character-by-character parser with no placeholder characters. `\ ` and `\\` keep their meaning; `\"` is a literal quote; a bare `""` token is the empty argument (the only new syntax). Any other backslash sequence is kept literally, as before. The encoder emits `""` for empty arguments and escapes the quotes of an argument that is literally `""`. The file is a classic browser script that also exports the two functions when loaded under CommonJS, so it can be unit-tested. * On load from a URL, decode ?argv=, ?stdin= and ?files= once and derive both the form contents and the arguments handed to the worker from that single decoded value. The Synthesize button likewise goes through one `getInputArgs` helper. ?inputType= is now normalised to 'json' or 'string' instead of being interpolated into a CSS selector. * Fix the permalink, which emitted `&inputType=&inputType=json` and so never reopened in JSON view. * Add fiat-html/tests/argv-roundtrip.test.js (node's built-in test runner, no dependencies): hand-picked adversarial cases, 20000 random arrays over an adversarial alphabet, agreement with the old decoder on legacy input, and stability under string/JSON toggling. A small workflow runs it on changes to fiat-html/. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Rbn2gww3MGhvrh52fNjpD
1 parent 5691ca0 commit 2a3e519

5 files changed

Lines changed: 388 additions & 43 deletions

File tree

.github/workflows/fiat-html.yml

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
name: Test fiat-html
2+
3+
on:
4+
push:
5+
paths:
6+
- 'fiat-html/**'
7+
- '.github/workflows/fiat-html.yml'
8+
pull_request:
9+
paths:
10+
- 'fiat-html/**'
11+
- '.github/workflows/fiat-html.yml'
12+
merge_group:
13+
14+
jobs:
15+
test-fiat-html:
16+
17+
runs-on: ubuntu-latest
18+
19+
concurrency:
20+
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
21+
cancel-in-progress: true
22+
23+
steps:
24+
- uses: actions/checkout@v7
25+
- uses: actions/setup-node@v6
26+
with:
27+
node-version: 'lts/*'
28+
- name: node --test 'fiat-html/tests/*.test.js'
29+
run: node --test 'fiat-html/tests/*.test.js'

fiat-html/argv-string.js

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Conversion between the "Input String" form of the command line shown in
2+
// the text box and the array of arguments that is actually passed to
3+
// fiat_crypto.js.
4+
//
5+
// The two functions below are an encode/decode pair and must be exact
6+
// inverses of each other: for every array of strings `a`,
7+
//
8+
// splitUnescapedSpaces(joinWithEscaping(a)) deep-equals a
9+
//
10+
// (this is checked by fiat-html/tests/argv-roundtrip.test.js). The page
11+
// relies on this so that the command line a visitor sees in the text box is
12+
// the command line that gets run, in particular when the page is opened via
13+
// a ?argv=... link (scrutineer finding #2519).
14+
//
15+
// Format of the string form:
16+
//
17+
// * Arguments are separated by one or more unescaped ASCII spaces.
18+
// Leading and trailing spaces are ignored.
19+
// * `\ ` (backslash space) is a literal space inside an argument.
20+
// * `\\` (two backslashes) is a literal backslash.
21+
// * `\"` is a literal double quote.
22+
// * The token `""` on its own (two unescaped double quotes, delimited by
23+
// spaces or the ends of the string) is an empty argument. This is the
24+
// only place where double quotes have any meaning; anywhere else they
25+
// are ordinary characters, e.g. `"a b"` is the two arguments `"a` and
26+
// `b"`, and `"" ` inside a longer token is literal.
27+
// * A backslash followed by any other character, or a trailing backslash,
28+
// is kept literally (so `C:\path` and `\n` mean exactly what they say).
29+
// joinWithEscaping never produces these; they are accepted for the sake
30+
// of hand-typed input.
31+
// * Every other character, including control characters such as U+0000
32+
// and U+0001, stands for itself. The decoder does not use placeholder
33+
// characters, so no character is off limits inside an argument.
34+
//
35+
// `\ ` and `\\` have the same meaning as in the original format, so existing
36+
// hand-typed command lines that do not contain a bare `""` token are parsed
37+
// exactly as before.
38+
39+
function splitUnescapedSpaces(input) {
40+
const args = [];
41+
let current = ''; // The argument being accumulated
42+
let inArg = false; // Whether `current` has been started (may still be '')
43+
let i = 0;
44+
const n = input.length;
45+
while (i < n) {
46+
const c = input[i];
47+
if (c === ' ') {
48+
if (inArg) {
49+
args.push(current);
50+
current = '';
51+
inArg = false;
52+
}
53+
i++;
54+
} else if (c === '\\' && i + 1 < n && (input[i + 1] === ' ' || input[i + 1] === '\\' || input[i + 1] === '"')) {
55+
// Recognised escape: the next character is literal
56+
current += input[i + 1];
57+
inArg = true;
58+
i += 2;
59+
} else if (c === '"' && !inArg && input[i + 1] === '"' && (i + 2 >= n || input[i + 2] === ' ')) {
60+
// A bare `""` token is the empty argument
61+
args.push('');
62+
i += 2;
63+
} else {
64+
// Ordinary character, or a backslash that is not part of a
65+
// recognised escape (kept literally)
66+
current += c;
67+
inArg = true;
68+
i++;
69+
}
70+
}
71+
if (inArg) {
72+
args.push(current);
73+
}
74+
return args;
75+
}
76+
77+
function joinWithEscaping(inputArray) {
78+
return inputArray
79+
.map(s => {
80+
if (s === '') {
81+
return '""';
82+
}
83+
const escaped = s
84+
.replace(/\\/g, '\\\\') // Escape backslashes
85+
.replace(/ /g, '\\ '); // Escape spaces
86+
// A token that would read as the empty-argument marker must have
87+
// its quotes escaped; quotes are literal everywhere else.
88+
return escaped === '""' ? '\\"\\"' : escaped;
89+
})
90+
.join(' ');
91+
}
92+
93+
// Allow the test-suite (and any other CommonJS consumer) to load this file;
94+
// in the browser it is a classic script and the functions are globals.
95+
if (typeof module !== 'undefined' && module.exports) {
96+
module.exports = { splitUnescapedSpaces, joinWithEscaping };
97+
}

fiat-html/fiat-crypto.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ <h3>Input Files <button type="button" id="addFileButton" class="add-btn">+</butt
249249
</div>
250250
<script src="version.js"></script>
251251
<script src="https://unpkg.com/wasm-feature-detect/dist/umd/index.js"></script>
252+
<script src="argv-string.js"></script>
252253
<script src="file-input.js"></script>
253254
<script src="main.js"></script>
254255
<!-- N.B. disable-wasm-option.js must come after main.js so that the wasm box is unchecked correctly after parsing argv -->

fiat-html/main.js

Lines changed: 71 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,26 +20,10 @@ document.addEventListener('DOMContentLoaded', function () {
2020
const isSafari = /^((?!chrome|android).)*safari/i.test(navigator.userAgent);
2121
const isMacOrIOS = /Macintosh|MacIntel|MacPPC|Mac68K|iPhone|iPad|iPod/.test(navigator.platform);
2222

23-
function splitUnescapedSpaces(input) {
24-
return input
25-
.replace(/\\\\/g, '\u0000') // Temporarily replace \\ with a placeholder
26-
.replace(/\\ /g, '\u0001') // Temporarily replace escaped spaces with a placeholder
27-
.split(/ +/) // Split by spaces
28-
.filter(s => s)
29-
.map(s => s
30-
.replace(/\u0000/g, '\\') // Restore backslashes
31-
.replace(/\u0001/g, ' ') // Restore spaces
32-
);
33-
}
34-
35-
function joinWithEscaping(inputArray) {
36-
return inputArray
37-
.map(s => s
38-
.replace(/\\/g, '\\\\') // Escape backslashes
39-
.replace(/ /g, '\\ ') // Escape spaces
40-
)
41-
.join(' ');
42-
}
23+
// splitUnescapedSpaces and joinWithEscaping (the "Input String" <-> argv
24+
// array conversion) live in argv-string.js, which is loaded before this
25+
// file. They are exact inverses of each other, which is what keeps the
26+
// command line shown in the text box in sync with the one that runs.
4327

4428
function parseToStringArray(str, name) {
4529
let args = JSON.parse(str);
@@ -194,8 +178,7 @@ document.addEventListener('DOMContentLoaded', function () {
194178
const files = getFilesFromFormBoxRaw();
195179
const stdinString = stdin ? `&stdin=${encodeURIComponent(stdin)}` : '';
196180
const filesString = files ? `&files=${encodeURIComponent(files)}` : '';
197-
const inputType = document.querySelector('input[name="inputType"]:checked').value === 'json' ? `&inputType=json` : '';
198-
const inputTypeString = inputType !== 'string' ? `&inputType=${inputType}` : '';
181+
const inputTypeString = getInputType() === 'json' ? `&inputType=json` : '';
199182
const queryString = `?argv=${encodeURIComponent(JSON.stringify(args.slice(1)))}${stdinString}${filesString}${inputTypeString}&interactive${wasmString}`;
200183
// Handle both file and http(s) URLs
201184
let baseUrl = window.location.href.split('?')[0]; // Get base URL without query string
@@ -360,22 +343,51 @@ document.addEventListener('DOMContentLoaded', function () {
360343
currentWorker.onerror = recieveMessage(false);
361344
}
362345

363-
function parseAndRun(argv, stdinv, filesv) {
364-
try {
365-
let args = parseToStringArray(decodeURIComponent(argv), 'argv');
366-
args.unshift('fiat_crypto.js');
367-
let stdin = parseToStringArrayArray(decodeURIComponent(stdinv), 'stdin');
368-
let files = parseToStringMapStringArray(decodeURIComponent(filesv), 'files');
369-
handleSynthesis(args, stdin, files);
370-
} catch (e) {
371-
displayError(`Error: ${e.message}: ${argv}, ${stdinv}, ${filesv}`);
372-
}
346+
// Decode the ?argv=, ?stdin= and ?files= query parameters into the values
347+
// handed to the worker. Throws on malformed input.
348+
function parseQueryArgs(argv, stdinv, filesv) {
349+
const args = parseToStringArray(decodeURIComponent(argv), 'argv');
350+
const stdin = parseToStringArrayArray(decodeURIComponent(stdinv), 'stdin');
351+
const files = parseToStringMapStringArray(decodeURIComponent(filesv), 'files');
352+
return { args, stdin, files };
353+
}
354+
355+
// `args` is the argument list without the program name.
356+
function runSynthesis(args, stdin, files) {
357+
handleSynthesis(['fiat_crypto.js', ...args], stdin, files);
373358
}
374359

375360
function nonFalseQueryParam(value) {
376361
return value !== null && value != 'false' && value != '0';
377362
}
378363

364+
// The selected input type, always one of 'json' or 'string'.
365+
function getInputType() {
366+
return document.querySelector('input[name="inputType"]:checked').value === 'json' ? 'json' : 'string';
367+
}
368+
369+
function setInputType(inputType) {
370+
document.querySelector(`input[name="inputType"][value="${inputType === 'json' ? 'json' : 'string'}"]`).checked = true;
371+
}
372+
373+
// Show `args` (an array of strings, without the program name) in the text
374+
// box, in the representation selected by the input-type radio buttons.
375+
function setInputArgs(args) {
376+
inputArgs.value = getInputType() === 'json' ? JSON.stringify(args) : joinWithEscaping(args);
377+
validateInput();
378+
}
379+
380+
// Parse the text box into an array of strings (without the program name),
381+
// according to the selected input type. Throws if the JSON is invalid.
382+
function getInputArgs() {
383+
return getInputType() === 'json'
384+
? parseToStringArray(inputArgs.value, 'input')
385+
: splitUnescapedSpaces(inputArgs.value);
386+
}
387+
388+
// Convert the text box between the two representations when the user
389+
// switches input type. Because joinWithEscaping and splitUnescapedSpaces
390+
// are exact inverses, switching back and forth never changes the arguments.
379391
function updateInputType(inputType) {
380392
if (inputType === 'string') {
381393
if (isValidJsonStringArray(inputArgs.value)) {
@@ -392,7 +404,7 @@ document.addEventListener('DOMContentLoaded', function () {
392404
const argv = queryParams.get('argv');
393405
const interactive = queryParams.get('interactive');
394406
const wasm = queryParams.get('wasm')
395-
const inputType = queryParams.get('inputType') || 'string';
407+
const inputType = queryParams.get('inputType') === 'json' ? 'json' : 'string';
396408
const stdin = queryParams.get('stdin') || '[]';
397409
const files = queryParams.get('files') || '{}';
398410

@@ -403,15 +415,27 @@ document.addEventListener('DOMContentLoaded', function () {
403415
setupWorkers();
404416

405417
if (argv) {
418+
let parsed;
419+
try {
420+
parsed = parseQueryArgs(argv, stdin, files);
421+
} catch (e) {
422+
displayError(`Error: ${e.message}: ${argv}, ${stdin}, ${files}`);
423+
if (nonFalseQueryParam(interactive)) {
424+
inputForm.classList.remove('hidden');
425+
}
426+
return;
427+
}
406428
if (nonFalseQueryParam(interactive)) {
407-
inputArgs.value = decodeURIComponent(argv);
408-
populateStdinEntries(JSON.parse(decodeURIComponent(stdin)));
409-
populateFileEntries(JSON.parse(decodeURIComponent(files)));
410-
document.querySelector(`input[value="${inputType}"]`).checked = true;
411-
updateInputType(inputType);
429+
// Fill the form from the same decoded values that are about to
430+
// run, so that the command line the visitor sees is exactly the
431+
// one that executes (scrutineer finding #2519).
432+
populateStdinEntries(parsed.stdin);
433+
populateFileEntries(parsed.files);
434+
setInputType(inputType);
435+
setInputArgs(parsed.args);
412436
inputForm.classList.remove('hidden');
413437
}
414-
parseAndRun(argv, stdin, files);
438+
runSynthesis(parsed.args, parsed.stdin, parsed.files);
415439
} else {
416440
inputForm.classList.remove('hidden');
417441
}
@@ -433,12 +457,16 @@ document.addEventListener('DOMContentLoaded', function () {
433457
synthesizeButton.disabled = true;
434458
cancelButton.disabled = false;
435459
// Parse arguments and handle synthesis
436-
const argsType = document.querySelector('input[name="inputType"]:checked').value;
437-
const args = argsType === 'json' ? JSON.parse(inputArgs.value) : splitUnescapedSpaces(inputArgs.value);
438-
args.unshift('fiat_crypto.js');
460+
let args;
461+
try {
462+
args = getInputArgs();
463+
} catch (e) {
464+
displayError(`Error: ${e.message}: ${inputArgs.value}`);
465+
return;
466+
}
439467
const stdin = getStdinFromFormBox();
440468
const files = getFilesFromFormBox();
441-
handleSynthesis(args, stdin, files);
469+
runSynthesis(args, stdin, files);
442470
});
443471

444472
inputForm.addEventListener('submit', function (event) {

0 commit comments

Comments
 (0)