Skip to content

Commit 305dc0f

Browse files
fix(pptx): emit a structurally valid package on serialize (#92)
Three serializeDeck bugs corrupted the generated .pptx (missing parts / invalid image bytes) even from clean source templates, triggering a PowerPoint repair prompt and rejection by stricter consumers (Google Slides, LibreOffice, OOXML validators): - Dangling tags relationships: the chrome-preserve path re-pointed a slide's tag rel at a slidewise_preserved_* name, then clobbered that part by re-copying the source tags under their original names. The rel now resolves to the de-prefixed part it should always have pointed at. - Dangling notesMaster relationships: pptxgenjs writes a notesSlide per slide linked to a notes master, which chrome preservation removed without a source replacement. The orphaned (implicit, non-body- referenced) relationship is now dropped. - SVG markup in .png raster fallbacks: dual SVG images had the SVG source written into the .png fallback. It is now a real rasterized PNG (browser) or a valid transparent PNG (SSR/Node); the svgBlip is intact. Adds a final reconcileDanglingRels invariant guard (repair recoverable targets, drop only safe-to-remove optional ones, keep critical rels), and runs pruneDanglingContentTypes on the source path so stale Content_Types overrides can't invalidate the package either. Includes unit, SVG, and a whole-corpus validity test net.
1 parent 2fd7a26 commit 305dc0f

6 files changed

Lines changed: 847 additions & 0 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@textcortex/slidewise": patch
3+
---
4+
5+
fix(pptx): emit a structurally valid package on serialize
6+
7+
Three `serializeDeck` bugs corrupted the generated `.pptx` (missing parts /
8+
invalid image bytes) even from clean source templates, triggering a PowerPoint
9+
repair prompt and outright rejection by stricter consumers (Google Slides,
10+
LibreOffice, OOXML validators):
11+
12+
- **Dangling `tags` relationships:** the chrome-preserve path re-pointed a
13+
slide's tag rel at a `slidewise_preserved_*` name, then clobbered that part by
14+
re-copying the source tags under their original names. The rel now resolves
15+
to the de-prefixed part it should always have pointed at.
16+
- **Dangling `notesMaster` relationships:** pptxgenjs writes a notesSlide per
17+
slide linked to a notes master, which chrome preservation removed without a
18+
source replacement. The orphaned (implicit, non-body-referenced) relationship
19+
is now dropped.
20+
- **SVG markup in `.png` raster fallbacks:** dual SVG images (`<a:blip>` raster
21+
+ `<asvg:svgBlip>` vector) had the SVG source written into the `.png`
22+
fallback. The fallback is now a real rasterized PNG (browser) or a valid
23+
transparent PNG (SSR/Node); the vector `svgBlip` part is untouched.
24+
25+
Adds a final `reconcileDanglingRels` invariant guard — every internal
26+
relationship target must resolve to a shipped part — that backstops both
27+
dangling-rel shapes (repairing recoverable targets, dropping only
28+
safe-to-remove optional ones, and leaving critical rels untouched). Also runs
29+
`pruneDanglingContentTypes` on the source-preservation path so stale
30+
`[Content_Types]` overrides (pptxgenjs's `slideMaster1..N`, leftover notes
31+
overrides) can't invalidate the package either.

packages/slidewise/src/lib/pptx/__tests__/chrome-preservation.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ async function loadFixture(name: string): Promise<ArrayBuffer> {
3535
const hasEon = await fixtureExists("eon-deck.pptx");
3636
const hasDickinson = await fixtureExists("Dickinson_Sample_Slides.pptx");
3737

38+
// Templates from the dangling-rel bug report (1a tags / 1b notesMaster).
39+
const DANGLING_FIXTURES = [
40+
"Intero Master Template.pptx",
41+
"Intero.pptx",
42+
"44 - Education.pptx",
43+
"eon-deck.pptx",
44+
"Dickinson_Sample_Slides.pptx",
45+
] as const;
46+
const danglingFixtures = (
47+
await Promise.all(
48+
DANGLING_FIXTURES.map(async (name) =>
49+
(await fixtureExists(name)) ? name : null
50+
)
51+
)
52+
).filter((n): n is (typeof DANGLING_FIXTURES)[number] => n !== null);
53+
3854
async function listZipPaths(buf: ArrayBuffer | Blob): Promise<Set<string>> {
3955
const ab = buf instanceof Blob ? await buf.arrayBuffer() : buf;
4056
const zip = await JSZip.loadAsync(ab);
@@ -43,6 +59,52 @@ async function listZipPaths(buf: ArrayBuffer | Blob): Promise<Set<string>> {
4359
return paths;
4460
}
4561

62+
/**
63+
* Walk every `*.rels` in the package and return the internal relationship
64+
* targets that don't resolve to a shipped part. A non-empty result means the
65+
* output would trigger a PowerPoint repair prompt / strict-consumer rejection.
66+
*/
67+
async function findDanglingRels(
68+
buf: Blob | ArrayBuffer
69+
): Promise<Array<{ rels: string; id: string; target: string }>> {
70+
const ab = buf instanceof Blob ? await buf.arrayBuffer() : buf;
71+
const zip = await JSZip.loadAsync(ab);
72+
const present = new Set<string>();
73+
const relsPaths: string[] = [];
74+
zip.forEach((p, e) => {
75+
if (e.dir) return;
76+
present.add(p);
77+
if (p.endsWith(".rels")) relsPaths.push(p);
78+
});
79+
const normalise = (target: string, base: string): string => {
80+
if (target.startsWith("/")) return target.slice(1);
81+
let t = target;
82+
const segs = base.split("/").filter(Boolean);
83+
while (t.startsWith("../")) {
84+
segs.pop();
85+
t = t.slice(3);
86+
}
87+
return [...segs, t].filter(Boolean).join("/");
88+
};
89+
const dangling: Array<{ rels: string; id: string; target: string }> = [];
90+
for (const relsPath of relsPaths) {
91+
const xml = await zip.file(relsPath)!.async("string");
92+
const ownerDir = relsPath.replace(/(^|\/)_rels\/[^/]+$/, "");
93+
const re = /<Relationship\b[^>]*?\/>/g;
94+
let m: RegExpExecArray | null;
95+
while ((m = re.exec(xml))) {
96+
const tag = m[0];
97+
const mode = /\bTargetMode="([^"]+)"/.exec(tag)?.[1];
98+
const target = /\bTarget="([^"]+)"/.exec(tag)?.[1];
99+
const id = /\bId="([^"]+)"/.exec(tag)?.[1] ?? "?";
100+
if (!target || mode === "External" || /^https?:\/\//i.test(target)) continue;
101+
const full = normalise(target, ownerDir === relsPath ? "" : ownerDir);
102+
if (!present.has(full)) dangling.push({ rels: relsPath, id, target });
103+
}
104+
}
105+
return dangling;
106+
}
107+
46108
async function countSlidesWithSpTreeChildren(
47109
buf: Blob
48110
): Promise<number> {
@@ -184,4 +246,23 @@ describe("deck chrome preservation", () => {
184246
expect(fontCount).toBe(5);
185247
}
186248
);
249+
250+
// Package invariant: no internal relationship may point at a missing part.
251+
// Catches the tags (1a) and notesMaster (1b) danglers the chrome-preserve
252+
// path used to emit. Runs per available fixture (skipped in CI where the
253+
// branded decks aren't committed).
254+
for (const name of danglingFixtures) {
255+
it(`emits no dangling internal relationships (${name})`, async () => {
256+
const source = await loadFixture(name);
257+
const deck = await parsePptx(source);
258+
const blob = await serializeDeck(deck, { source });
259+
const dangling = await findDanglingRels(blob);
260+
expect(
261+
dangling,
262+
`dangling rels:\n${dangling
263+
.map((d) => ` ${d.rels} ${d.id}${d.target}`)
264+
.join("\n")}`
265+
).toEqual([]);
266+
});
267+
}
187268
});
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
import { describe, it, expect } from "vitest";
2+
import { readFile, readdir } from "node:fs/promises";
3+
import { fileURLToPath } from "node:url";
4+
import path from "node:path";
5+
import JSZip from "jszip";
6+
import { parsePptx, serializeDeck } from "../index";
7+
8+
/**
9+
* Whole-corpus structural-validity net. Drop any number of `.pptx` files into
10+
* the gitignored `.context/attachments/` dir (the same place the other fixture
11+
* tests read from) and this round-trips EVERY one through
12+
* `parsePptx → serializeDeck`, asserting the output is a structurally valid
13+
* OOXML package:
14+
*
15+
* 1. every internal relationship target resolves to a shipped part,
16+
* 2. every `[Content_Types]` Override points at a part that exists,
17+
* 3. every shipped part has a declared content type (Default by extension
18+
* or an explicit Override),
19+
* 4. every `ppt/media/*.png` holds real PNG bytes (never SVG markup).
20+
*
21+
* These are exactly the four ways the reported bugs corrupted the artifact.
22+
* The whole suite skips when no decks are present, so CI stays green for
23+
* outside contributors while it scans the full corpus locally.
24+
*/
25+
26+
const __filename = fileURLToPath(import.meta.url);
27+
const __dirname = path.dirname(__filename);
28+
const attachmentsDir = path.resolve(
29+
__dirname,
30+
"../../../../../../.context/attachments"
31+
);
32+
33+
async function findPptx(dir: string): Promise<string[]> {
34+
const out: string[] = [];
35+
let entries;
36+
try {
37+
entries = await readdir(dir, { withFileTypes: true });
38+
} catch {
39+
return out;
40+
}
41+
for (const e of entries) {
42+
const full = path.join(dir, e.name);
43+
if (e.isDirectory()) out.push(...(await findPptx(full)));
44+
else if (/\.(pptx|potx)$/i.test(e.name)) out.push(full);
45+
}
46+
return out;
47+
}
48+
49+
function normalise(target: string, base: string): string {
50+
if (target.startsWith("/")) return target.slice(1);
51+
let t = target;
52+
const segs = base.split("/").filter(Boolean);
53+
while (t.startsWith("../")) {
54+
segs.pop();
55+
t = t.slice(3);
56+
}
57+
return [...segs, t].filter(Boolean).join("/");
58+
}
59+
60+
const PNG_MAGIC = [0x89, 0x50, 0x4e, 0x47];
61+
62+
interface Problems {
63+
danglingRels: string[];
64+
danglingOverrides: string[];
65+
undeclaredParts: string[];
66+
invalidPngs: string[];
67+
}
68+
69+
async function validate(buf: ArrayBuffer): Promise<Problems> {
70+
const zip = await JSZip.loadAsync(buf);
71+
const present = new Set<string>();
72+
const relsPaths: string[] = [];
73+
const mediaPngs: string[] = [];
74+
zip.forEach((p, e) => {
75+
if (e.dir) return;
76+
present.add(p);
77+
if (p.endsWith(".rels")) relsPaths.push(p);
78+
if (/^ppt\/media\/.+\.png$/i.test(p)) mediaPngs.push(p);
79+
});
80+
81+
const problems: Problems = {
82+
danglingRels: [],
83+
danglingOverrides: [],
84+
undeclaredParts: [],
85+
invalidPngs: [],
86+
};
87+
88+
// 1. Relationship targets.
89+
for (const relsPath of relsPaths) {
90+
const xml = await zip.file(relsPath)!.async("string");
91+
const ownerDir = relsPath.replace(/(^|\/)_rels\/[^/]+$/, "");
92+
const base = ownerDir === relsPath ? "" : ownerDir;
93+
const re = /<Relationship\b[^>]*?\/>/g;
94+
let m: RegExpExecArray | null;
95+
while ((m = re.exec(xml))) {
96+
const tag = m[0];
97+
const mode = /\bTargetMode="([^"]+)"/.exec(tag)?.[1];
98+
const target = /\bTarget="([^"]+)"/.exec(tag)?.[1];
99+
const id = /\bId="([^"]+)"/.exec(tag)?.[1] ?? "?";
100+
if (!target || mode === "External" || /^https?:\/\//i.test(target)) continue;
101+
if (!present.has(normalise(target, base))) {
102+
problems.danglingRels.push(`${relsPath} ${id}${target}`);
103+
}
104+
}
105+
}
106+
107+
// 2 + 3. Content types.
108+
const ct = await zip.file("[Content_Types].xml")?.async("string");
109+
if (ct) {
110+
const defaults = new Set<string>();
111+
const overrides = new Set<string>();
112+
let dm: RegExpExecArray | null;
113+
const defRe = /<Default\b[^>]*Extension="([^"]+)"[^>]*\/>/g;
114+
while ((dm = defRe.exec(ct))) defaults.add(dm[1].toLowerCase());
115+
const ovRe = /<Override\b[^>]*PartName="([^"]+)"[^>]*\/>/g;
116+
let om: RegExpExecArray | null;
117+
while ((om = ovRe.exec(ct))) {
118+
overrides.add(om[1]);
119+
if (!present.has(om[1].replace(/^\//, ""))) {
120+
problems.danglingOverrides.push(om[1]);
121+
}
122+
}
123+
for (const p of present) {
124+
if (p === "[Content_Types].xml" || p.endsWith(".rels")) continue;
125+
const ext = (p.split(".").pop() ?? "").toLowerCase();
126+
if (defaults.has(ext) || overrides.has("/" + p)) continue;
127+
problems.undeclaredParts.push(p);
128+
}
129+
}
130+
131+
// 4. PNG media bytes.
132+
for (const p of mediaPngs) {
133+
const bytes = await zip.file(p)!.async("uint8array");
134+
const ok =
135+
bytes.length >= 4 && PNG_MAGIC.every((b, i) => bytes[i] === b);
136+
if (!ok) problems.invalidPngs.push(p);
137+
}
138+
139+
return problems;
140+
}
141+
142+
const decks = await findPptx(attachmentsDir);
143+
144+
describe.skipIf(decks.length === 0)("corpus structural validity", () => {
145+
it.each(decks)("serializes a valid package: %s", async (deckPath) => {
146+
const file = await readFile(deckPath);
147+
const source = file.buffer.slice(
148+
file.byteOffset,
149+
file.byteOffset + file.byteLength
150+
) as ArrayBuffer;
151+
152+
const deck = await parsePptx(source);
153+
const blob = await serializeDeck(deck, { source });
154+
const problems = await validate(await blob.arrayBuffer());
155+
156+
const summary = [
157+
problems.danglingRels.length
158+
? `dangling rels:\n ${problems.danglingRels.join("\n ")}`
159+
: "",
160+
problems.danglingOverrides.length
161+
? `Content_Types overrides with no part:\n ${problems.danglingOverrides.join("\n ")}`
162+
: "",
163+
problems.undeclaredParts.length
164+
? `parts with no content type:\n ${problems.undeclaredParts.join("\n ")}`
165+
: "",
166+
problems.invalidPngs.length
167+
? `.png parts holding non-PNG bytes:\n ${problems.invalidPngs.join("\n ")}`
168+
: "",
169+
]
170+
.filter(Boolean)
171+
.join("\n");
172+
173+
expect(summary, summary || undefined).toBe("");
174+
});
175+
});

0 commit comments

Comments
 (0)