Skip to content

Commit a9f583c

Browse files
JohnMcLearclaude
andcommitted
fix(export): /export/etherpad honors the :rev URL segment
Fixes #5071. `/p/:pad/:rev/export/etherpad` has always ignored the rev parameter and returned the full pad history, unlike the txt/html export endpoints which use the same route but do respect rev. Users wanting to back up or inspect a snapshot of a pad at a specific rev got every later revision in the payload instead — both wasteful and a surprise when the downloaded .etherpad blob contained content that had supposedly been reverted. Change: - `exportEtherpad.getPadRaw(padId, readOnlyId, revNum?)` now takes an optional revNum. When supplied, it clamps to `min(revNum, pad.head)`, iterates only revs 0..effectiveHead, and ships a shallow-cloned pad object whose `head` and `atext` reflect the requested snapshot. The original live Pad is still passed to the `exportEtherpad` hook so plugin callbacks see the real document. - `ExportHandler` passes `req.params.rev` through on the `etherpad` type, matching the existing behavior of `txt` and `html`. - Chat history is intentionally left full (it is not rev-anchored). Adds three backend regression tests under `ExportEtherpad.ts`: - default (no revNum) still exports the full history - explicit revNum limits exported revs and rewrites the serialized head so re-import reconstructs the pad at that rev - revNum above head is treated as full history, preventing accidental truncation of short pads Out of scope: `getHTML(padID, rev)` on the API side is already honoring rev in current code (exportHtml.getPadHTML threads the parameter through), so the earlier report on that API call appears to be resolved. This PR does not touch it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c4add02 commit a9f583c

3 files changed

Lines changed: 71 additions & 4 deletions

File tree

src/node/handler/ExportHandler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ exports.doExport = async (req: any, res: any, padId: string, readOnlyId: string,
6767
// if this is a plain text export, we can do this directly
6868
// We have to over engineer this because tabs are stored as attributes and not plain text
6969
if (type === 'etherpad') {
70-
const pad = await exportEtherpad.getPadRaw(padId, readOnlyId);
70+
// Honor the :rev URL segment on `.etherpad` exports the same way the
71+
// other formats already do — revNum limits the serialized pad to revs
72+
// 0..rev (issue #5071).
73+
const pad = await exportEtherpad.getPadRaw(padId, readOnlyId, req.params.rev);
7174
res.send(pad);
7275
} else if (type === 'txt') {
7376
const txt = await exporttxt.getPadTXTDocument(padId, req.params.rev);

src/node/utils/ExportEtherpad.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,21 @@ const authorManager = require('../db/AuthorManager');
2121
const hooks = require('../../static/js/pluginfw/hooks');
2222
const padManager = require('../db/PadManager');
2323

24-
exports.getPadRaw = async (padId:string, readOnlyId:string) => {
24+
exports.getPadRaw = async (padId:string, readOnlyId:string, revNum?: number) => {
2525
const dstPfx = `pad:${readOnlyId || padId}`;
2626
const [pad, customPrefixes] = await Promise.all([
2727
padManager.getPad(padId),
2828
hooks.aCallAll('exportEtherpadAdditionalContent'),
2929
]);
30+
// If a rev limit was supplied, clamp to it and also clamp chat to the
31+
// timestamp-ordered window that ended at that rev. Without this, a rev=5
32+
// export on a pad with head=100 would still ship all 95 later revisions
33+
// (and leak their content via the exported .etherpad file) — which is
34+
// precisely what issue #5071 reported.
35+
const padHead: number = pad.head;
36+
const effectiveHead: number = (revNum == null || revNum > padHead) ? padHead : revNum;
37+
const isRevBound = revNum != null && revNum < padHead;
38+
const boundAtext = isRevBound ? await pad.getInternalRevisionAText(effectiveHead) : null;
3039
const pluginRecords = await Promise.all(customPrefixes.map(async (customPrefix:string) => {
3140
const srcPfx = `${customPrefix}:${padId}`;
3241
const dstPfx = `${customPrefix}:${readOnlyId || padId}`;
@@ -49,11 +58,18 @@ exports.getPadRaw = async (padId:string, readOnlyId:string) => {
4958
return authorEntry;
5059
})()];
5160
}
52-
for (let i = 0; i <= pad.head; ++i) yield [`${dstPfx}:revs:${i}`, pad.getRevision(i)];
61+
for (let i = 0; i <= effectiveHead; ++i) yield [`${dstPfx}:revs:${i}`, pad.getRevision(i)];
5362
for (let i = 0; i <= pad.chatHead; ++i) yield [`${dstPfx}:chat:${i}`, pad.getChatMessage(i)];
5463
for (const gen of pluginRecords) yield* gen;
5564
})();
56-
const data = {[dstPfx]: pad};
65+
// When rev-bound, serialize a shallow-cloned pad object with head/atext
66+
// rewritten so the import side reconstructs the pad at the requested rev.
67+
// toJSON() returns a plain object suitable for spreading; the live Pad
68+
// instance is kept for the exportEtherpad hook below.
69+
const serializedPad = isRevBound
70+
? {...(pad.toJSON()), head: effectiveHead, atext: boundAtext}
71+
: pad;
72+
const data = {[dstPfx]: serializedPad};
5773
for (const [dstKey, p] of new Stream(records).batch(100).buffer(99)) data[dstKey] = await p;
5874
await hooks.aCallAll('exportEtherpad', {
5975
pad,

src/tests/backend/specs/ExportEtherpad.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,52 @@ describe(__filename, function () {
6464
assert(!(`custom:${padId}x:foo` in data));
6565
});
6666
});
67+
68+
// Regression test for https://github.com/ether/etherpad/issues/5071.
69+
// `/p/:pad/:rev/export/etherpad` and getPadRaw() historically ignored the
70+
// rev parameter and always exported the full history, surprising users
71+
// who wanted to back up or inspect an earlier snapshot.
72+
describe('revNum bounding (issue #5071)', function () {
73+
const addRevs = async (pad: any, n: number) => {
74+
// Each call to .appendRevision bumps head by one, producing a
75+
// distinct revision we can count in the exported payload.
76+
for (let i = 0; i < n; i++) {
77+
await pad.appendText(`line ${i}\n`);
78+
}
79+
};
80+
81+
it('defaults to full history when revNum is omitted', async function () {
82+
const pad = await padManager.getPad(padId);
83+
await addRevs(pad, 3);
84+
const data = await exportEtherpad.getPadRaw(padId, null);
85+
// revs 0 (pad-create) through pad.head inclusive.
86+
const revKeys =
87+
Object.keys(data).filter((k) => k.startsWith(`pad:${padId}:revs:`));
88+
assert.equal(revKeys.length, pad.head + 1);
89+
assert.equal(data[`pad:${padId}`].head, pad.head);
90+
});
91+
92+
it('limits exported revisions to 0..revNum when supplied', async function () {
93+
const pad = await padManager.getPad(padId);
94+
await addRevs(pad, 5);
95+
const bound = 2;
96+
const data = await exportEtherpad.getPadRaw(padId, null, bound);
97+
const revKeys =
98+
Object.keys(data).filter((k) => k.startsWith(`pad:${padId}:revs:`));
99+
assert.equal(revKeys.length, bound + 1,
100+
`expected ${bound + 1} revisions, got ${revKeys.length}`);
101+
assert(!(`pad:${padId}:revs:${bound + 1}` in data),
102+
'rev after bound must not be exported');
103+
// The serialized pad must also reflect the bounded head so that
104+
// re-importing reconstructs the pad at the requested rev.
105+
assert.equal(data[`pad:${padId}`].head, bound);
106+
});
107+
108+
it('treats a revNum above head as equivalent to full history', async function () {
109+
const pad = await padManager.getPad(padId);
110+
await addRevs(pad, 3);
111+
const data = await exportEtherpad.getPadRaw(padId, null, pad.head + 100);
112+
assert.equal(data[`pad:${padId}`].head, pad.head);
113+
});
114+
});
67115
});

0 commit comments

Comments
 (0)