Chunk id collisions between extensions: ExportRegistry.chunks is not keyed by namespace
- Dominant language
- PHP
- Stars
- 6.7k
- Forks
- 883
- Avg merge
- 15h 16m
- Merged PRs (30d)
- 73
Description
## Current Behavior
`ExportRegistry.chunks` is keyed by chunk id alone, with no namespace:
```ts
// framework/core/js/src/common/ExportRegistry.ts:143-158
addChunkModule(chunkId, moduleId, namespace, urlPath): void {
if (!this.chunks.has(chunkId.toString())) {
this.chunks.set(chunkId.toString(), { namespace, urlPath, modules: [urlPath] });
} else {
this.chunks.get(chunkId.toString())?.modules?.push(urlPath);
}
this.chunkModules.set(`${namespace}:${urlPath}`, { chunkId, moduleId });
}
```
Every extension gets its chunk ids from `DeterministicChunkIdsPlugin` with `maxLength: 3`, so all extensions draw from the same 1000 slots with no coordination. When two extensions get the same id, the second one is appended to the first one's `modules` array, and `chunkUrl()` (`:211-228`) returns the first one's asset URL for both.
The second extension's `import()` then either fetches the wrong file, or throws `TypeError: Cannot read properties of undefined (reading 'call')` with no network request at all. The second case happens because all extensions share one jsonp array (`webpackChunkmodule_exports`, from `output.library: 'module.exports'`), so a foreign chunk marks the id installed in every runtime.
Ids are hashed from the chunk name, not its contents, so a collision does not go away when either extension changes its code. Nothing warns about it either: `addChunkModule` just takes the `else` branch.
`flarum/audit` has `forum/components/ActorAuditModal` at id 767 today. Any extension with a chunk name hashing to 767 collides with it.
Collision chance for a new extension against the 25 forum ids the bundled set uses today:
| chunks emitted | P(collision) |
|---|---|
| 3 | 7% |
| 26 | 49% |
## Steps to Reproduce
1. Confirm two names collide, using webpack's own hash function:
```js
const numberHash = require('./node_modules/webpack/lib/util/numberHash');
for (const name of ['forum/components/ActorAuditModal', 'common/builder/canvas/minimap'])
console.log(name, numberHash(name + '0', 1000)); // both print 767
```
2. Build two extensions with different composer names, one lazily importing `src/forum/components/ActorAuditModal`, the other `src/common/builder/canvas/minimap`. Their `js/dist/forum.js` files contain:
```
flarum.reg.addChunkModule("767","936","flarum-audit","forum/components/ActorAuditModal")
flarum.reg.addChunkModule("767","264","acme-widgets","common/builder/canvas/minimap")
```
3. Install both, load a forum page, and check the registry:
```js
flarum.reg.chunkUrl(767)
// "/assets/js/flarum-audit/forum/components/ActorAuditModal.js"
// expected "/assets/js/acme-widgets/common/builder/canvas/minimap.js"
```
4. Open the feature in the second extension that lazily imports its chunk. If audit's chunk 767 was loaded first, the import resolves with no network request and throws `TypeError`. If it was not, the wrong file is fetched and the import fails with `ChunkLoadError`.
## Expected Behavior
Each extension's `import()` resolves to its own chunk, whatever else is installed and whatever order extensions register in. A chunk id only means something inside the extension that emitted it, so `chunks` should be keyed by namespace and id, the way `chunkModules` already is.
## Environment
- Flarum version: 2.0.0-rc.8, `2.x` branch at ecd7962
- Website URL: n/a, local checkout
- Webserver: n/a
- Hosting environment: local development
- PHP version: 8.5.9
- Database: n/a
- Browser: not reproduced in a browser yet, see below
- flarum-webpack-config: 3.0.4, webpack 5.110.3, Node 26.5.0
I reproduced this by transpiling `ExportRegistry.ts` from the repo and replaying the real `addChunkModule` calls from two compiled bundles against it, then evaluating both entry bundles in one Node vm with a fake network. The jsonp code and module tables are webpack's real output, but the script tag and the network are simulated. I have not opened it in a browser yet. I can do that first if you want it before looking at a fix.
## Possible Solution
Key `chunks` by `${namespace}:${chunkId}`, and add an optional trailing `namespace` argument to the three lookup methods.
`framework/core/js/src/common/ExportRegistry.ts`:
```diff
addChunkModule(chunkId, moduleId, namespace, urlPath): void {
- if (!this.chunks.has(chunkId.toString())) {
- this.chunks.set(chunkId.toString(), { namespace, urlPath, modules: [urlPath] });
+ const key = `${namespace}:${chunkId}`;
+ if (!this.chunks.has(key)) {
+ this.chunks.set(key, { namespace, urlPath, modules: [urlPath] });
} else {
- this.chunks.get(chunkId.toString())?.modules?.push(urlPath);
+ this.chunks.get(key)?.modules?.push(urlPath);
}
- getChunk(chunkId: number | string): Chunk | null {
- const chunk = this.chunks.get(chunkId.toString()) ?? null;
+ getChunk(chunkId: number | string, namespace?: string): Chunk | null {
+ // No namespace means a bundle built before this change.
+ const legacyKey = namespace ? null : [...this.chunks.keys()].find((k) => k.endsWith(`:${chunkId}`));
+ const chunk = this.chunks.get(namespace ? `${namespace}:${chunkId}` : legacyKey ?? '') ?? null;
- async loadChunk(original, url, done, key, chunkId): Promise {
- const chunkUrl = this.chunkUrl(chunkId) || url;
+ async loadChunk(original, url, done, key, chunkId, namespace?: string): Promise {
+ const chunkUrl = this.chunkUrl(chunkId, namespace) || url;
- chunkUrl(chunkId: number | string): string | null {
- const chunk = this.getChunk(chunkId.toString());
+ chunkUrl(chunkId: number | string, namespace?: string): string | null {
+ const chunk = this.getChunk(chunkId.toString(), namespace);
```
plus the same optional parameters on `IChunkRegistry` (`:48`, `:53`).
`js-packages/webpack-config/src/OverrideChunkLoaderFunction.cjs:21`, so the runtime can say which extension is asking. `namespace` is already computed at `:11`:
```diff
- '\nconst originalLoadChunk = __webpack_require__.l;' +
- '\n__webpack_require__.l = flarum.reg.loadChunk.bind(flarum.reg, originalLoadChunk);'
+ '\nconst originalLoadChunk = __webpack_require__.l;' +
+ '\n__webpack_require__.l = (url, done, key, chunkId) =>' +
+ ` flarum.reg.loadChunk(originalLoadChunk, url, done, key, chunkId, ${JSON.stringify(namespace)});`
```
With both applied, the correct file is fetched and the correct module is returned.
Notes:
- `namespace` is optional and last, so bundles built with the current webpack-config keep working. They take the legacy branch and get today's first-registrant-wins behaviour.
- The fix only helps extensions rebuilt with the updated webpack-config, since the namespace has to come from the emitted runtime. Already published bundles keep colliding.
- `getChunk`, `chunkUrl` and `loadChunk` are on the public `IChunkRegistry`, but I found no call site outside `ExportRegistry` itself and the webpack runtime, in core, `js-packages` or `extensions/*/js/src`.
- Falling back to the `url` webpack computed does not work here. `Extend\Frontend` adds every extension's dist as a source file to the same compiler and `Assets::makeJs()` concatenates them into one asset, so every runtime auto-detects the same `publicPath`. That is why core overrides `__webpack_require__.l` in the first place.
- If changing the signatures is not wanted, a `console.warn` in `addChunkModule` when a second namespace registers an id already present would at least make this diagnosable. That is two lines and fixes nothing.
Until there is a release, an extension can work around it with `optimization.chunkIds: false` plus a small plugin setting `chunk.id` to `/`, and `output.chunkLoadingGlobal: 'webpackChunk'`. Both are needed. Over a 29 file package that cost me 332 gzip bytes in total.
I am happy to open a PR with the two file change and a regression test.
Contributor guide
Research direction
Start with framework/core/js/src/common/ExportRegistry.ts, especially addChunkModule, getChunk, chunkUrl, and loadChunk, then inspect js-packages/webpack-config/src/OverrideChunkLoaderFunction.cjs. Reproduce the collision using the two names in the issue and trace the emitted runtime namespace. Done means independently registered extension chunks resolve to their own assets and modules, with a regression test covering the collision.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript, webpack
- Domain
- build-system, frontend, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100