Merge branch 'develop' of github.com:ether/etherpad-lite into npm

pull/5763/merge^2
John McLear 2023-06-26 19:33:53 +01:00
commit b911d7adcf
20 changed files with 347 additions and 51 deletions

View File

@ -18,7 +18,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node: [14, 16, 18]
node: [16, 18, 20]
steps:
-
name: Checkout repository
@ -55,7 +55,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node: [14, 16, 18]
node: [16, 18, 20]
steps:
-
name: Checkout repository
@ -124,7 +124,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -157,7 +157,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json

View File

@ -14,7 +14,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node: [14, 16, 18]
node: [16, 18, 20]
steps:
-

View File

@ -34,7 +34,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -98,7 +98,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -126,7 +126,7 @@ jobs:
# Etherpad core dependencies must be installed after installing the
# plugin's dependencies, otherwise npm will try to hoist common
# dependencies by removing them from src/node_modules and installing them
# in the top-level node_modules. As of v6.14.10, npm's hoist logic appears
# in the top-level node_modules. As of v6.20.10, npm's hoist logic appears
# to be buggy, because it sometimes removes dependencies from
# src/node_modules but fails to add them to the top-level node_modules.
# Even if npm correctly hoists the dependencies, the hoisting seems to

View File

@ -22,7 +22,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json

View File

@ -22,7 +22,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -52,7 +52,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -109,7 +109,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json

View File

@ -22,7 +22,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json

View File

@ -18,7 +18,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node: [14, 16, 18]
node: [16, 18, 20]
steps:
-
name: Check out latest release

View File

@ -28,7 +28,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
src/package-lock.json
@ -108,7 +108,7 @@ jobs:
-
uses: actions/setup-node@v3
with:
node-version: 14
node-version: 20
cache: 'npm'
cache-dependency-path: |
etherpad/src/package-lock.json

View File

@ -1,3 +1,14 @@
# Next release
### Notable enhancements and fixes
* Security
* Limit requested revisions in timeslider and export to head revision. (affects v1.9.0)
* Bugfixes
* revisions in `CHANGESET_REQ` (timeslider) and export (txt, html, custom)
are now checked to be numbers.
# 1.9.0
### Notable enhancements and fixes

View File

@ -46,7 +46,7 @@ We're looking for maintainers and have some funding available. Please contact J
### Requirements
[Node.js](https://nodejs.org/) >= **14.0.0**.
[Node.js](https://nodejs.org/) >= **16.20.1**.
### GNU/Linux and other UNIX-like systems

View File

@ -33,6 +33,7 @@ const exportTxt = require('../utils/ExportTxt');
const importHtml = require('../utils/ImportHtml');
const cleanText = require('./Pad').cleanText;
const PadDiff = require('../utils/padDiff');
const { checkValidRev, isInt } = require('../utils/checkValidRev');
/* ********************
* GROUP FUNCTIONS ****
@ -777,6 +778,13 @@ exports.createDiffHTML = async (padID, startRev, endRev) => {
// get the pad
const pad = await getPadSafe(padID, true);
const headRev = pad.getHeadRevisionNumber();
if (startRev > headRev)
startRev = headRev;
if (endRev > headRev)
endRev = headRev;
let padDiff;
try {
padDiff = new PadDiff(pad, startRev, endRev);
@ -822,9 +830,6 @@ exports.getStats = async () => {
** INTERNAL HELPER FUNCTIONS *
**************************** */
// checks if a number is an int
const isInt = (value) => (parseFloat(value) === parseInt(value, 10)) && !isNaN(value);
// gets a pad safe
const getPadSafe = async (padID, shouldExist, text, authorId = '') => {
// check if padID is a string
@ -854,31 +859,6 @@ const getPadSafe = async (padID, shouldExist, text, authorId = '') => {
return padManager.getPad(padID, text, authorId);
};
// checks if a rev is a legal number
// pre-condition is that `rev` is not undefined
const checkValidRev = (rev) => {
if (typeof rev !== 'number') {
rev = parseInt(rev, 10);
}
// check if rev is a number
if (isNaN(rev)) {
throw new CustomError('rev is not a number', 'apierror');
}
// ensure this is not a negative number
if (rev < 0) {
throw new CustomError('rev is not a negative number', 'apierror');
}
// ensure this is not a float value
if (!isInt(rev)) {
throw new CustomError('rev is a float value', 'apierror');
}
return rev;
};
// checks if a padID is part of a group
const checkGroupPad = (padID, field) => {
// ensure this is a group pad

View File

@ -172,6 +172,9 @@ class Pad {
async getInternalRevisionAText(targetRev) {
const keyRev = this.getKeyRevisionNumber(targetRev);
const headRev = this.getHeadRevisionNumber();
if (targetRev > headRev)
targetRev = headRev;
const [keyAText, changesets] = await Promise.all([
this._getKeyRevisionAText(keyRev),
Promise.all(

View File

@ -29,6 +29,7 @@ const os = require('os');
const hooks = require('../../static/js/pluginfw/hooks');
const TidyHtml = require('../utils/TidyHtml');
const util = require('util');
const { checkValidRev } = require('../utils/checkValidRev');
const fsp_writeFile = util.promisify(fs.writeFile);
const fsp_unlink = util.promisify(fs.unlink);
@ -53,6 +54,12 @@ exports.doExport = async (req, res, padId, readOnlyId, type) => {
// tell the browser that this is a downloadable file
res.attachment(`${fileName}.${type}`);
if (req.params.rev !== undefined) {
// ensure revision is a number
// modify req, as we use it in a later call to exportConvert
req.params.rev = checkValidRev(req.params.rev);
}
// if this is a plain text export, we can do this directly
// We have to over engineer this because tabs are stored as attributes and not plain text
if (type === 'etherpad') {

View File

@ -39,6 +39,7 @@ const stats = require('../stats');
const assert = require('assert').strict;
const {RateLimiterMemory} = require('rate-limiter-flexible');
const webaccess = require('../hooks/express/webaccess');
const { checkValidRev } = require('../utils/checkValidRev');
let rateLimiter;
let socketio = null;
@ -1076,10 +1077,14 @@ const handleChangesetRequest = async (socket, {data: {granularity, start, reques
if (granularity == null) throw new Error('missing granularity');
if (!Number.isInteger(granularity)) throw new Error('granularity is not an integer');
if (start == null) throw new Error('missing start');
start = checkValidRev(start);
if (requestID == null) throw new Error('mising requestID');
const end = start + (100 * granularity);
const {padId, author: authorId} = sessioninfos[socket.id];
const pad = await padManager.getPad(padId, null, authorId);
const headRev = pad.getHeadRevisionNumber();
if (start > headRev)
start = headRev;
const data = await getChangesetInfo(pad, start, end, granularity);
data.requestID = requestID;
socket.json.send({type: 'CHANGESET_REQ', data});

View File

@ -21,10 +21,14 @@
const AttributeMap = require('../../static/js/AttributeMap');
const Changeset = require('../../static/js/Changeset');
const { checkValidRev } = require('./checkValidRev');
/*
* This method seems unused in core and no plugins depend on it
*/
exports.getPadPlainText = (pad, revNum) => {
const _analyzeLine = exports._analyzeLine;
const atext = ((revNum !== undefined) ? pad.getInternalRevisionAText(revNum) : pad.atext);
const atext = ((revNum !== undefined) ? pad.getInternalRevisionAText(checkValidRev(revNum)) : pad.atext);
const textLines = atext.text.slice(0, -1).split('\n');
const attribLines = Changeset.splitAttributionLines(atext.attribs, atext.text);
const apool = pad.pool;

View File

@ -0,0 +1,34 @@
'use strict';
const CustomError = require('../utils/customError');
// checks if a rev is a legal number
// pre-condition is that `rev` is not undefined
const checkValidRev = (rev) => {
if (typeof rev !== 'number') {
rev = parseInt(rev, 10);
}
// check if rev is a number
if (isNaN(rev)) {
throw new CustomError('rev is not a number', 'apierror');
}
// ensure this is not a negative number
if (rev < 0) {
throw new CustomError('rev is not a negative number', 'apierror');
}
// ensure this is not a float value
if (!isInt(rev)) {
throw new CustomError('rev is a float value', 'apierror');
}
return rev;
};
// checks if a number is an int
const isInt = (value) => (parseFloat(value) === parseInt(value, 10)) && !isNaN(value);
exports.isInt = isInt;
exports.checkValidRev = checkValidRev;

6
src/package-lock.json generated
View File

@ -7331,9 +7331,9 @@
}
},
"semver": {
"version": "7.5.2",
"resolved": "https://registry.npmjs.org/semver/-/semver-7.5.2.tgz",
"integrity": "sha512-SoftuTROv/cRjCze/scjGyiDtcUyxw1rgYQSZY7XTmtR5hX+dm76iDbTH8TkLPHCQmlbQVSSbNZCPM2hb0knnQ==",
"version": "7.5.3",
"resolved": "https://registry.npmjs.org/semver/-/semver-7.5.3.tgz",
"integrity": "sha512-QBlUtyVk/5EeHbi7X0fw6liDZc7BBmEaSYn01fMU1OUYbf6GPsbTtd8WmnqbI20SeycoHSeiybkE/q1Q+qlThQ==",
"requires": {
"lru-cache": "^6.0.0"
}

View File

@ -61,7 +61,7 @@
"request": "2.88.2",
"resolve": "1.22.2",
"security": "1.0.0",
"semver": "^7.5.2",
"semver": "^7.5.3",
"socket.io": "^2.4.1",
"superagent": "^8.0.9",
"terser": "^5.18.1",
@ -93,7 +93,7 @@
"typescript": "^4.9.5"
},
"engines": {
"node": ">=14.15.0",
"node": ">=16.20.1",
"npm": ">=6.14.0"
},
"repository": {

View File

@ -447,6 +447,175 @@ describe(__filename, function () {
});
});
describe('revisions are supported in txt and html export', function () {
const makeGoodExport = () => ({
'pad:testing': {
atext: {
text: 'oofoo\n',
attribs: '|1+6',
},
pool: {
numToAttrib: {
0: ['author', 'a.foo'],
},
nextNum: 1,
},
head: 2,
savedRevisions: [],
},
'globalAuthor:a.foo': {
colorId: '#000000',
name: 'author foo',
timestamp: 1598747784631,
padIDs: 'testing',
},
'pad:testing:revs:0': {
changeset: 'Z:1>3+3$foo',
meta: {
author: 'a.foo',
timestamp: 1597632398288,
pool: {
nextNum: 1,
numToAttrib: {
0: ['author', 'a.foo'],
},
},
atext: {
text: 'foo\n',
attribs: '|1+4',
},
},
},
'pad:testing:revs:1': {
changeset: 'Z:4>1+1$o',
meta: {
author: 'a.foo',
timestamp: 1597632398288,
pool: {
nextNum: 1,
numToAttrib: {
0: ['author', 'a.foo'],
},
},
atext: {
text: 'fooo\n',
attribs: '*0|1+5',
},
},
},
'pad:testing:revs:2': {
changeset: 'Z:5>1+1$o',
meta: {
author: 'a.foo',
timestamp: 1597632398288,
pool: {
numToAttrib: {},
nextNum: 0,
},
atext: {
text: 'foooo\n',
attribs: '*0|1+6',
},
},
},
});
const importEtherpad = (records) => agent.post(`/p/${testPadId}/import`)
.attach('file', Buffer.from(JSON.stringify(records), 'utf8'), {
filename: '/test.etherpad',
contentType: 'application/etherpad',
});
before(async function () {
// makeGoodExport() is assumed to produce good .etherpad records. Verify that assumption so
// that a buggy makeGoodExport() doesn't cause checks to accidentally pass.
const records = makeGoodExport();
await deleteTestPad();
await importEtherpad(records)
.expect(200)
.expect('Content-Type', /json/)
.expect((res) => assert.deepEqual(res.body, {
code: 0,
message: 'ok',
data: {directDatabaseAccess: true},
}));
await agent.get(`/p/${testPadId}/export/txt`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.equal(res.text, 'oofoo\n'));
});
it('txt request rev 1', async function () {
await agent.get(`/p/${testPadId}/1/export/txt`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.equal(res.text, 'ofoo\n'));
});
it('txt request rev 2', async function () {
await agent.get(`/p/${testPadId}/2/export/txt`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.equal(res.text, 'oofoo\n'));
});
it('txt request rev 1test returns rev 1', async function () {
await agent.get(`/p/${testPadId}/1test/export/txt`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.equal(res.text, 'ofoo\n'));
});
it('txt request rev test1 is 403', async function () {
await agent.get(`/p/${testPadId}/test1/export/txt`)
.expect(500)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /rev is not a number/));
});
it('txt request rev 5 returns head rev', async function () {
await agent.get(`/p/${testPadId}/5/export/txt`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.equal(res.text, 'oofoo\n'));
});
it('html request rev 1', async function () {
await agent.get(`/p/${testPadId}/1/export/html`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /ofoo<br>/));
});
it('html request rev 2', async function () {
await agent.get(`/p/${testPadId}/2/export/html`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /oofoo<br>/));
});
it('html request rev 1test returns rev 1', async function () {
await agent.get(`/p/${testPadId}/1test/export/html`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /ofoo<br>/));
});
it('html request rev test1 results in 500 response', async function () {
await agent.get(`/p/${testPadId}/test1/export/html`)
.expect(500)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /rev is not a number/));
});
it('html request rev 5 returns head rev', async function () {
await agent.get(`/p/${testPadId}/5/export/html`)
.expect(200)
.buffer(true).parse(superagent.parse.text)
.expect((res) => assert.match(res.text, /oofoo<br>/));
});
});
describe('Import authorization checks', function () {
let authorize;

View File

@ -77,6 +77,89 @@ describe(__filename, function () {
await otherPad.remove();
}
});
it('CHANGESET_REQ: verify revNum is a number (regression)', async function () {
const otherPadId = `${padId}other`;
assert(!await padManager.doesPadExist(otherPadId));
const otherPad = await padManager.getPad(otherPadId, 'other text\n');
let errorCatched = 0;
try {
await otherPad.setText('other text\n');
await common.sendMessage(roSocket, {
component: 'pad',
padId: otherPadId, // The server should ignore this.
type: 'CHANGESET_REQ',
data: {
granularity: 1,
start: 'test123',
requestID: 'requestId',
},
});
assert.equal('This code should never run', 1);
}
catch(e) {
assert.match(e.message, /rev is not a number/);
errorCatched = 1;
}
finally {
await otherPad.remove();
assert.equal(errorCatched, 1);
}
});
it('CHANGESET_REQ: revNum is converted to number if possible (regression)', async function () {
const otherPadId = `${padId}other`;
assert(!await padManager.doesPadExist(otherPadId));
const otherPad = await padManager.getPad(otherPadId, 'other text\n');
try {
await otherPad.setText('other text\n');
const resP = common.waitForSocketEvent(roSocket, 'message');
await common.sendMessage(roSocket, {
component: 'pad',
padId: otherPadId, // The server should ignore this.
type: 'CHANGESET_REQ',
data: {
granularity: 1,
start: '1test123',
requestID: 'requestId',
},
});
const res = await resP;
assert.equal(res.type, 'CHANGESET_REQ');
assert.equal(res.data.requestID, 'requestId');
assert.equal(res.data.start, 1);
}
finally {
await otherPad.remove();
}
});
it('CHANGESET_REQ: revNum 2 is converted to head rev 1 (regression)', async function () {
const otherPadId = `${padId}other`;
assert(!await padManager.doesPadExist(otherPadId));
const otherPad = await padManager.getPad(otherPadId, 'other text\n');
try {
await otherPad.setText('other text\n');
const resP = common.waitForSocketEvent(roSocket, 'message');
await common.sendMessage(roSocket, {
component: 'pad',
padId: otherPadId, // The server should ignore this.
type: 'CHANGESET_REQ',
data: {
granularity: 1,
start: '2',
requestID: 'requestId',
},
});
const res = await resP;
assert.equal(res.type, 'CHANGESET_REQ');
assert.equal(res.data.requestID, 'requestId');
assert.equal(res.data.start, 1);
}
finally {
await otherPad.remove();
}
});
});
describe('USER_CHANGES', function () {