Skip to content

Commit a954cce

Browse files
refactor: centralize ims package path resolution
Move IMS CP path normalization into a shared core module so package graph analysis and QTI 3 delivery context use the same checked and permissive resolution rules. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 6f5c13b commit a954cce

5 files changed

Lines changed: 120 additions & 65 deletions

File tree

packages/ims-cp-core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
export * from './manifest-parser.js';
7+
export * from './package-path.js';
78
export * from './package-graph.js';
89
export * from './localized-resources.js';
910
export * from './passage-reusability.js';

packages/ims-cp-core/src/package-graph.ts

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
import { type ManifestResource, type ParsedManifest, parseManifest } from './manifest-parser.js';
2+
import {
3+
dirnamePackagePath,
4+
isExternalPackageHref,
5+
joinPackagePath,
6+
resolvePackagePath,
7+
} from './package-path.js';
28

39
export type PackageDiagnosticSeverity = 'info' | 'warning' | 'error';
410

@@ -385,7 +391,7 @@ function discoverReferences(xml: string, owner: PackageResourceNode): {
385391
} {
386392
const references: PackageReference[] = [];
387393
const assets: PackageAssetRef[] = [];
388-
const basePath = owner.resolvedHref ? dirname(owner.resolvedHref) : '';
394+
const basePath = owner.resolvedHref ? dirnamePackagePath(owner.resolvedHref) : '';
389395

390396
for (const match of findAttributeReferences(xml)) {
391397
const resolvedPath = resolvePackagePath(basePath, match.rawHref);
@@ -433,7 +439,7 @@ function findAttributeReferences(xml: string): Array<{
433439
const attrs = match.groups?.attrs ?? '';
434440
const attribute = element === 'object' ? 'data' : element === 'img' ? 'src' : 'href';
435441
const rawHref = readAttribute(attrs, attribute) ?? readAttribute(attrs, 'src') ?? readAttribute(attrs, 'href');
436-
if (!rawHref || isExternalHref(rawHref)) continue;
442+
if (!rawHref || isExternalPackageHref(rawHref)) continue;
437443
refs.push({
438444
kind: kindFromElement(element),
439445
element,
@@ -583,35 +589,6 @@ function readAttribute(attrs: string, name: string): string | undefined {
583589
return match?.[1];
584590
}
585591

586-
function isExternalHref(href: string): boolean {
587-
return /^(?:[a-z][a-z0-9+.-]*:|\/\/|#)/i.test(href);
588-
}
589-
590-
function joinPackagePath(...parts: Array<string | undefined>): string {
591-
return parts.filter(Boolean).join('/');
592-
}
593-
594-
function resolvePackagePath(basePath: string | undefined, href: string): string {
595-
const raw = href.replaceAll('\\', '/');
596-
const combined = raw.startsWith('/') ? raw.slice(1) : joinPackagePath(basePath, raw);
597-
const normalized: string[] = [];
598-
for (const part of combined.split('/')) {
599-
if (!part || part === '.') continue;
600-
if (part === '..') {
601-
normalized.pop();
602-
continue;
603-
}
604-
normalized.push(part);
605-
}
606-
return normalized.join('/');
607-
}
608-
609-
function dirname(path: string): string {
610-
const normalized = path.replaceAll('\\', '/');
611-
const index = normalized.lastIndexOf('/');
612-
return index === -1 ? '' : normalized.slice(0, index);
613-
}
614-
615592
async function packagePathExists(
616593
path: string,
617594
fileAccess: PackageFileAccess,
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
export interface PackagePathResolutionOptions {
2+
rejectRootEscape?: boolean;
3+
}
4+
5+
export function joinPackagePath(...parts: Array<string | undefined>): string {
6+
return parts.filter(Boolean).join('/');
7+
}
8+
9+
export function dirnamePackagePath(path: string): string {
10+
const normalized = path.replaceAll('\\', '/');
11+
const index = normalized.lastIndexOf('/');
12+
return index === -1 ? '' : normalized.slice(0, index);
13+
}
14+
15+
export function resolvePackagePath(basePath: string | undefined, href: string): string {
16+
return resolvePackagePathInternal(basePath, href).path;
17+
}
18+
19+
export function resolvePackagePathFromFile(baseHref: string, href: string): string {
20+
return resolvePackagePath(dirnamePackagePath(baseHref), href);
21+
}
22+
23+
export function resolveCheckedPackagePath(basePath: string | undefined, href: string): string | null {
24+
if (!isPackageRelativeHref(href)) return null;
25+
const result = resolvePackagePathInternal(basePath, href, { rejectRootEscape: true });
26+
return result.escapedRoot ? null : result.path;
27+
}
28+
29+
export function resolveCheckedPackagePathFromFile(baseHref: string, href: string): string | null {
30+
return resolveCheckedPackagePath(dirnamePackagePath(baseHref), href);
31+
}
32+
33+
export function isPackageRelativeHref(href: string): boolean {
34+
const value = href.trim();
35+
return Boolean(value) && !value.startsWith('/') && !value.startsWith('//') && !/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(value);
36+
}
37+
38+
export function isExternalPackageHref(href: string): boolean {
39+
return /^(?:[a-z][a-z0-9+.-]*:|\/\/|#)/i.test(href);
40+
}
41+
42+
function resolvePackagePathInternal(
43+
basePath: string | undefined,
44+
href: string,
45+
options: PackagePathResolutionOptions = {}
46+
): { path: string; escapedRoot: boolean } {
47+
const raw = href.replaceAll('\\', '/');
48+
const combined = raw.startsWith('/') ? raw.slice(1) : joinPackagePath(basePath, raw);
49+
const normalized: string[] = [];
50+
let escapedRoot = false;
51+
52+
for (const part of combined.split('/')) {
53+
if (!part || part === '.') continue;
54+
if (part === '..') {
55+
if (normalized.length === 0) {
56+
escapedRoot = true;
57+
if (options.rejectRootEscape) continue;
58+
} else {
59+
normalized.pop();
60+
}
61+
continue;
62+
}
63+
normalized.push(part);
64+
}
65+
66+
return { path: normalized.join('/'), escapedRoot };
67+
}

packages/ims-cp-core/src/qti3-shared-content.ts

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { parse, type HTMLElement, type Node } from 'node-html-parser';
2+
import {
3+
isPackageRelativeHref,
4+
resolveCheckedPackagePathFromFile,
5+
resolvePackagePathFromFile,
6+
} from './package-path.js';
27

38
export interface AssessmentStimulusRef {
49
identifier: string;
@@ -225,39 +230,12 @@ export function extractCatalogInfoXml(xml: string): string[] {
225230
}
226231

227232
export function resolveRelativePath(baseHref: string, relativePath: string): string {
228-
const baseDir = baseHref.includes('/') ? baseHref.substring(0, baseHref.lastIndexOf('/') + 1) : '';
229-
const combined = baseDir + relativePath;
230-
const resolved: string[] = [];
231-
for (const part of combined.split('/')) {
232-
if (!part || part === '.') continue;
233-
if (part === '..') {
234-
if (resolved.length > 0) resolved.pop();
235-
} else {
236-
resolved.push(part);
237-
}
238-
}
239-
return resolved.join('/');
240-
}
241-
242-
function resolveCheckedRelativePath(baseHref: string, relativePath: string): string | null {
243-
const baseDir = baseHref.includes('/') ? baseHref.substring(0, baseHref.lastIndexOf('/') + 1) : '';
244-
const combined = baseDir + relativePath;
245-
const resolved: string[] = [];
246-
for (const part of combined.split('/')) {
247-
if (!part || part === '.') continue;
248-
if (part === '..') {
249-
if (resolved.length === 0) return null;
250-
resolved.pop();
251-
} else {
252-
resolved.push(part);
253-
}
254-
}
255-
return resolved.join('/');
233+
return resolvePackagePathFromFile(baseHref, relativePath);
256234
}
257235

258236
function resolveCheckedPackagePath(baseHref: string, href: string): string | null {
259237
if (!isPackageRelativeHref(href)) return null;
260-
return resolveCheckedRelativePath(baseHref, href);
238+
return resolveCheckedPackagePathFromFile(baseHref, href);
261239
}
262240

263241
function resolvePackageHref(
@@ -320,11 +298,6 @@ function sanitizeStylesheetCss(css: string, validationMessages: string[], resolv
320298
return css;
321299
}
322300

323-
function isPackageRelativeHref(href: string): boolean {
324-
const value = href.trim();
325-
return Boolean(value) && !value.startsWith('/') && !value.startsWith('//') && !/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(value);
326-
}
327-
328301
function rewriteHtmlAssetRefs(
329302
html: string,
330303
baseHref: string,
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import {
3+
dirnamePackagePath,
4+
isExternalPackageHref,
5+
isPackageRelativeHref,
6+
resolveCheckedPackagePathFromFile,
7+
resolvePackagePath,
8+
resolvePackagePathFromFile,
9+
} from '../src/package-path';
10+
11+
describe('IMS CP package path resolution', () => {
12+
test('normalizes package paths with manifest-style base directories', () => {
13+
expect(resolvePackagePath('content/items', './images/../item.xml')).toBe('content/items/item.xml');
14+
expect(resolvePackagePath('content/items', '/shared/passage.xml')).toBe('shared/passage.xml');
15+
expect(resolvePackagePath('content/items', '..\\assets\\chart.png')).toBe('content/assets/chart.png');
16+
});
17+
18+
test('resolves hrefs relative to source files', () => {
19+
expect(resolvePackagePathFromFile('items/item.xml', 'images/chart.png')).toBe('items/images/chart.png');
20+
expect(resolvePackagePathFromFile('tests/test.xml', '../items/item.xml')).toBe('items/item.xml');
21+
expect(dirnamePackagePath('tests/unit/test.xml')).toBe('tests/unit');
22+
});
23+
24+
test('checked resolution rejects hrefs that escape the package root', () => {
25+
expect(resolveCheckedPackagePathFromFile('items/item.xml', '../shared/passage.xml')).toBe('shared/passage.xml');
26+
expect(resolveCheckedPackagePathFromFile('item.xml', '../outside.xml')).toBeNull();
27+
expect(resolveCheckedPackagePathFromFile('item.xml', 'http://example.com/item.xml')).toBeNull();
28+
});
29+
30+
test('classifies package-relative and external hrefs', () => {
31+
expect(isPackageRelativeHref('items/item.xml')).toBe(true);
32+
expect(isPackageRelativeHref('/items/item.xml')).toBe(false);
33+
expect(isPackageRelativeHref('https://example.com/item.xml')).toBe(false);
34+
expect(isExternalPackageHref('#fragment')).toBe(true);
35+
expect(isExternalPackageHref('//cdn.example.com/item.xml')).toBe(true);
36+
});
37+
});

0 commit comments

Comments
 (0)