fix: find core and extended properties through the root relationships (#457)

.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.

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<CoreProperties>,
 ): 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<typeof parseXml>;
   if (part) {
@@ -212,14 +233,24 @@ 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,
   });
   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<ExtendedProperties>,
 ): 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) {

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,

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;

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('<Company>Renamed</Company>');
+  });
+
+  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(/<Relationship [^>]*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(
+        '</Relationships>',
+        '<Relationship Id="rId99" Type="http://schemas.openxmlformats.org/package/2006/relationships/metadata/core-properties" Target="missing/core.xml"/></Relationships>',
+      ),
+    );
+    // Put the dangling relationship first.
+    files['_rels/.rels'] = strToU8(
+      strFromU8(files['_rels/.rels']!).replace(
+        /(<Relationships[^>]*>)([\s\S]*)(<Relationship Id="rId99"[^>]*\/>)/,
+        '$1$3$2',
+      ),
+    );
+    const pres = await loadPresentation(zipSync(files));
+    expect(getCoreProperties(pres)?.title).toBe('Quarterly review');
+  });
+});