diff --git a/packages/melonjs/src/loader/loader.js b/packages/melonjs/src/loader/loader.js index 93c5e90bf..262e9322c 100644 --- a/packages/melonjs/src/loader/loader.js +++ b/packages/melonjs/src/loader/loader.js @@ -305,6 +305,51 @@ function onLoadingError(res) { throw new Error("Failed loading resource " + res.src); } +/** a src that is already a bare path needs no unwrapping @ignore @internal */ +const keepSrc = (src) => { + return src; +}; + +/** + * The one rule that holds for every asset type: a data URI carries its own + * payload, so nothing can be relative to it. Checked by `load()` for all types + * rather than left to `needsBaseURL`, so that a type overriding that hook + * declares only its OWN exceptions and cannot forget this one. + * @param {string} src - the normalized src + * @returns {boolean} true for a data URI + * @ignore + * @internal + */ +const isDataURI = (src) => { + return src.startsWith("data:"); +}; + +/** default: a src is a path relative to its type's base URL @ignore @internal */ +const alwaysRelative = () => { + return true; +}; + +/** + * A registered parser, normalized by {@link loader.setParser}. + * + * Every type gets the same shape whether or not it needs the src hooks, so + * `load()` can treat all of them alike. The alternative — a bare function that + * some types decorate with extra properties — puts the knowledge of which + * types are special back into the caller, which is what moving it out was for. + * @typedef {object} AssetParser + * @property {Function} parse - preloads the asset and returns how many + * resources it will load (0 when already cached) + * @property {function(string): string} normalizeSrc - turns a type-specific + * descriptor into a bare path, before any base URL is applied. Identity unless + * the type overrides it + * @property {function(string): boolean} needsBaseURL - whether the normalized + * src is relative to this type's base URL. Declares only this type's own + * exceptions, such as a `local()` font naming an installed family — data URIs + * are excluded for every type before this is consulted + * @ignore + * @internal + */ + /** * an asset definition to be used with the loader * @typedef {object} Asset @@ -390,7 +435,11 @@ export function setParser(type, parserFn) { warning("overriding parser for " + type + " format"); } - parsers.set(type, parserFn); + parsers.set(type, { + parse: parserFn, + normalizeSrc: parserFn.normalizeSrc ?? keepSrc, + needsBaseURL: parserFn.needsBaseURL ?? alwaysRelative, + }); } /** @@ -567,6 +616,12 @@ export function load(asset, onload, onerror) { initParsers(); } + const parser = parsers.get(asset.type); + + if (typeof parser === "undefined") { + throw new Error("load : unknown or invalid resource type : " + asset.type); + } + // Resolve the effective src WITHOUT mutating the caller's asset // descriptor: load() used to write the transformed url back into // asset.src, so retrying the same object — loader.reload() after a @@ -576,33 +631,23 @@ export function load(asset, onload, onerror) { // caller's original src; only the parser sees the resolved one. let src = asset.src; - // strip url() wrapper for fontface assets so baseURL can be prepended to the raw path - if (asset.type === "fontface" && typeof src === "string") { - const urlMatch = src.match(/^url\(\s*['"]?(.*?)['"]?\s*\)$/); - if (urlMatch) { - src = urlMatch[1]; + // Normalize, then prefix — both through the parser, so this function never + // names an asset type. `src` may legitimately be an array (an image + // fallback chain) or absent, and neither hook applies to those. + if (typeof src === "string") { + src = parser.normalizeSrc(src); + if ( + typeof baseURL[asset.type] !== "undefined" && + !isDataURI(src) && + parser.needsBaseURL(src) + ) { + src = baseURL[asset.type] + src; } } - // transform the url if necessary (skip for local() font sources and data URIs) - if ( - typeof baseURL[asset.type] !== "undefined" && - typeof src === "string" && - !src.startsWith("local(") && - !src.startsWith("data:") - ) { - src = baseURL[asset.type] + src; - } - const resource = src === asset.src ? asset : Object.assign({}, asset, { src }); - const parser = parsers.get(asset.type); - - if (typeof parser === "undefined") { - throw new Error("load : unknown or invalid resource type : " + asset.type); - } - const settings = { nocache: nocache, crossOrigin: crossOrigin, @@ -616,7 +661,7 @@ export function load(asset, onload, onerror) { return new Promise((resolve, reject) => { // parser returns the amount of asset to be loaded (usually 1, more // if it splits into several); 0 means already cached → resolve now. - const count = parser.call( + const count = parser.parse.call( this, resource, () => { @@ -632,7 +677,7 @@ export function load(asset, onload, onerror) { } // parser returns the amount of asset to be loaded (usually 1 unless an asset is splitted into several ones) - return parser.call(this, resource, onload, onerror, settings); + return parser.parse.call(this, resource, onload, onerror, settings); } /** diff --git a/packages/melonjs/src/loader/parsers/fontface.js b/packages/melonjs/src/loader/parsers/fontface.js index 80f490fb7..adc2fc74e 100644 --- a/packages/melonjs/src/loader/parsers/fontface.js +++ b/packages/melonjs/src/loader/parsers/fontface.js @@ -70,3 +70,30 @@ export function preloadFontFace(data, onload, onerror) { return 1; } + +/** + * A `fontface` src may be a CSS font descriptor rather than a bare path, so + * unwrap `url(...)` before the loader prefixes the font base URL — it cannot be + * repaired afterwards, since `"data/font/" + "url('x.woff2')"` is not a path + * either half of the pipeline can recover from. + * @param {string} src - the descriptor as written in the manifest + * @returns {string} the path inside it, or the descriptor unchanged + * @ignore + * @internal + */ +preloadFontFace.normalizeSrc = (src) => { + const urlMatch = src.match(/^url\(\s*['"]?(.*?)['"]?\s*\)$/); + return urlMatch ? urlMatch[1] : src; +}; + +/** + * `local('Family Name')` names a font already installed on the machine, which + * is not a location and has no base to be relative to. + * @param {string} src - the normalized src + * @returns {boolean} false for an installed family, true for a path + * @ignore + * @internal + */ +preloadFontFace.needsBaseURL = (src) => { + return !src.startsWith("local("); +}; diff --git a/packages/melonjs/tests/loader-parser-contract.spec.js b/packages/melonjs/tests/loader-parser-contract.spec.js new file mode 100644 index 000000000..1e62b3ddf --- /dev/null +++ b/packages/melonjs/tests/loader-parser-contract.spec.js @@ -0,0 +1,245 @@ +import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { boot, loader } from "../src/index.js"; +import { preloadFontFace } from "../src/loader/parsers/fontface.js"; + +/** + * The parser contract: every registered parser is normalized to the same + * shape — `{ parse, normalizeSrc, needsBaseURL }` — so `load()` can resolve a + * src without ever naming an asset type. + * + * These are adversarial on purpose. The thing that makes the contract worth + * having is that a type's rules live with that type, and the failure mode when + * it breaks is silent: a src resolves to a slightly wrong URL and the asset + * 404s at runtime, far from the cause. + */ +describe("the parser src contract", () => { + /** every type registered here, cleaned up after each test */ + const registered = new Set(); + + beforeAll(() => { + boot(); + }); + + afterEach(() => { + for (const type of registered) { + loader.setBaseURL(type, ""); + } + registered.clear(); + }); + + /** + * Register a probe parser and return what src the parser actually received. + * @param {string} type - a unique asset type for this test + * @param {object} [hooks] - optional `normalizeSrc` / `needsBaseURL` + * @param {string} [base] - base URL for the type + * @returns {function(string): Promise} loads a src, resolves to what the parser saw + */ + const probe = (type, hooks = {}, base = "assets/") => { + let seen; + const parser = (asset, onload) => { + seen = asset.src; + onload(); + return 1; + }; + Object.assign(parser, hooks); + loader.setParser(type, parser); + loader.setBaseURL(type, base); + registered.add(type); + return async (src) => { + await loader.load(Object.freeze({ name: `${type}-probe`, type, src })); + return seen; + }; + }; + + describe("a parser that declares nothing", () => { + it("gets the base URL, and keeps its src otherwise untouched", async () => { + const load = probe("plain-a"); + expect(await load("sprite.png")).toBe("assets/sprite.png"); + }); + + it("still has data URIs excluded — the one universal rule", async () => { + // a type that declares no hooks must not have to know about data: + const load = probe("plain-b"); + expect(await load("data:image/png;base64,AA==")).toBe( + "data:image/png;base64,AA==", + ); + }); + + it("is handed a src that looks like another type's descriptor, verbatim", async () => { + // `url(...)` is CSS, and means nothing to a type that has not said so. + // If `load()` ever unwraps it for everyone, this breaks — which is + // exactly the leak the contract exists to prevent. + const load = probe("plain-c"); + expect(await load("url(weird.png)")).toBe("assets/url(weird.png)"); + const localish = probe("plain-d"); + expect(await localish("local('Not A Font')")).toBe( + "assets/local('Not A Font')", + ); + }); + }); + + describe("normalizeSrc", () => { + it("runs before the base URL, not after", async () => { + // the ordering IS the reason the hook exists: a parser cannot repair + // "assets/" + "url(x.png)" once the prefix is on + const load = probe("order-a", { + normalizeSrc: (src) => { + return src.replace(/^wrapped\((.*)\)$/, "$1"); + }, + }); + expect(await load("wrapped(x.png)")).toBe("assets/x.png"); + }); + + it("can normalize a src INTO a data URI and still be excluded", async () => { + // the nasty ordering case: the exclusion has to be tested against the + // NORMALIZED value, not the original. Tested against the original, + // this resolves to "assets/data:font/..." and 404s + const load = probe("order-b", { + normalizeSrc: (src) => { + return src.replace(/^url\((.*)\)$/, "$1"); + }, + }); + expect(await load("url(data:font/woff2;base64,AA==)")).toBe( + "data:font/woff2;base64,AA==", + ); + }); + + it("can normalize a src into one its own needsBaseURL then rejects", async () => { + // same ordering trap, through the other hook + const load = probe("order-c", { + normalizeSrc: (src) => { + return src.replace(/^wrapped\((.*)\)$/, "$1"); + }, + needsBaseURL: (src) => { + return !src.startsWith("local("); + }, + }); + expect(await load("wrapped(local('Family'))")).toBe("local('Family')"); + }); + + it("is not called for an array src (an image fallback chain)", async () => { + let calls = 0; + const load = probe("array-a", { + normalizeSrc: (src) => { + calls++; + return src; + }, + }); + const chain = ["a.ktx", "b.png"]; + expect(await load(chain)).toEqual(chain); + expect(calls).toBe(0); + }); + }); + + describe("needsBaseURL", () => { + it("suppresses the prefix when it answers false", async () => { + const load = probe("skip-a", { + needsBaseURL: () => { + return false; + }, + }); + expect(await load("anything.bin")).toBe("anything.bin"); + }); + + it("cannot re-enable the base URL for a data URI", async () => { + // data: is decided before the hook is consulted, so a parser that + // answers true for everything still cannot break a data URI + const load = probe("skip-b", { + needsBaseURL: () => { + return true; + }, + }); + expect(await load("data:text/plain,hi")).toBe("data:text/plain,hi"); + }); + }); + + describe("the manifest entry is never mutated", () => { + it("survives being loaded twice, frozen", async () => { + // the bug this guards: load() used to write the resolved src back + // onto the asset, so a retry prefixed an already-prefixed path + const load = probe("frozen-a"); + expect(await load("x.png")).toBe("assets/x.png"); + expect(await load("x.png")).toBe("assets/x.png"); + }); + + it("leaves the caller's object untouched", async () => { + const type = "frozen-b"; + let seen; + const parser = (asset, onload) => { + seen = asset.src; + onload(); + return 1; + }; + loader.setParser(type, parser); + loader.setBaseURL(type, "assets/"); + registered.add(type); + const asset = Object.freeze({ name: "f", type, src: "y.png" }); + await loader.load(asset); + expect(seen).toBe("assets/y.png"); + expect(asset.src).toBe("y.png"); + }); + }); + + describe("the fontface rules, through the generic path", () => { + /** + * Registered under a type that is NOT "fontface", so nothing can pass by + * the loader recognising the name. + * @returns {function(string): Promise} the probe + */ + const fontProbe = () => { + return probe( + "font-rules", + { + normalizeSrc: preloadFontFace.normalizeSrc, + needsBaseURL: preloadFontFace.needsBaseURL, + }, + "fonts/", + ); + }; + + it.each([ + ["bare path", "t.woff2", "fonts/t.woff2"], + ["unquoted url()", "url(t.woff2)", "fonts/t.woff2"], + ["single-quoted", "url('t.woff2')", "fonts/t.woff2"], + ["double-quoted", 'url("t.woff2")', "fonts/t.woff2"], + ["padded", "url( 't.woff2' )", "fonts/t.woff2"], + ["a space in the name", "url('My Font.woff2')", "fonts/My Font.woff2"], + ["local family", "local('My Font')", "local('My Font')"], + [ + "bare data URI", + "data:font/woff2;base64,AA==", + "data:font/woff2;base64,AA==", + ], + [ + "data URI inside url()", + "url('data:font/woff2;base64,AA==')", + "data:font/woff2;base64,AA==", + ], + [ + "a path that merely contains url(", + "my-url(x).woff2", + "fonts/my-url(x).woff2", + ], + ["an unterminated url(", "url(t.woff2", "fonts/url(t.woff2"], + ])("%s", async (_label, src, expected) => { + const load = fontProbe(); + expect(await load(src)).toBe(expected); + }); + }); + + describe("a type with no base URL configured", () => { + it("leaves the src alone but still normalizes it", async () => { + const load = probe( + "nobase-a", + { + normalizeSrc: (src) => { + return src.replace(/^w\((.*)\)$/, "$1"); + }, + }, + "", + ); + // setBaseURL("") is still "defined", so the prefix is an empty string + expect(await load("w(x.png)")).toBe("x.png"); + }); + }); +}); diff --git a/packages/melonjs/tests/loader-src-resolution.spec.js b/packages/melonjs/tests/loader-src-resolution.spec.js new file mode 100644 index 000000000..eb2c6706e --- /dev/null +++ b/packages/melonjs/tests/loader-src-resolution.spec.js @@ -0,0 +1,47 @@ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { boot, loader } from "../src/index.js"; +import { preloadFontFace } from "../src/loader/parsers/fontface.js"; + +describe("parser-owned source resolution", () => { + const type = "font-source-probe"; + let received; + + beforeAll(() => { + boot(); + const parser = (asset, onload) => { + received = asset.src; + onload(); + return 1; + }; + // Use the font parser's rules under another type, so resolution cannot + // depend on the loader recognizing the name "fontface". + Object.assign(parser, { + normalizeSrc: preloadFontFace.normalizeSrc, + needsBaseURL: preloadFontFace.needsBaseURL, + }); + loader.setParser(type, parser); + loader.setBaseURL(type, "fonts/"); + }); + + afterAll(() => { + loader.setBaseURL(type, ""); + }); + + it.each([ + ["test.woff2", "fonts/test.woff2"], + ["url(test.woff2)", "fonts/test.woff2"], + ["url('test.woff2')", "fonts/test.woff2"], + ['url( "Test Font.woff2" )', "fonts/Test Font.woff2"], + ["local('Test Font')", "local('Test Font')"], + ["data:font/woff2;base64,AA==", "data:font/woff2;base64,AA=="], + ["url('data:font/woff2;base64,AA==')", "data:font/woff2;base64,AA=="], + ])("resolves %s without changing the manifest", async (src, expected) => { + const asset = Object.freeze({ name: "source-probe", type, src }); + await loader.load(asset); + expect(received).toBe(expected); + expect(asset.src).toBe(src); + // Retrying the same entry must not prepend the base URL twice. + await loader.load(asset); + expect(received).toBe(expected); + }); +}); diff --git a/packages/melonjs/tests/loader.spec.js b/packages/melonjs/tests/loader.spec.js index 0db9b9535..b60412faf 100644 --- a/packages/melonjs/tests/loader.spec.js +++ b/packages/melonjs/tests/loader.spec.js @@ -1,6 +1,7 @@ import { beforeAll, describe, expect, it } from "vitest"; import { audio, boot, event, loader } from "../src/index.js"; import { fontList, videoList } from "../src/loader/cache.js"; +import { preloadFontFace } from "../src/loader/parsers/fontface.js"; describe("loader", () => { let audioURI; @@ -275,14 +276,15 @@ describe("loader", () => { loader.setBaseURL("fontface", "assets/"); const receivedSrc = []; - // stub fontface parser to capture the resolved src - loader.setParser("fontface", (data, onload) => { + // Capture the resolved src while retaining the font parser's rules. + const parser = (data, onload) => { receivedSrc.push(data.src); if (typeof onload === "function") { onload(); } return 1; - }); + }; + loader.setParser("fontface", Object.assign(parser, preloadFontFace)); // plain path loader.load( @@ -307,6 +309,7 @@ describe("loader", () => { // reset loader.setBaseURL("fontface", "./"); + loader.setParser("fontface", preloadFontFace); }); it("should not strip local() wrapper from fontface src", () => { @@ -314,13 +317,14 @@ describe("loader", () => { loader.setBaseURL("fontface", "./"); const receivedSrc = []; - loader.setParser("fontface", (data, onload) => { + const parser = (data, onload) => { receivedSrc.push(data.src); if (typeof onload === "function") { onload(); } return 1; - }); + }; + loader.setParser("fontface", Object.assign(parser, preloadFontFace)); loader.load( { name: "font4", type: "fontface", src: "local('My Font')" }, @@ -328,6 +332,7 @@ describe("loader", () => { ); expect(receivedSrc[0]).toBe("local('My Font')"); + loader.setParser("fontface", preloadFontFace); }); it("should configure loader options", () => {