Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 3 additions & 34 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 20 additions & 1 deletion src/node/utils/AbsolutePaths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* limitations under the License.
*/
import log4js from 'log4js';
import fs from 'fs';
import path from 'path';
import _ from 'underscore';

Expand All @@ -30,6 +31,25 @@ const absPathLogger = log4js.getLogger('AbsolutePaths');
*/
let etherpadRoot: string|null = null;

/**
* Walks up the directory tree from `start`, returning the closest ancestor
* directory (including `start` itself) that contains a package.json. Replaces
* the unmaintained `find-root` package, mirroring its semantics: it throws if
* no package.json is found before reaching the filesystem root.
*
* @param {string} start - The directory to start searching from.
* @return {string} The closest ancestor directory containing a package.json.
*/
const findRoot = (start: string): string => {
let dir = start;
for (;;) {
if (fs.existsSync(path.join(dir, 'package.json'))) return dir;
const parent = path.dirname(dir);
if (parent === dir) throw new Error('package.json not found in path');
dir = parent;
}
};

/**
* If stringArray's last elements are exactly equal to lastDesiredElements,
* returns a copy in which those last elements are popped, or false otherwise.
Expand Down Expand Up @@ -79,7 +99,6 @@ export const findEtherpadRoot = () => {
return etherpadRoot;
}

const findRoot = require('find-root');
const foundRoot = findRoot(__dirname);
const splitFoundRoot = foundRoot.split(path.sep);

Expand Down
1 change: 0 additions & 1 deletion src/node/utils/Minify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const LIBRARY_WHITELIST = [
'split-grid',
'tinycon',
'underscore',
'unorm',
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
];

// What follows is a terrible hack to avoid loop-back within the server.
Expand Down
17 changes: 13 additions & 4 deletions src/node/utils/Settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {argv} from './Cli'
import jsonminify from 'jsonminify';
import {parse as parseJsonc, printParseErrorCode, ParseError} from 'jsonc-parser';
import log4js from 'log4js';
import {createHash} from 'node:crypto';
import randomString from './randomstring';
Expand Down Expand Up @@ -116,9 +116,18 @@ const parseSettings = (settingsFilename: string, isSettings: boolean) => {
}

try {
settingsStr = jsonminify(settingsStr).replace(',]', ']').replace(',}', '}');

const settings = JSON.parse(settingsStr);
// jsonc-parser tolerates comments and trailing commas, so settings files
// can stay annotated. Unlike the old jsonminify + naive ',]'/',}' string
// replace, it fixes *every* trailing comma (not just the first of each
// kind) and never mangles those sequences when they appear inside strings.
const errors: ParseError[] = [];
const settings = parseJsonc(settingsStr, errors, {allowTrailingComma: true});

if (errors.length > 0) {
const {error, offset} = errors[0];
throw new Error(`${printParseErrorCode(error)} at offset ${offset}`);
}
if (settings === undefined) throw new Error('file is empty or not valid JSON');

logger.info(`${settingsType} loaded from: ${settingsFilename}`);

Expand Down
1 change: 0 additions & 1 deletion src/node/utils/tar.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
, "skiplist.js"
, "colorutils.js"
, "undomodule.js"
, "$unorm/lib/unorm.js"
, "contentcollector.js"
, "changesettracker.js"
, "linestylefilter.js"
Expand Down
5 changes: 1 addition & 4 deletions src/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,14 @@
"express": "^5.2.1",
"express-rate-limit": "^8.5.1",
"express-session": "^1.19.0",
"find-root": "1.1.0",
"formidable": "^3.5.4",
"html-to-docx": "^1.8.0",
"htmlparser2": "^12.0.0",
"http-errors": "^2.0.1",
"jose": "^6.2.3",
"js-cookie": "^3.0.8",
"jsdom": "^29.1.1",
"jsonminify": "0.4.2",
"jsonc-parser": "^3.3.1",
"jsonwebtoken": "^9.0.3",
"jwt-decode": "^4.0.0",
"languages4translatewiki": "0.1.3",
Expand Down Expand Up @@ -90,7 +89,6 @@
"ueberdb2": "6.1.13",
"underscore": "1.13.8",
"undici": "^8.5.0",
"unorm": "1.6.0",
"wtfnode": "^0.10.1"
},
"bin": {
Expand All @@ -110,7 +108,6 @@
"@types/jquery": "^4.0.1",
"@types/js-cookie": "^3.0.6",
"@types/jsdom": "^28.0.3",
"@types/jsonminify": "^0.4.3",
"@types/jsonwebtoken": "^9.0.10",
"@types/mime-types": "^3.0.1",
"@types/mocha": "^10.0.9",
Expand Down
5 changes: 3 additions & 2 deletions src/static/js/contentcollector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ import Op from "./Op";
const _MAX_LIST_LEVEL = 16;

import AttributeMap from './AttributeMap';
import UNorm from 'unorm';
import {subattribution} from './Changeset';
import {SmartOpAssembler} from "./SmartOpAssembler";
const hooks = require('./pluginfw/hooks');

const sanitizeUnicode = (s) => UNorm.nfc(s);
// NFC-normalize via the native String API (replaces the unmaintained `unorm`
// polyfill; String.prototype.normalize is available in every supported runtime).
const sanitizeUnicode = (s: string) => s.normalize('NFC');
const tagName = (n) => n.tagName && n.tagName.toLowerCase();
// supportedElems are Supported natively within Etherpad and don't require a plugin
const supportedElems = new Set([
Expand Down
58 changes: 58 additions & 0 deletions src/tests/backend/specs/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,4 +278,62 @@ describe(__filename, function () {
assert.strictEqual(over!.privacy.pluginCatalog, false);
});
});

// Regression test for the jsonminify -> jsonc-parser migration.
// The old parser was `jsonminify(str).replace(',]', ']').replace(',}', '}')`,
// which had two correctness bugs that jsonc-parser (allowTrailingComma) fixes:
// 1. String#replace with a string needle only swaps the FIRST match, so a
// file with more than one trailing comma of the same kind stayed invalid.
// 2. The blind ',]' / ',}' replace also corrupted those byte sequences when
// they appeared *inside* a string value (e.g. a URL or literal text).
describe('JSONC settings parsing (jsonminify -> jsonc-parser)', function () {
const fs = require('fs');
const os = require('os');
let tmpFile: string;

const writeTmp = (contents: string) => {
tmpFile = path.join(os.tmpdir(), `ep-settings-jsonc-${process.pid}.json`);
fs.writeFileSync(tmpFile, contents);
return tmpFile;
};

afterEach(function () {
if (tmpFile) { try { fs.unlinkSync(tmpFile); } catch (e) { /* ignore */ } }
});

it('strips comments and tolerates multiple trailing commas', function () {
const file = writeTmp(`// leading line comment
/* block comment */
{
"list": [
"a",
"b",
],
"nested": [
[1, 2,],
[3, 4,],
],
"obj": {
"x": 1,
"y": 2,
},
}`);
const s: any = exportedForTestingOnly.parseSettings(file, true);
assert.deepEqual(s.list, ['a', 'b']);
assert.deepEqual(s.nested, [[1, 2], [3, 4]]);
assert.deepEqual(s.obj, {x: 1, y: 2});
});

it('does not corrupt ",]" / ",}" sequences inside string values', function () {
const file = writeTmp(`{
"url": "http://example.com/a,]b,}c",
"text": "trailing-comma-like ,] and ,} must survive"
}`);
const s: any = exportedForTestingOnly.parseSettings(file, true);
// The old replace() would have stripped the comma and produced
// "http://example.com/a]b}c" here.
assert.strictEqual(s.url, 'http://example.com/a,]b,}c');
assert.strictEqual(s.text, 'trailing-comma-like ,] and ,} must survive');
});
});
});
7 changes: 4 additions & 3 deletions src/tests/container/loadSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@
*/

const fs = require('fs');
const jsonminify = require('jsonminify');
const {parse: parseJsonc} = require('jsonc-parser');

function loadSettings() {
let settingsStr = fs.readFileSync(`${__dirname}/../../../settings.json.docker`).toString();
// try to parse the settings
try {
if (settingsStr) {
settingsStr = jsonminify(settingsStr).replace(',]', ']').replace(',}', '}');
const settings = JSON.parse(settingsStr);
// jsonc-parser tolerates the comments and trailing commas in the docker
// settings file, matching node/utils/Settings.ts.
const settings = parseJsonc(settingsStr, [], {allowTrailingComma: true});

// custom settings for running in a container
settings.ip = 'localhost';
Expand Down
Loading