From df8f959347fb131510905dc8522d6237fd9a2a0e Mon Sep 17 00:00:00 2001 From: baseballyama Date: Mon, 12 Oct 2026 23:47:39 +0900 Subject: [PATCH 1/2] fix: find core and extended properties through the root relationships (#457) OPC locates the core-properties part through the package-root relationship of that type; `/docProps/core.xml` is only the name the reference desktop app gives it. Looking the part up by that fixed name made getCoreProperties return null for packages that store it elsewhere, and setCoreProperties then bootstrapped a second part and a second core-properties relationship, which OPC forbids. The extended-properties helpers had the same fixed-name lookup for `/docProps/app.xml`. All four helpers now resolve the part from the root relationships. When setCoreProperties bootstraps, it replaces a part already sitting at `/docProps/core.xml` that no relationship points at, and drops a core-properties relationship whose target part is missing, so the package always ends up with exactly one. Closes #457 Co-Authored-By: Claude Opus 5.5 --- .../properties-parts-by-relationship.md | 5 + src/api/fn/properties.ts | 47 ++++++-- src/internal/opc/index.ts | 1 + src/internal/opc/part-name.ts | 12 ++- test/issue-457-core-properties-rel.test.ts | 102 ++++++++++++++++++ 5 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 .changeset/properties-parts-by-relationship.md create mode 100644 test/issue-457-core-properties-rel.test.ts diff --git a/.changeset/properties-parts-by-relationship.md b/.changeset/properties-parts-by-relationship.md new file mode 100644 index 000000000..90f8f9337 --- /dev/null +++ b/.changeset/properties-parts-by-relationship.md @@ -0,0 +1,5 @@ +--- +'@office-kit/pptx': patch +--- + +Fixed: `getCoreProperties` returned `null`, and `setCoreProperties` added a second core-properties part and relationship, for packages that store core properties somewhere other than `/docProps/core.xml` (e.g. `package/services/metadata/core-properties/….psmdcp`) (#457). Core and extended properties are now found through the package's root relationships, as OPC specifies, so `getExtendedProperties` / `setExtendedProperties` also work when `app.xml` lives elsewhere. diff --git a/src/api/fn/properties.ts b/src/api/fn/properties.ts index c6217d4c6..3312c4f18 100644 --- a/src/api/fn/properties.ts +++ b/src/api/fn/properties.ts @@ -1,6 +1,13 @@ // Core properties and extended properties. -import { emptyRels, nextRelId, partName } from '../../internal/opc/index.ts'; +import { + PACKAGE_ROOT, + type PartName, + emptyRels, + nextRelId, + partName, + resolveTarget, +} from '../../internal/opc/index.ts'; import { REL_TYPES } from '../../internal/presentationml/index.ts'; import { type XmlElement, @@ -11,9 +18,23 @@ import { serializeXml, text as textNode, } from '../../internal/xml/index.ts'; +import type { OpcPackage } from '../../internal/parts/index.ts'; import { INTERNAL_PACKAGE, type PresentationData } from '../_internal-symbols.ts'; import { decode, encode } from './_helpers.ts'; +// OPC locates the properties parts through the package-root relationships; +// `/docProps/*.xml` is only the name the reference desktop app gives them, and +// other producers store them elsewhere (e.g. `/package/services/metadata/…`). +const findRootRelPart = (pkg: OpcPackage, type: string): PartName | null => { + const rel = pkg.rootRels()?.items.find((r) => r.type === type && r.targetMode === 'Internal'); + return rel ? resolveTarget(PACKAGE_ROOT, rel.target) : null; +}; + +const getRootRelPart = (pkg: OpcPackage, type: string) => { + const name = findRootRelPart(pkg, type); + return name ? pkg.getPart(name) : null; +}; + // --------------------------------------------------------------------------- // Core properties (`/docProps/core.xml`). @@ -100,8 +121,7 @@ export const getPresentationModified = (pres: PresentationData): Date | null => }; export const getCoreProperties = (pres: PresentationData): CoreProperties | null => { - const pkg = pres[INTERNAL_PACKAGE]; - const part = pkg.getPart(CORE_PROPS_PART_NAME); + const part = getRootRelPart(pres[INTERNAL_PACKAGE], REL_TYPES.coreProperties); if (!part) return null; const root = parseXml(decode(part.data)).root; const read = (uri: string, local: string): string | null => { @@ -177,7 +197,7 @@ export const setCoreProperties = ( values: Partial, ): void => { const pkg = pres[INTERNAL_PACKAGE]; - let part = pkg.getPart(CORE_PROPS_PART_NAME); + const part = getRootRelPart(pkg, REL_TYPES.coreProperties); let root: XmlElement; let doc: ReturnType; if (part) { @@ -212,7 +232,10 @@ export const setCoreProperties = ( return; } - // Bootstrap: register override, add part, wire root rel. + // Bootstrap: register override, add part, wire root rel. A part already at + // the default name is not the core-properties part (no relationship points + // at it), so it is replaced rather than adopted. + pkg.removePart(CORE_PROPS_PART_NAME); pkg.contentTypes.overrides.push({ partName: CORE_PROPS_PART_NAME, contentType: CORE_PROPS_CONTENT_TYPE, @@ -220,6 +243,9 @@ export const setCoreProperties = ( pkg.addPart(CORE_PROPS_PART_NAME, CORE_PROPS_CONTENT_TYPE, bytes); const rootRels = pkg.rootRels() ?? emptyRels(); + // A relationship whose target part is missing would otherwise sit beside the + // new one, and OPC allows only one core-properties relationship. + rootRels.items = rootRels.items.filter((r) => r.type !== REL_TYPES.coreProperties); const rId = nextRelId(rootRels.items.map((r) => r.id)); rootRels.items.push({ id: rId, @@ -234,7 +260,6 @@ export const setCoreProperties = ( // Extended properties (`/docProps/app.xml`). const NS_EXT_PROPS = 'http://schemas.openxmlformats.org/officeDocument/2006/extended-properties'; -const EXT_PROPS_PART_NAME = partName('/docProps/app.xml'); /** * Selected string fields from `/docProps/app.xml` @@ -261,8 +286,7 @@ export interface ExtendedProperties { * corresponding element is absent or empty. */ export const getExtendedProperties = (pres: PresentationData): ExtendedProperties | null => { - const pkg = pres[INTERNAL_PACKAGE]; - const part = pkg.getPart(EXT_PROPS_PART_NAME); + const part = getRootRelPart(pres[INTERNAL_PACKAGE], REL_TYPES.extendedProperties); if (!part) return null; const root = parseXml(decode(part.data)).root; const read = (local: string): string | null => { @@ -305,10 +329,11 @@ export const setExtendedProperties = ( pres: PresentationData, values: Partial, ): void => { - const pkg = pres[INTERNAL_PACKAGE]; - const part = pkg.getPart(EXT_PROPS_PART_NAME); + const part = getRootRelPart(pres[INTERNAL_PACKAGE], REL_TYPES.extendedProperties); if (!part) { - throw new Error('setExtendedProperties: /docProps/app.xml not present; cannot bootstrap'); + throw new Error( + 'setExtendedProperties: the package has no extended-properties part; cannot bootstrap', + ); } const doc = parseXml(decode(part.data)); for (const field of EXT_PROP_FIELDS) { diff --git a/src/internal/opc/index.ts b/src/internal/opc/index.ts index e7b2e47a3..bcbdc250e 100644 --- a/src/internal/opc/index.ts +++ b/src/internal/opc/index.ts @@ -6,6 +6,7 @@ export { readZip, shouldStore, writeZip } from './zip.ts'; export type { PartName } from './part-name.ts'; export { + PACKAGE_ROOT, basename, dirname, fromZipPath, diff --git a/src/internal/opc/part-name.ts b/src/internal/opc/part-name.ts index 6a96a92b3..8ecd38a87 100644 --- a/src/internal/opc/part-name.ts +++ b/src/internal/opc/part-name.ts @@ -77,18 +77,24 @@ export const relsPartNameFor = (name: PartName): PartName => { return partName(`${prefix}${file}.rels`); }; +/** The source of the package-root relationships (`/_rels/.rels`). */ +export const PACKAGE_ROOT = '/'; + /** * Resolves a relationship Target (which may be relative) against a base part - * name. Returns the absolute part name (always starts with `/`). + * name, or against `PACKAGE_ROOT` for a package-root relationship. Returns the absolute part name (always starts with `/`). * * `target` is from a Relationship element's `Target` attribute when * `TargetMode="Internal"`. External targets must not be passed through here. */ -export const resolveTarget = (basePart: PartName, target: string): PartName => { +export const resolveTarget = ( + basePart: PartName | typeof PACKAGE_ROOT, + target: string, +): PartName => { if (target === '') throw new Error('relationship target is empty'); if (target.startsWith('/')) return partName(target); - const baseDir = dirname(basePart); + const baseDir = basePart === PACKAGE_ROOT ? PACKAGE_ROOT : dirname(basePart); const stack = baseDir === '/' ? [] : baseDir.slice(1).split('/'); for (const segment of target.split('/')) { if (segment === '' || segment === '.') continue; diff --git a/test/issue-457-core-properties-rel.test.ts b/test/issue-457-core-properties-rel.test.ts new file mode 100644 index 000000000..0e3ce9473 --- /dev/null +++ b/test/issue-457-core-properties-rel.test.ts @@ -0,0 +1,102 @@ +import { strFromU8, strToU8, unzipSync, zipSync } from 'fflate'; +import { describe, expect, it } from 'vitest'; +import { + createPresentation, + getCoreProperties, + getExtendedProperties, + loadPresentation, + savePresentation, + setCoreProperties, + setExtendedProperties, + validatePresentation, +} from '../src/api/index.ts'; + +const CORE_RELATIONSHIP = /relationships\/metadata\/core-properties"/g; + +// OPC locates these parts through the package's root relationships, not by +// name, so a producer may store them anywhere. `target` is written into +// `_rels/.rels` as is, so it can be relative to the package root or absolute. +const movePart = async (from: string, to: string, target = to) => { + const pres = createPresentation(); + setCoreProperties(pres, { title: 'Quarterly review' }); + setExtendedProperties(pres, { company: 'Example' }); + const files = unzipSync(await savePresentation(pres)); + files[to] = files[from]!; + delete files[from]; + files['_rels/.rels'] = strToU8( + strFromU8(files['_rels/.rels']!).replace(`"${from}"`, `"${target}"`), + ); + files['[Content_Types].xml'] = strToU8( + strFromU8(files['[Content_Types].xml']!).replace(`/${from}`, `/${to}`), + ); + return zipSync(files); +}; + +describe('issue-457: properties parts are found through the root relationships', () => { + const moved = 'package/services/metadata/core-properties/abc.psmdcp'; + + it('reads core properties stored under another name', async () => { + const pres = await loadPresentation(await movePart('docProps/core.xml', moved)); + expect(getCoreProperties(pres)?.title).toBe('Quarterly review'); + }); + + it('follows an absolute relationship target', async () => { + const pres = await loadPresentation(await movePart('docProps/core.xml', moved, `/${moved}`)); + expect(getCoreProperties(pres)?.title).toBe('Quarterly review'); + }); + + it('updates the existing core-properties part instead of adding a second one', async () => { + const pres = await loadPresentation(await movePart('docProps/core.xml', moved)); + setCoreProperties(pres, { title: 'Renamed' }); + const out = await savePresentation(pres); + const files = unzipSync(out); + expect(strFromU8(files['_rels/.rels']!).match(CORE_RELATIONSHIP)).toHaveLength(1); + expect(files['docProps/core.xml']).toBeUndefined(); + expect(strFromU8(files[moved]!)).toContain('Renamed'); + const reloaded = await loadPresentation(out); + expect(getCoreProperties(reloaded)?.title).toBe('Renamed'); + expect(validatePresentation(reloaded).filter((issue) => issue.severity === 'error')).toEqual( + [], + ); + }); + + it('reads and writes extended properties stored under another name', async () => { + const pres = await loadPresentation(await movePart('docProps/app.xml', 'meta/app-props.xml')); + expect(getExtendedProperties(pres)?.company).toBe('Example'); + setExtendedProperties(pres, { company: 'Renamed' }); + const files = unzipSync(await savePresentation(pres)); + expect(files['docProps/app.xml']).toBeUndefined(); + expect(strFromU8(files['meta/app-props.xml']!)).toContain('Renamed'); + }); + + it('ignores a /docProps/core.xml part that no relationship points at', async () => { + const pres = createPresentation(); + setCoreProperties(pres, { title: 'Quarterly review' }); + const files = unzipSync(await savePresentation(pres)); + files['_rels/.rels'] = strToU8( + strFromU8(files['_rels/.rels']!).replace(/]*core-properties"[^>]*\/>/, ''), + ); + const unlinked = await loadPresentation(zipSync(files)); + expect(getCoreProperties(unlinked)).toBeNull(); + + setCoreProperties(unlinked, { title: 'Linked' }); + const out = await savePresentation(unlinked); + expect(strFromU8(unzipSync(out)['_rels/.rels']!).match(CORE_RELATIONSHIP)).toHaveLength(1); + const reloaded = await loadPresentation(out); + expect(getCoreProperties(reloaded)).toMatchObject({ title: 'Linked', creator: null }); + expect(validatePresentation(reloaded).filter((issue) => issue.severity === 'error')).toEqual( + [], + ); + }); + + it('replaces a core-properties relationship whose part is missing', async () => { + const pres = createPresentation(); + const files = unzipSync(await savePresentation(pres)); + delete files['docProps/core.xml']; + const dangling = await loadPresentation(zipSync(files)); + setCoreProperties(dangling, { title: 'Restored' }); + const out = await savePresentation(dangling); + expect(strFromU8(unzipSync(out)['_rels/.rels']!).match(CORE_RELATIONSHIP)).toHaveLength(1); + expect(getCoreProperties(await loadPresentation(out))?.title).toBe('Restored'); + }); +}); From b3695409a87c213b92f5bc28291a5debefc7fa8d Mon Sep 17 00:00:00 2001 From: baseballyama Date: Mon, 12 Oct 2026 23:53:55 +0900 Subject: [PATCH 2/2] fix: keep one core-properties override when bootstrapping over a dangling relationship Also prefer the core-properties relationship whose part exists when an invalid package holds several. Co-Authored-By: Claude Opus 5.5 --- .../properties-parts-by-relationship.md | 2 +- src/api/fn/properties.ts | 21 ++++++++++++------- test/issue-457-core-properties-rel.test.ts | 21 +++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/.changeset/properties-parts-by-relationship.md b/.changeset/properties-parts-by-relationship.md index 90f8f9337..7eed8bb02 100644 --- a/.changeset/properties-parts-by-relationship.md +++ b/.changeset/properties-parts-by-relationship.md @@ -2,4 +2,4 @@ '@office-kit/pptx': patch --- -Fixed: `getCoreProperties` returned `null`, and `setCoreProperties` added a second core-properties part and relationship, for packages that store core properties somewhere other than `/docProps/core.xml` (e.g. `package/services/metadata/core-properties/….psmdcp`) (#457). Core and extended properties are now found through the package's root relationships, as OPC specifies, so `getExtendedProperties` / `setExtendedProperties` also work when `app.xml` lives elsewhere. +Fixed: `getCoreProperties` returned `null`, and `setCoreProperties` added a second core-properties part and relationship, for packages that store core properties somewhere other than `/docProps/core.xml` (e.g. `package/services/metadata/core-properties/….psmdcp`) (#457). Core and extended properties are now found through the package's root relationships, as OPC specifies, so `getExtendedProperties` / `setExtendedProperties` also work when `app.xml` lives elsewhere. A `/docProps/core.xml` part that no relationship points at is no longer read as the core properties, and a core-properties relationship whose target escapes the package root (`../…`) now throws instead of being ignored. diff --git a/src/api/fn/properties.ts b/src/api/fn/properties.ts index 3312c4f18..7defaf93b 100644 --- a/src/api/fn/properties.ts +++ b/src/api/fn/properties.ts @@ -2,10 +2,10 @@ import { PACKAGE_ROOT, - type PartName, emptyRels, nextRelId, partName, + partNamesEqual, resolveTarget, } from '../../internal/opc/index.ts'; import { REL_TYPES } from '../../internal/presentationml/index.ts'; @@ -25,14 +25,15 @@ import { decode, encode } from './_helpers.ts'; // OPC locates the properties parts through the package-root relationships; // `/docProps/*.xml` is only the name the reference desktop app gives them, and // other producers store them elsewhere (e.g. `/package/services/metadata/…`). -const findRootRelPart = (pkg: OpcPackage, type: string): PartName | null => { - const rel = pkg.rootRels()?.items.find((r) => r.type === type && r.targetMode === 'Internal'); - return rel ? resolveTarget(PACKAGE_ROOT, rel.target) : null; -}; - +// A package may hold only one relationship of each of these types; if an +// invalid one holds several, the first whose part exists is used. const getRootRelPart = (pkg: OpcPackage, type: string) => { - const name = findRootRelPart(pkg, type); - return name ? pkg.getPart(name) : null; + for (const rel of pkg.rootRels()?.items ?? []) { + if (rel.type !== type || rel.targetMode !== 'Internal') continue; + const part = pkg.getPart(resolveTarget(PACKAGE_ROOT, rel.target)); + if (part) return part; + } + return null; }; // --------------------------------------------------------------------------- @@ -236,6 +237,10 @@ export const setCoreProperties = ( // the default name is not the core-properties part (no relationship points // at it), so it is replaced rather than adopted. pkg.removePart(CORE_PROPS_PART_NAME); + // `removePart` leaves the override of a part that was already missing. + pkg.contentTypes.overrides = pkg.contentTypes.overrides.filter( + (o) => !partNamesEqual(o.partName, CORE_PROPS_PART_NAME), + ); pkg.contentTypes.overrides.push({ partName: CORE_PROPS_PART_NAME, contentType: CORE_PROPS_CONTENT_TYPE, diff --git a/test/issue-457-core-properties-rel.test.ts b/test/issue-457-core-properties-rel.test.ts index 0e3ce9473..8034ba351 100644 --- a/test/issue-457-core-properties-rel.test.ts +++ b/test/issue-457-core-properties-rel.test.ts @@ -98,5 +98,26 @@ describe('issue-457: properties parts are found through the root relationships', const out = await savePresentation(dangling); expect(strFromU8(unzipSync(out)['_rels/.rels']!).match(CORE_RELATIONSHIP)).toHaveLength(1); expect(getCoreProperties(await loadPresentation(out))?.title).toBe('Restored'); + const contentTypes = strFromU8(unzipSync(out)['[Content_Types].xml']!); + expect(contentTypes.match(/PartName="\/docProps\/core\.xml"/g)).toHaveLength(1); + }); + + it('uses the relationship whose part exists when several are present', async () => { + const files = unzipSync(await movePart('docProps/core.xml', moved)); + files['_rels/.rels'] = strToU8( + strFromU8(files['_rels/.rels']!).replace( + '', + '', + ), + ); + // Put the dangling relationship first. + files['_rels/.rels'] = strToU8( + strFromU8(files['_rels/.rels']!).replace( + /(]*>)([\s\S]*)(]*\/>)/, + '$1$3$2', + ), + ); + const pres = await loadPresentation(zipSync(files)); + expect(getCoreProperties(pres)?.title).toBe('Quarterly review'); }); });