From 9cc8927f33b2de6d222871b78634f5d638b15de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 7 Jan 2025 10:19:57 +0100 Subject: [PATCH 1/3] Failing build if translation files contain invalid values --- Dockerfile | 3 + buildtools/sanitize_translations.js | 87 ++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/Dockerfile b/Dockerfile index c57e2dba79..19d379a340 100644 --- a/Dockerfile +++ b/Dockerfile @@ -33,7 +33,10 @@ COPY test/chai-as-promised.js /grist/test/chai-as-promised.js COPY app /grist/app COPY stubs /grist/stubs COPY buildtools /grist/buildtools +COPY static/locales /grist/static/locales RUN yarn run build:prod +RUN rm -rf /grist/static/locales + # Prepare material for optional pyodide sandbox COPY sandbox/pyodide /grist/sandbox/pyodide diff --git a/buildtools/sanitize_translations.js b/buildtools/sanitize_translations.js index 874c7392f3..e8720d311f 100644 --- a/buildtools/sanitize_translations.js +++ b/buildtools/sanitize_translations.js @@ -14,6 +14,12 @@ function handleSanitizeAttribute(node) { node.setAttribute('target', '_blank'); } +// If the the first arg is test, do the self test. +if (process.argv[2] === "test") { + selfTest(); + process.exit(0); +} + const directoryPath = readDirectoryPath(); const fileStream = fs.readdirSync(directoryPath) @@ -25,38 +31,38 @@ const fileStream = fs.readdirSync(directoryPath) // Read the contents and put it into an array [path, json] .map((file) => [file, JSON.parse(fs.readFileSync(file, "utf8"))]); -console.debug(`Found ${fileStream.length} files to sanitize`); - const sanitized = fileStream.map(([file, json]) => { - return [file, json, sanitizedJson(json)]; + return [file, json, invalidValues(json)]; }); -const onlyDifferent = sanitized.filter(([file, json, sanitizedJson]) => { - return JSON.stringify(json) !== JSON.stringify(sanitizedJson); -}); - -console.debug(`Found ${onlyDifferent.length} files that need sanitizing`); - -// Write the sanitized json back to the files -onlyDifferent.forEach(([file, json, sanitizedJson]) => { - console.info(`Sanitizing ${file}`); - fs.writeFileSync(file, JSON.stringify(sanitizedJson, null, 4) + "\n"); +const onlyDifferent = sanitized.filter(([file, json, invalidKeys]) => { + return invalidKeys.length > 0; }); -console.info("Sanitization complete"); +if (onlyDifferent.length > 0) { + console.error("The following files contain invalid values:"); + onlyDifferent.forEach(([file, json, invalidKeys]) => { + console.error(`File: ${file}`); + console.error(`Values: ${invalidKeys.join(", ")}`); + }); + process.exit(1); +} -function sanitizedJson(json) { +function invalidValues(json) { // This is recursive function as some keys can be objects themselves, but all values are either // strings or objects. return Object.keys(json).reduce((acc, key) => { const value = json[key]; if (typeof value === "string") { - acc[key] = purify(value); + const sanitized = purify(value); + if (value !== sanitized) { + acc.push(value); + } } else if (typeof value === "object") { - acc[key] = sanitizedJson(value); + acc.push(...invalidValues(value)); } return acc; - }, {}); + }, []); } @@ -72,5 +78,48 @@ function readDirectoryPath() { function purify(inputString) { // This removes any html tags from the string - return DOMPurify.sanitize(inputString); + return DOMPurify.sanitize(inputString, { ALLOWED_TAGS: [] }); } + +function selfTest() { + const sys = require('child_process'); + const okDir = createTmpDir(); + const okFile = path.join(okDir, "ok.json"); + fs.writeFileSync(okFile, JSON.stringify({ "key": "value" })); + + const badDir = createTmpDir(); + const badFile = path.join(badDir, "bad.json"); + fs.writeFileSync(badFile, JSON.stringify({ "key": "" })); + + // Run this script in the okDir, it should pass (return value 0) + const okResult = exec(`node ${__filename} ${okDir}`); + if (!okResult) { + console.error("Self test failed, expected 0 for okDir"); + process.exit(1); + } + + // Run this script in the badDir, it should fail (return value 1) + const badResult = exec(`node ${__filename} ${badDir}`); + if (badResult) { + process.exit(1); + } + + console.log("Self test passed"); + + function createTmpDir() { + const os = require('os'); + const tmpDir = os.tmpdir(); + const prefix = path.join(tmpDir, 'tmp-folder-'); + const tmpFolderPath = fs.mkdtempSync(prefix); + return tmpFolderPath; + } + + function exec(args) { + try { + sys.execSync(args); + return true; + } catch (e) { + return false; + } + } +} \ No newline at end of file From f872736956019ef53547f3e34e1f56918d58a473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 7 Jan 2025 10:24:34 +0100 Subject: [PATCH 2/3] Adding comments --- Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Dockerfile b/Dockerfile index 19d379a340..a5ea4505a0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -33,8 +33,10 @@ COPY test/chai-as-promised.js /grist/test/chai-as-promised.js COPY app /grist/app COPY stubs /grist/stubs COPY buildtools /grist/buildtools +# Copy locales files early. During build process they are validated. COPY static/locales /grist/static/locales RUN yarn run build:prod +# We don't need them anymore, they will by copied to the final image. RUN rm -rf /grist/static/locales From cc379f4a199d4f48d95c26ab2c6ea88ee03ead58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 7 Jan 2025 14:03:14 +0100 Subject: [PATCH 3/3] Making code simpler --- buildtools/sanitize_translations.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/buildtools/sanitize_translations.js b/buildtools/sanitize_translations.js index e8720d311f..016f7bf0c1 100644 --- a/buildtools/sanitize_translations.js +++ b/buildtools/sanitize_translations.js @@ -51,8 +51,7 @@ if (onlyDifferent.length > 0) { function invalidValues(json) { // This is recursive function as some keys can be objects themselves, but all values are either // strings or objects. - return Object.keys(json).reduce((acc, key) => { - const value = json[key]; + return Object.values(json).reduce((acc, value) => { if (typeof value === "string") { const sanitized = purify(value); if (value !== sanitized) { @@ -82,7 +81,6 @@ function purify(inputString) { } function selfTest() { - const sys = require('child_process'); const okDir = createTmpDir(); const okFile = path.join(okDir, "ok.json"); fs.writeFileSync(okFile, JSON.stringify({ "key": "value" })); @@ -92,15 +90,16 @@ function selfTest() { fs.writeFileSync(badFile, JSON.stringify({ "key": "" })); // Run this script in the okDir, it should pass (return value 0) - const okResult = exec(`node ${__filename} ${okDir}`); - if (!okResult) { + const okResult = exitCode(`node ${__filename} ${okDir}`); + if (okResult !== 0) { console.error("Self test failed, expected 0 for okDir"); process.exit(1); } // Run this script in the badDir, it should fail (return value 1) - const badResult = exec(`node ${__filename} ${badDir}`); - if (badResult) { + const badResult = exitCode(`node ${__filename} ${badDir}`); + if (badResult !== 1) { + console.error("Self test failed, expected 1 for badDir"); process.exit(1); } @@ -114,12 +113,13 @@ function selfTest() { return tmpFolderPath; } - function exec(args) { + function exitCode(args) { + const {execSync} = require('child_process'); try { - sys.execSync(args); - return true; + execSync(args); // will throw if exit code is not 0 + return 0; } catch (e) { - return false; + return 1; } } } \ No newline at end of file