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
6 changes: 6 additions & 0 deletions .changeset/revision-id-int32-bounds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@stll/folio-core": patch
"@stll/docx-core": patch
---

Keep tracked-change revision ids within the range OOXML consumers accept. Suggestion-mode edits seeded their `w:ins`/`w:del` id counter from the clock, producing 13-digit `w:id` values that made exported documents fail to open. Ids now continue from the document's own highest revision id. Port of eigenpal/docx-editor#1093.
6 changes: 6 additions & 0 deletions api-reports/docx-core/model.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,9 @@ export type MathEquation = {
plainText?: string;
};

// @public
export const MAX_REVISION_ID = 2147483647;

// @public
export type MediaFile = {
path: string; /** Original filename */
Expand Down Expand Up @@ -600,6 +603,9 @@ export type NoBreakHyphenContent = {
type: "noBreakHyphen";
};

// @public
export function normalizeRevisionId(id: number): number;

// @public
export type NoteNumberRestart = "continuous" | "eachSect" | "eachPage";

Expand Down
33 changes: 33 additions & 0 deletions packages/core/src/docx/paragraphParser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,39 @@ describe("parseParagraph tracked-change hardening", () => {
id: 5,
});
});

test("folds an out-of-range id on an inline w:ins into the int32 range (eigenpal #1093)", () => {
const OUT_OF_RANGE = "2147483654"; // MAX_REVISION_ID + 7
const paragraph = parseParagraphXml(`
<w:p xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
<w:ins w:id="${OUT_OF_RANGE}" w:author="A">
<w:r><w:t>added</w:t></w:r>
</w:ins>
</w:p>
`);

const insertion = paragraph.content.find((c) => c.type === "insertion");
expect(insertion?.type).toBe("insertion");
if (!insertion || insertion.type !== "insertion") {
return;
}
expect(insertion.info.id).toBeLessThanOrEqual(2_147_483_647);
expect(insertion.info.id).toBeGreaterThanOrEqual(0);
});

test("folds an out-of-range id on a paragraph-mark w:ins (pPrMark)", () => {
const OUT_OF_RANGE = "2147483654";
const paragraph = parseParagraphXml(`
<w:p xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
<w:pPr><w:rPr><w:ins w:id="${OUT_OF_RANGE}" w:author="A"/></w:rPr></w:pPr>
<w:r><w:t>text</w:t></w:r>
</w:p>
`);

expect(paragraph.pPrMark).toBeDefined();
expect(paragraph.pPrMark?.info.id).toBeLessThanOrEqual(2_147_483_647);
expect(paragraph.pPrMark?.info.id).toBeGreaterThanOrEqual(0);
});
});

describe("parseParagraph rendered page break markers", () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/docx/paragraphParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import type {
TrackedChangeInfo,
MathEquation,
} from "../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { isValidHexId } from "../utils/hexId";
import {
parseBookmarkStart as parseBookmarkStartFromModule,
Expand Down Expand Up @@ -954,7 +955,9 @@ function parseTrackedChangeInfo(node: XmlElement): TrackedChangeInfo {
const initials = (getAttribute(node, "w", "initials") ?? "").trim();

const info: TrackedChangeInfo = {
id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
// `w:id` is attacker-controlled and unbounded in the schema; fold it into
// the range consumers accept at the parse boundary (eigenpal #1093).
id: normalizeRevisionId(parsedId),
author: author.length > 0 ? author : "Unknown",
};
if (date.length > 0) {
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/docx/runParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import type {
MediaFile,
ShapeContent,
} from "../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { parseGroupDrawing } from "./groupDrawingParser";
import { parseImage } from "./imageParser";
import {
Expand Down Expand Up @@ -656,7 +657,9 @@ function parsePropertyChangeInfo(changeElement: XmlElement): RunPropertyChange["
const rsid = (getAttribute(changeElement, "w", "rsid") ?? "").trim();

const info: RunPropertyChange["info"] = {
id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
// `w:id` is attacker-controlled and unbounded in the schema; fold at the
// parse boundary (eigenpal #1093).
id: normalizeRevisionId(parsedId),
author: author.length > 0 ? author : "Unknown",
};
if (date.length > 0) {
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/docx/sectionParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import type {
BorderSpec,
ColorValue,
} from "../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { parseHeaderReference, parseFooterReference } from "./headerFooterRefParser";
import { parseFootnoteProperties, parseEndnoteProperties } from "./notePropertiesParser";
import { BorderStyleSchema, ThemeColorSlotSchema, narrowEnum } from "./parserEnums";
Expand Down Expand Up @@ -142,7 +143,9 @@ function parsePropertyChangeInfo(node: XmlElement): SectionPropertyChange["info"
const rsid = (getAttribute(node, "w", "rsid") ?? "").trim();

const info: SectionPropertyChange["info"] = {
id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
// `w:id` is attacker-controlled and unbounded in the schema; fold at the
// parse boundary (eigenpal #1093).
id: normalizeRevisionId(parsedId),
author: author.length > 0 ? author : "Unknown",
};
if (date.length > 0) {
Expand Down
38 changes: 38 additions & 0 deletions packages/core/src/docx/serializer/paragraphSerializer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,44 @@ describe("serializeParagraph tracked-change hardening", () => {
expect(xml).toContain('<w:ins w:id="0" w:author="Unknown">');
expect(xml).not.toContain("w:date=");
});

test("folds an out-of-range revision id into the signed 32-bit range (eigenpal #1093)", () => {
// A `Date.now()`-derived id (~1.8e12) is a well-formed positive integer, so
// the invalid/negative guard used to pass it straight through to `w:id`.
const paragraph: Paragraph = {
type: "paragraph",
content: [
{
type: "insertion",
info: { id: 1_784_212_345_678, author: "Reviewer" },
content: [{ type: "run", content: [{ type: "text", text: "Added" }] }],
},
],
};

const xml = serializeParagraph(paragraph);
const id = Number(/<w:ins w:id="(\d+)"/u.exec(xml)?.[1]);
expect(id).toBeLessThanOrEqual(2_147_483_647);
expect(id).toBeGreaterThanOrEqual(0);
});

test("keeps distinct out-of-range ids distinct so revisions do not merge", () => {
const idFor = (id: number): string => {
const paragraph: Paragraph = {
type: "paragraph",
content: [
{
type: "insertion",
info: { id, author: "Reviewer" },
content: [{ type: "run", content: [{ type: "text", text: "Added" }] }],
},
],
};
return /<w:ins w:id="(\d+)"/u.exec(serializeParagraph(paragraph))?.[1] ?? "";
};

expect(idFor(1_784_212_345_678)).not.toBe(idFor(1_784_212_345_679));
});
});

describe("serializeParagraph native frame geometry", () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/docx/serializer/paragraphSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import type {
TextFormatting,
TrackedChangeInfo,
} from "../../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { isValidHexColor } from "../../utils/colorResolver";
import { numPrEqual } from "../numberingParser";
import { reconcileRawSdtPr } from "../sdtPropertiesPatch";
Expand Down Expand Up @@ -375,7 +376,8 @@ function serializeTrackedChangeAttrs(info: TrackedChangeInfo): string {
// NOTE: `w:initials` is intentionally NOT emitted — ECMA-376 CT_TrackChange
// defines only w:id/w:author/w:date. Initials are carried in-model for UI
// attribution only (w:comment is the sole standards-clean initials target).
const parts = [`w:id="${info.id}"`, `w:author="${escapeXml(info.author)}"`];
// Bound `w:id` here so an overflowing id cannot reach the XML (eigenpal #1093).
const parts = [`w:id="${normalizeRevisionId(info.id)}"`, `w:author="${escapeXml(info.author)}"`];
if (info.date !== undefined) {
parts.push(`w:date="${escapeXml(info.date)}"`);
}
Expand Down Expand Up @@ -547,7 +549,7 @@ function extractRPrInner(rPrXml: string): string {
}

function serializeParagraphPropertyChange(change: ParagraphPropertyChange): string {
const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
const normalizedId = normalizeRevisionId(change.info.id);
const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;
Expand Down Expand Up @@ -950,7 +952,7 @@ function serializeTrackedChange(
change: Insertion | Deletion | MoveFrom | MoveTo,
): string {
const info = change.info;
const normalizedId = Number.isInteger(info.id) && info.id >= 0 ? info.id : 0;
const normalizedId = normalizeRevisionId(info.id);
const authorCandidate = typeof info.author === "string" ? info.author.trim() : "";
const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
const normalizedDate = typeof info.date === "string" ? info.date.trim() : undefined;
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/docx/serializer/runSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type {
BlockContent,
RunPropertyChange,
} from "../../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { HIGHLIGHT_COLOR_VALUES } from "../../types/documentEnumValues";
import { isValidHexColor } from "../../utils/colorResolver";
// oxlint-disable-next-line import/no-cycle -- OOXML model is mutually recursive: shape textboxes hold paragraphs, paragraphs hold runs
Expand Down Expand Up @@ -444,7 +445,7 @@ function extractRPrInner(rPrXml: string): string {
}

function serializeRunPropertyChange(change: RunPropertyChange): string {
const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
const normalizedId = normalizeRevisionId(change.info.id);
const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
SectionPropertyChange,
SectionProperties,
} from "../../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { getUnserializedSectionPropertyChildNames } from "../sectionParser";
import { serializeBorder } from "./borderSerializer";
import { escapeXml, intAttr } from "./xmlUtils";
Expand Down Expand Up @@ -303,7 +304,7 @@ function serializeOnOffElement(value: boolean | undefined, name: string): string
}

function serializeSectionPropertyChange(change: SectionPropertyChange): string {
const normalizedId = Number.isInteger(change.info.id) && change.info.id >= 0 ? change.info.id : 0;
const normalizedId = normalizeRevisionId(change.info.id);
const authorCandidate = typeof change.info.author === "string" ? change.info.author.trim() : "";
const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
const normalizedDate = typeof change.info.date === "string" ? change.info.date.trim() : undefined;
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/docx/serializer/tableSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import type {
ShadingProperties,
Paragraph,
} from "../../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { isValidHexColor } from "../../utils/colorResolver";
import { serializeBorder } from "./borderSerializer";
import { escapeXml, intAttr } from "./xmlUtils";
Expand All @@ -47,7 +48,7 @@ function normalizeTrackedChangeInfo(info: { id: number; author: string; date?: s
author: string;
date?: string;
} {
const normalizedId = Number.isInteger(info.id) && info.id >= 0 ? info.id : 0;
const normalizedId = normalizeRevisionId(info.id);
const authorCandidate = typeof info.author === "string" ? info.author.trim() : "";
const normalizedAuthor = authorCandidate.length > 0 ? authorCandidate : "Unknown";
const normalizedDate = typeof info.date === "string" ? info.date.trim() : undefined;
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/docx/tableParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import type {
BookmarkEnd,
BookmarkStart,
} from "../types/document";
import { normalizeRevisionId } from "@stll/docx-core/model";
import { parseBookmarkEnd, parseBookmarkStart } from "./bookmarkParser";
import {
appendBookmarkMarkerToLastParagraphInBlocks,
Expand Down Expand Up @@ -133,7 +134,9 @@ function parseTrackedChangeInfo(node: XmlElement): TableStructuralChangeInfo["in
const initials = (getAttribute(node, "w", "initials") ?? "").trim();

const info: TableStructuralChangeInfo["info"] = {
id: Number.isInteger(parsedId) && parsedId >= 0 ? parsedId : 0,
// `w:id` is attacker-controlled and unbounded in the schema; fold at the
// parse boundary (eigenpal #1093).
id: normalizeRevisionId(parsedId),
author: author.length > 0 ? author : "Unknown",
};
if (date.length > 0) {
Expand Down
38 changes: 33 additions & 5 deletions packages/core/src/prosemirror/commentIdAllocator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@
*/

import type { EditorView } from "prosemirror-view";

import { MAX_REVISION_ID } from "@stll/docx-core/model";

import type { Comment } from "../types/content";
import { seedRevisionIdsAbove } from "./plugins/revisionIds";

/** Sentinel ID for a comment that hasn't been persisted yet (anchored to selection). */
export const PENDING_COMMENT_ID = -1;
Expand All @@ -37,9 +41,19 @@ export type CommentIdAllocator = {
export function createCommentIdAllocator(): CommentIdAllocator {
let nextId = 1;
return {
next: () => nextId++,
next: () => {
if (nextId > MAX_REVISION_ID) {
nextId = 1;
}
return nextId++;
},
seedAbove(maxId: number) {
if (maxId >= nextId) nextId = maxId + 1;
if (!Number.isInteger(maxId) || maxId < 0 || maxId >= MAX_REVISION_ID) {
return;
}
if (maxId >= nextId) {
nextId = maxId + 1;
}
},
};
}
Expand All @@ -58,14 +72,28 @@ export function seedCommentAllocator(
view: EditorView | null,
): void {
let max = 0;
for (const comment of comments ?? []) max = Math.max(max, comment.id);
for (const comment of comments ?? []) {
if (Number.isInteger(comment.id) && comment.id > max && comment.id <= MAX_REVISION_ID) {
max = comment.id;
}
}
if (view) {
view.state.doc.descendants((node) => {
for (const mark of node.marks) {
if (mark.attrs["revisionId"] != null)
max = Math.max(max, mark.attrs["revisionId"] as number);
const revisionId = mark.attrs["revisionId"];
if (
typeof revisionId === "number" &&
Number.isInteger(revisionId) &&
revisionId > max &&
revisionId <= MAX_REVISION_ID
) {
max = revisionId;
}
}
});
}
allocator.seedAbove(max);
// Keep the tracked-revision counter in the same shared OOXML id space so
// suggestion-mode mints cannot collide with comment ids (eigenpal #1093).
seedRevisionIdsAbove(max);
}
Loading
Loading