diff --git a/.changeset/properties-parts-by-relationship.md b/.changeset/properties-parts-by-relationship.md new file mode 100644 index 000000000..7eed8bb02 --- /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. 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 c6217d4c6..7defaf93b 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, + emptyRels, + nextRelId, + partName, + partNamesEqual, + resolveTarget, +} from '../../internal/opc/index.ts'; import { REL_TYPES } from '../../internal/presentationml/index.ts'; import { type XmlElement, @@ -11,9 +18,24 @@ 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/…`). +// 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) => { + 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; +}; + // --------------------------------------------------------------------------- // Core properties (`/docProps/core.xml`). @@ -100,8 +122,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 +198,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 +233,14 @@ 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); + // `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, @@ -220,6 +248,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 +265,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 +291,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 +334,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..8034ba351 --- /dev/null +++ b/test/issue-457-core-properties-rel.test.ts @@ -0,0 +1,123 @@ +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'); + 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'); + }); +});