Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/features/site-import.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,13 +374,22 @@ On success the same step switches to its **complete** state — a success mark,
| `invalid-rule` | A CSS rule caused `replaceSync` to throw (sheet-level parse error) |
| `blocked-property` | A CSS property name is on the security denylist (`behavior`, `-moz-binding`, …) — declaration dropped |
| `duplicate-class` | Two `.foo {}` rules in the same file; later declarations win |
| `missing-stylesheet` | A `<link rel="stylesheet">` href was not found in the FileMap |
| `missing-stylesheet` | A `<link rel="stylesheet">` href was not found in the FileMap (a cache-busting `?v=…` query is stripped before the lookup, so it is not a cause) |
| `asset-upload-failed` | An individual asset upload was rejected by the server; the original FileMap path remains in the import |
| `asset-folder-failed` | The asset uploaded and its URL was rewritten, but filing it under the folder mirroring its bundle path failed; it sits at the media root |
| `script-order-conflict` | Two pages link the same scripts in contradictory orders; the order from the page that links them first wins |
| `external-font` | An `@font-face` with no bundled file (all `src` entries are external URLs) — skipped |
| `unresolved-asset` | An HTML/CSS reference to a media file the archive does not contain under that path — one warning per distinct path |

### Cache-busted URLs

Reference resolution strips any query string or fragment before looking a path up in the FileMap:
`assets/site.css?v=8f3a1c` and `assets/site.css` address the same file. This applies to
`<link rel="stylesheet">`, `<script src>`, CSS `@import`, CSS `url()`, and node `src` / `href` / `srcset` —
static site generators commonly emit a content hash on all of them.

The original URL is preserved for the in-place CSS rewrite; only the lookup is affected.

The import log shows the first 12 warnings, ordered so the kinds that name a
missing file come first and the CSS interpretation notes last
(`rankWarning` in `importProgress.ts`).
Expand Down
110 changes: 110 additions & 0 deletions src/__tests__/siteImport/cacheBustedUrls.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* Cache-busted asset URLs (`site.css?v=8f3a1c`) resolve to the file they address.
*
* Static site generators routinely append a content hash or version to stylesheet, script and media URLs.
* The FileMap is keyed by path, so a query has to be stripped before the lookup — otherwise the reference
* resolves to a key that cannot exist and the import reports `missing-stylesheet` / `missing-script` while
* the file is sitting in the archive.
*
* These are end-to-end over `buildImportPlan` rather than unit tests of the resolver, because the unit was
* never the thing that broke: `resolveHref` returned a plausible-looking key and every consumer of it
* dutifully failed to find that key. The defect was only visible as warnings and missing style rules.
*/
import { describe, it, expect } from 'bun:test'
import '@modules/base'
import { buildImportPlan } from '@core/siteImport'
import type { FileMap } from '@core/siteImport'
import { makeEmptySiteDocument } from './mockSite'

const enc = new TextEncoder()
const txt = (s: string): Uint8Array => enc.encode(s)

const CSS = ':root{--brand:#0055ff}.hero{color:var(--brand);background:url(img/hero.png?v=9)}'

/** One page whose every local reference carries a cache-busting query. */
function cacheBustedSite(): FileMap {
return {
files: {
'index.html': {
bytes: txt(
'<!doctype html><html><head>' +
'<link rel="stylesheet" href="assets/site.css?v=8f3a1c">' +
'<script src="assets/app.js?v=8f3a1c"></script>' +
'</head><body>' +
'<img src="assets/img/hero.png?v=9" alt="Hero">' +
'<a href="about.html?utm_source=nav">About</a>' +
'</body></html>',
),
mimeType: 'text/html',
},
'about.html': { bytes: txt('<!doctype html><html><body><h1>About</h1></body></html>'), mimeType: 'text/html' },
'assets/site.css': { bytes: txt(CSS), mimeType: 'text/css' },
'assets/app.js': { bytes: txt('console.log("app")'), mimeType: 'application/javascript' },
'assets/img/hero.png': { bytes: new Uint8Array([0x89, 0x50, 0x4e, 0x47]), mimeType: 'image/png' },
},
}
}

/** The same site with every query removed — the control. */
function plainSite(): FileMap {
const src = cacheBustedSite()
const files: FileMap['files'] = {}
for (const [k, v] of Object.entries(src.files)) {
files[k] = k.endsWith('.html') || k.endsWith('.css')
? { ...v, bytes: txt(new TextDecoder().decode(v.bytes).replace(/\?(?:v=\w+|utm_source=nav)/g, '')) }
: v
}
return { files }
}

describe('cache-busted asset URLs', () => {
it('a stylesheet with ?v= is found, not reported missing', () => {
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
const missing = plan.warnings.filter((w) => w.kind === 'missing-stylesheet')
expect(missing).toHaveLength(0)
// Select by source — page order is not part of this contract.
const index = plan.pages.find((p) => p.source === 'index.html')!
expect(index.linkedCssPaths).toContain('assets/site.css')
})

it('a script with ?v= is found, not reported missing', () => {
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
expect(plan.warnings.filter((w) => w.kind === 'missing-script')).toHaveLength(0)
})

it('its style rules and root colour tokens are actually parsed', () => {
// The user-visible cost of the bug: the sheet was skipped, so every rule and token in it was lost.
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
expect(plan.styleRules.length).toBeGreaterThan(0)
expect(plan.colors.some((c) => c.value.toLowerCase() === '#0055ff')).toBe(true)
})

it('a cache-busted image is queued as an asset', () => {
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
expect(plan.assets.some((a) => a.sourcePath === "assets/img/hero.png")).toBe(true)
})

it('a cache-busted url() inside CSS resolves too', () => {
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
expect(plan.warnings.filter((w) => w.kind === 'unresolved-asset')).toHaveLength(0)
})

it('produces the same plan as the identical site without query strings', () => {
// The strongest statement available: cache-busting is addressing, not content, so it must make no
// difference to the outcome at all.
const busted = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
const plain = buildImportPlan({ fileMap: plainSite(), currentSite: makeEmptySiteDocument() })
expect(busted.pages).toHaveLength(plain.pages.length)
expect(busted.styleRules).toHaveLength(plain.styleRules.length)
expect(busted.colors).toHaveLength(plain.colors.length)
expect(busted.assets).toHaveLength(plain.assets.length)
expect(busted.warnings).toHaveLength(plain.warnings.length)
})

it('an internal page link keeps working when it carries a query', () => {
// linkRewrite used to strip the query itself before calling resolveHref. That strip now lives in
// resolveHref, so this pins that the behaviour survived the move.
const plan = buildImportPlan({ fileMap: cacheBustedSite(), currentSite: makeEmptySiteDocument() })
expect(plan.pages.map((p) => p.slug).sort()).toEqual(['about', 'index'])
})
})
36 changes: 36 additions & 0 deletions src/__tests__/siteImport/htmlPagePlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,42 @@ describe('resolveHref', () => {
expect(resolveHref('style.css', 'index.html')).toBe('style.css')
})

// Static site generators routinely cache-bust stylesheet and script URLs. The FileMap is keyed by path, so
// a query has to be stripped before the lookup or the reference resolves to a key that cannot exist and is
// reported missing while the file sits in the archive.
it('strips a cache-busting query string', () => {
expect(resolveHref('assets/site.css?v=8f3a1c', 'index.html')).toBe('assets/site.css')
})

it('strips a query string on a root-relative href', () => {
expect(resolveHref('/assets/site.css?v=8f3a1c', 'pages/about.html')).toBe('assets/site.css')
})

it('strips a query string before resolving a parent-relative href', () => {
expect(resolveHref('../css/main.css?ver=2', 'pages/about.html')).toBe('css/main.css')
})

it('strips a trailing fragment', () => {
expect(resolveHref('assets/icons.svg#arrow', 'index.html')).toBe('assets/icons.svg')
})

it('strips both a query and a fragment', () => {
expect(resolveHref('assets/icons.svg?v=2#arrow', 'index.html')).toBe('assets/icons.svg')
})

it('returns null when nothing remains after stripping', () => {
expect(resolveHref('?v=8f3a1c', 'index.html')).toBeNull()
})

// The point is not what traversal resolves TO — `joinPaths` clamps an escaping `..` to the root by design,
// which is pre-existing behaviour. The point is that stripping a query must not CHANGE that handling.
it('a query string does not alter traversal handling', () => {
for (const href of ['../../secrets.css', '../secrets.css', 'a/../../secrets.css', '/../secrets.css']) {
expect(resolveHref(`${href}?v=1`, 'index.html')).toBe(resolveHref(href, 'index.html'))
expect(resolveHref(`${href}#frag`, 'pages/about.html')).toBe(resolveHref(href, 'pages/about.html'))
}
})

it('resolves nested relative href', () => {
expect(resolveHref('./main.css', 'styles/base.css')).toBe('styles/main.css')
})
Expand Down
15 changes: 12 additions & 3 deletions src/core/siteImport/assetPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,13 +652,22 @@ function normalizeAssetPath(path: string): string {
/**
* Resolve a raw URL against a base file path to produce a FileMap key.
* Returns null for traversal-escaping paths or empty strings.
*
* As in `resolveHref`, a query string or fragment is stripped before the
* lookup — it addresses a file, it does not identify one. Callers keep the
* original `rawUrl` for the in-place CSS string replacement
* (`replaceRawUrlInValue`), so stripping here affects only which FileMap entry
* is found.
*/
function resolveRelativePath(rawUrl: string, basePath: string): string | null {
const baseDir = dirname(basePath)

const resolved = rawUrl.startsWith('/')
? rawUrl.slice(1) // root-relative: strip leading /
: joinPaths(baseDir, rawUrl)
const pathOnly = rawUrl.split(/[?#]/)[0]
if (!pathOnly) return null

const resolved = pathOnly.startsWith('/')
? pathOnly.slice(1) // root-relative: strip leading /
: joinPaths(baseDir, pathOnly)

// Reject escaped or empty results
if (!resolved || resolved.startsWith('../')) return null
Expand Down
16 changes: 13 additions & 3 deletions src/core/siteImport/htmlPagePlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,24 @@ function attrValue(attrs: string, name: string): string | null {
*
* Returns a normalized FileMap key, or null when the href is external,
* a fragment, a data URL, or cannot be resolved to a safe relative path.
*
* A query string or fragment is stripped first: `site.css?v=8f3a1c` addresses
* the same file as `site.css`, and the FileMap is keyed by path. Static site
* generators routinely emit cache-busting queries on stylesheet and script
* URLs, and without this every one of them resolves to a key that cannot
* exist — reported as `missing-stylesheet` / `missing-script` while the file
* sits in the archive.
*/
export function resolveHref(href: string, htmlFilePath: string): string | null {
// Skip external, protocol-relative, data, fragment, mailto, tel
if (/^https?:\/\/|^\/\/|^data:|^mailto:|^tel:|^#/.test(href)) return null

const normalized = href.startsWith('/')
? href.slice(1) // root-relative: strip leading /
: joinPaths(dirname(htmlFilePath), href)
const pathOnly = href.split(/[?#]/)[0]
if (!pathOnly) return null

const normalized = pathOnly.startsWith('/')
? pathOnly.slice(1) // root-relative: strip leading /
: joinPaths(dirname(htmlFilePath), pathOnly)

// Must not escape to a parent-of-root path
if (!normalized || normalized.startsWith('../') || normalized.includes('/../')) return null
Expand Down
7 changes: 3 additions & 4 deletions src/core/siteImport/linkRewrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@ function hrefToPageRef(
): string | null {
if (!href) return null

// Split off the fragment (kept) and any query (dropped — CMS routes are slugs).
// Split off the fragment, which is KEPT and re-attached to the page ref below.
// Any query string is dropped by `resolveHref` — CMS routes are slugs.
const hashIdx = href.indexOf('#')
const fragment = hashIdx >= 0 ? href.slice(hashIdx) : ''
let pathPart = hashIdx >= 0 ? href.slice(0, hashIdx) : href
const queryIdx = pathPart.indexOf('?')
if (queryIdx >= 0) pathPart = pathPart.slice(0, queryIdx)
const pathPart = hashIdx >= 0 ? href.slice(0, hashIdx) : href

// Pure same-page anchor (`#features`) or empty → not an internal page link.
if (!pathPart) return null
Expand Down