mirror of
https://codeberg.org/PostERG/xamxam.git
synced 2026-09-25 09:53:08 +02:00
378 lines
15 KiB
Markdown
378 lines
15 KiB
Markdown
# FilePond crash analysis — TFE upload forms
|
||
|
||
Status: **unresolved** — analysis complete, root cause identified in vendor code.
|
||
Hand this doc (and the whole repo) to another agent for implementing the fix.
|
||
|
||
---
|
||
|
||
## Errors observed (Firefox, dev server 127.0.0.1:8000)
|
||
|
||
Trigger: adding an image to "Image de couverture" (cover queue) or any TFE file upload form.
|
||
|
||
```
|
||
InstallTrigger is deprecated and will be removed in the future. content.js:1
|
||
Failed to execute 'postMessage' on 'DOMWindow': target origin mismatch 2 20260609-...
|
||
htmx:targetError htmx.min.js:1
|
||
Uncaught TypeError: can't access property "main", n.status is undefined filepond.min.js:9
|
||
Wt filepond.min.js:9
|
||
A filepond.min.js:9
|
||
A filepond.min.js:9
|
||
_write filepond.min.js:9 (×16)
|
||
<anonymous> filepond.min.js:9
|
||
<anonymous> filepond.min.js:9
|
||
e filepond.min.js:9
|
||
e filepond.min.js:9
|
||
u filepond.min.js:9 (× many — retry loop)
|
||
e filepond.min.js:9
|
||
u ...
|
||
|
||
[filepond:event] error Object { pond: {…}, error: null, file: {…} } file-upload-filepond.js:587
|
||
```
|
||
|
||
Rows 1–3 are noise: `InstallTrigger`/`postMessage` are Firefox internals; `htmx:targetError` is an unrelated HTMX issue. The real crash is rows 4+.
|
||
|
||
---
|
||
|
||
## Root cause
|
||
|
||
### The crash
|
||
|
||
At `filepond.min.js:9:60852`, FilePond 4.32.12 crashes inside its view system's `_write` method. Two view writers dereference `action.status.main`:
|
||
|
||
**FilePond unminified (file-status view), line 7847:**
|
||
```js
|
||
var error = function error(_ref8) {
|
||
var root = _ref8.root,
|
||
action = _ref8.action;
|
||
text(root.ref.main, action.status.main); // ← crashes if action.status is undefined
|
||
text(root.ref.sub, action.status.sub);
|
||
};
|
||
```
|
||
|
||
**FilePond unminified (assistant view), line 10735:**
|
||
```js
|
||
var itemError = function itemError(_ref6) {
|
||
var root = _ref6.root,
|
||
action = _ref6.action;
|
||
var item = root.query('GET_ITEM', action.id);
|
||
var filename = item.filename;
|
||
assist(root, action.status.main + ' ' + filename + ' ' + action.status.sub);
|
||
};
|
||
```
|
||
|
||
Neither function guards against `action.status === undefined`.
|
||
|
||
### How `action.status` becomes undefined
|
||
|
||
FilePond's internal response objects use the property name **`code`**, not `status`:
|
||
|
||
```js
|
||
// line 4700
|
||
var createResponse = function createResponse(type, code, body, headers) {
|
||
return {
|
||
type: type,
|
||
code: code, // ← "code", not "status"
|
||
body: body,
|
||
headers: headers,
|
||
};
|
||
};
|
||
```
|
||
|
||
But the `load-file-error` event handler accesses `error.status`:
|
||
|
||
```js
|
||
// line 6777-6784
|
||
item.on('load-file-error', function(error) {
|
||
dispatch('DID_THROW_ITEM_INVALID', {
|
||
id: id,
|
||
error: error.status, // ← .status is undefined on createResponse objects!
|
||
status: error.status, // ← dispatches undefined as the status
|
||
});
|
||
failure({ error: error.status, file: createItemAPI(item) });
|
||
});
|
||
```
|
||
|
||
Because the error object has `.code` (not `.status`), both `error: error.status` and `status: error.status` are `undefined`. When the dispatched action reaches the view writer, `action.status` is `undefined` → crash.
|
||
|
||
### When does `load-file-error` fire?
|
||
|
||
The `load-file-error` event is emitted in the item `_load` method when the `LOAD_FILE` filter chain **rejects**:
|
||
|
||
```js
|
||
// line 5855-5863
|
||
loader.on('load', function(file) {
|
||
var error = function error(result) {
|
||
state.file = file;
|
||
fire('load-meta');
|
||
setStatus(ItemStatus.LOAD_ERROR);
|
||
fire('load-file-error', result); // ← fires when filter chain rejects
|
||
};
|
||
|
||
if (state.serverFileReference) {
|
||
success(file); // ← existing files take this safe path
|
||
return;
|
||
}
|
||
|
||
onload(file, success, error); // ← new files take this path
|
||
});
|
||
```
|
||
|
||
For existing DB files (edit mode), `state.serverFileReference` is set → `success(file)` is called directly → `load-file-error` never fires.
|
||
|
||
For **newly added files** (no serverId yet), `onload(file, success, error)` runs the `LOAD_FILE` filter chain. The FilePond **FileValidateType** plugin (v1.2.8) registers a `LOAD_FILE` filter:
|
||
|
||
```js
|
||
// plugin line 132
|
||
addFilter('LOAD_FILE', function(file, _ref3) {
|
||
// ...
|
||
var handleRejection = function handleRejection() {
|
||
reject({
|
||
status: { main: '...', sub: '...' } // ← plugin rejects with proper status object
|
||
});
|
||
};
|
||
// ...
|
||
});
|
||
```
|
||
|
||
The plugin rejects with `{ status: { main, sub } }`. This is CORRECT. The rejected value flows into the `error(result)` callback → `result.status` IS `{ main, sub }`. So when `load-file-error` fires from a plugin rejection, `error.status` is actually a proper object, NOT undefined. **This particular path is safe.**
|
||
|
||
However, `load-file-error` can also fire from the `DID_LOAD_ITEM` filter chain `catch` handler at line 6878:
|
||
|
||
```js
|
||
.catch(function(e) {
|
||
if (!e || !e.error || !e.status) return handleAdd(false);
|
||
dispatch('DID_THROW_ITEM_INVALID', {
|
||
id: id,
|
||
error: e.error,
|
||
status: e.status, // ← e.status could be anything
|
||
});
|
||
});
|
||
```
|
||
|
||
This dispatches directly to `DID_THROW_ITEM_INVALID` (bypassing `load-file-error`), but it still copies `e.status` into the action. If `e.status` is undefined or not an object with `main`/`sub`, same crash.
|
||
|
||
### How raw createResponse objects reach `load-file-error`
|
||
|
||
There IS one path where a raw `createResponse` object (with `.code`, no `.status`) reaches `load-file-error`:
|
||
|
||
When the server returns an HTTP error for a **load** request but the XHR onload handler treats it as success:
|
||
|
||
```js
|
||
// line 4652
|
||
xhr.onload = function() {
|
||
if (xhr.status >= 200 && xhr.status < 300) {
|
||
api.onload(xhr); // → blob processed
|
||
} else {
|
||
api.onerror(xhr); // → error callback
|
||
}
|
||
};
|
||
```
|
||
|
||
If `xhr.status` is 0 (aborted XHR), it goes to `api.onerror`. But if the XHR is in some intermediate state, or if there's a race, it might not reach either path cleanly. Firefox's behavior with aborted XHRs and `responseType: 'blob'` can produce edge cases where `xhr.response` is a malformed blob and FilePond's blob processing triggers internal errors that propagate differently.
|
||
|
||
### Summary of the bug
|
||
|
||
| Component | Issue |
|
||
|-----------|-------|
|
||
| `createResponse()` (line 4700) | Uses `.code`, not `.status` |
|
||
| `load-file-error` handler (line 6777) | Reads `.status` on a createResponse object → `undefined` |
|
||
| Error view writer (line 7847) | No guard: crashes on `undefined.status.main` |
|
||
| Assistant view writer (line 10735) | Same: crashes on `undefined.status.main` |
|
||
|
||
The bug is in FilePond 4.32.12 vendor code. We cannot modify `filepond.min.js`.
|
||
|
||
---
|
||
|
||
## Proposed fix
|
||
|
||
### Option A: Patch the minified JS (risky but direct)
|
||
|
||
Find the `load-file-error` → `DID_THROW_ITEM_INVALID` dispatch in `filepond.min.js` and add a guard. Difficult because the code is minified and version-pinned via cache-busting query params.
|
||
|
||
### Option B: Replace server.load with a custom function (cleanest)
|
||
|
||
In `file-upload-filepond.js`, replace the `server.load` URL string with a **custom function** that:
|
||
|
||
1. Makes its own `fetch`/XHR to load.php
|
||
2. On success: calls `load(blob)` — safe because `serverFileReference` is set for existing files
|
||
3. On error: calls `error('message')` — safe because this goes through `load-request-error` (NOT `load-file-error`) which properly creates `{ status: { main, sub } }`
|
||
4. Never lets FilePond's internal `createFetchFunction` create a createResponse object with `.code`
|
||
|
||
This completely bypasses the buggy code path.
|
||
|
||
```js
|
||
load: function(source, load, error, progress, abort, headers) {
|
||
var xhr = new XMLHttpRequest();
|
||
var url = base + '/load.php?id=' + encodeURIComponent(source);
|
||
xhr.open('GET', url);
|
||
xhr.responseType = 'blob';
|
||
xhr.onload = function() {
|
||
if (xhr.status >= 200 && xhr.status < 300) {
|
||
load(xhr.response);
|
||
} else {
|
||
error('Fichier introuvable (HTTP ' + xhr.status + ')');
|
||
}
|
||
};
|
||
xhr.onerror = function() {
|
||
error('Erreur réseau');
|
||
};
|
||
xhr.onabort = abort;
|
||
xhr.onprogress = function(e) {
|
||
if (e.lengthComputable) progress(e.lengthComputable, e.loaded, e.total);
|
||
};
|
||
xhr.send();
|
||
return { abort: function() { xhr.abort(); } };
|
||
},
|
||
```
|
||
|
||
### Option C: Abort in-flight loads before destroying (defense in depth)
|
||
|
||
The `destroyFilePondsIn()` function in `file-upload-filepond.js` should abort in-flight loads/processing before calling `pond.destroy()`. Already partially attempted in commit `znunoqpw` but needs clean implementation.
|
||
|
||
---
|
||
|
||
## Files involved
|
||
|
||
| File | Role |
|
||
|------|------|
|
||
| `app/public/assets/js/vendor/filepond.min.js` | FilePond 4.32.12 — **contains the bug** (unmodifiable) |
|
||
| `app/public/assets/js/app/file-upload-filepond.js` | Our FilePond wrapper — **where the fix goes** |
|
||
| `app/src/FilepondHandler.php` | Server-side FilePond endpoints (process, load, revert, remove) |
|
||
| `app/public/admin/actions/filepond/load.php` | Admin load endpoint |
|
||
| `app/public/admin/actions/filepond/process.php` | Admin process endpoint |
|
||
| `app/public/partage/actions/filepond/load.php` | Partage load endpoint |
|
||
| `app/public/partage/actions/filepond/process.php` | Partage process endpoint |
|
||
|
||
---
|
||
|
||
## Reproduction
|
||
|
||
1. `just dev` (PHP dev server on 127.0.0.1:8000)
|
||
2. Open Firefox (Firefox triggers this more readily than Chromium due to different XHR abort behavior)
|
||
3. Go to `/admin/edit.php?id=<any>` or `/admin/add.php`
|
||
4. Click "Parcourir" on the "Image de couverture" FilePond input
|
||
5. Select an image file → crash in console
|
||
6. Or: drag a file to the "TFE" FilePond input → same crash if the load fails or races with HTMX swaps
|
||
|
||
---
|
||
|
||
## What commit `znunoqpw` already did (insufficient)
|
||
|
||
- Added `Content-Type: text/plain` headers to all FilepondHandler error responses
|
||
- Fixed `server.process.onerror` to not access `.status` on a string
|
||
- Converted `server.load` from a URL string to an object with onload/onerror
|
||
- Added pre-destroy abort in `destroyFilePondsIn()`
|
||
|
||
These changes address server response format and cleanup ordering, but **do not bypass the buggy `load-file-error` → `action.status` path inside FilePond's internal code**. The crash still reproduces.
|
||
|
||
---
|
||
|
||
# HTMX/destroy race investigation (merged from filepond-race-investigation.md)
|
||
|
||
This section narrows the crash's trigger. It was formerly a separate doc
|
||
(`filepond-race-investigation.md`).
|
||
|
||
## HTMX destroy triggers
|
||
|
||
The only code path that destroys FilePond instances is `destroyFilePondsIn(el)`, called by the `htmx:beforeSwap` listener:
|
||
|
||
```js
|
||
window.htmx.on("htmx:beforeSwap", onHtmxBeforeSwap);
|
||
// → onHtmxBeforeSwap(evt) { destroyFilePondsIn(evt.detail.target); }
|
||
```
|
||
|
||
On the **edit page** (`/admin/edit.php`), the HTMX targets on page load are:
|
||
|
||
| Element | Trigger | Target selector | Scope |
|
||
|---------|---------|-----------------|-------|
|
||
| `#toast-region` | `load` | `#toast-region` | Footer `<aside>` |
|
||
| `.licence-license-choice` (hidden input) | `load` | `.licence-license-choice` | Licence fieldset |
|
||
| Language checkboxes | `change` | `#languages-required-asterisk` | A `<span>` |
|
||
| File browser buttons | `click` | `#relink-modal-body` | Modal body |
|
||
| Jury autocomplete | `change` | small targets | Form field |
|
||
| Tag search input | `input` | pill list container | Form field |
|
||
| Licence radio buttons | `change` | `.licence-license-choice` | Licence fieldset |
|
||
|
||
**None of these targets are ancestors of the `#format-fichiers-block` div**
|
||
(which contains all FilePond inputs including the cover queue). Therefore **no
|
||
HTMX swap on the edit page can trigger `destroyFilePondsIn` on the FilePond
|
||
container during normal operation.**
|
||
|
||
The `htmx:targetError` in the crash log is confirmed noise: `targetError` does
|
||
**not** fire `htmx:beforeSwap`, so no DOM swap occurs.
|
||
|
||
**Verdict: HTMX does NOT swap the FilePond container. The race hypothesis as stated is refuted.**
|
||
|
||
## In-flight state at file-pick time
|
||
|
||
No HTMX request is in flight when the crash occurs: the toast-region's
|
||
`hx-get` completes quickly (sub-second, 204 or small fragment) long before a
|
||
human clicks "Parcourir" and selects a file. Other triggers require explicit
|
||
user interaction; the native file picker is modal and blocks the main thread.
|
||
|
||
## `znunoqpw` abort analysis
|
||
|
||
Commit `znunoqpw` added a pre-destroy abort in `destroyFilePondsIn`:
|
||
|
||
```js
|
||
for (var i = 0; i < files.length; i++) {
|
||
var f = files[i];
|
||
if (f.status === 4 || f.status === 2 || f.status === 3) {
|
||
try { pond.removeFile(f); } catch (_abort) {}
|
||
}
|
||
}
|
||
pond.destroy();
|
||
```
|
||
|
||
**The status check is incorrect.** FilePond 4.32.12 internal status constants
|
||
are `INIT:1`, `IDLE:2`, `PROCESSING:3`, `PROCESSING_COMPLETE:5`, `LOADING:7`,
|
||
`LOAD_ERROR:8`, `PROCESSING_QUEUED:9`. The check catches `2` (IDLE, no-op),
|
||
`3` (PROCESSING), and `4` (does not exist). Status `7` (LOADING) is **not
|
||
caught**, so a file in the LOAD_FILE filter chain is never removed.
|
||
|
||
However, `pond.destroy()` → `ABORT_ALL` freezes items and calls `abortLoad()`.
|
||
Since `activeLoader` is null during the LOAD_FILE chain, the else branch sets
|
||
status INIT + fires `load-abort`; the chain Promise still runs but the freeze
|
||
gate (`i.frozen`) suppresses event dispatch. **So the abort mechanism prevents
|
||
the crash after destroy, but only when `destroyFilePondsIn` is actually called
|
||
— which it never is in the standard repro (HTMX never swaps the container).**
|
||
|
||
## Line 6878 catch reachability
|
||
|
||
Two paths dispatch `DID_THROW_ITEM_INVALID` to the `file-status` view writer:
|
||
|
||
```js
|
||
Wt = function(e) {
|
||
var t = e.root, n = e.action;
|
||
Nt(t.ref.main, n.status.main); // ← crashes if n.status undefined
|
||
Nt(t.ref.sub, n.status.sub);
|
||
};
|
||
```
|
||
|
||
- **Path A — `load-request-error` → SAFE.** Both branches wrap rejection in
|
||
`{ status: { main, sub } }`. No crash.
|
||
- **Path B — `load-file-error` → VULNERABLE.** Passes `t.status` through
|
||
unguarded. For local files, LOAD_FILE plugins (`FileValidateType`/`FileValidateSize`)
|
||
reject with a proper `{ status: { main, sub } }`. For server-loaded files with
|
||
error responses, `createResponse` has `.code` not `.status`, but the FileValidateType
|
||
filter still wraps its rejection correctly → still safe.
|
||
- **`.catch` handler (line 6878)** has an explicit `!t.status` guard → cannot crash.
|
||
|
||
## Verdict
|
||
|
||
- **HTMX race hypothesis: REFUTED** (no swap targets the FilePond container;
|
||
freeze gate prevents post-destroy dispatch).
|
||
- **Actual crash cause: INDETERMINATE (but narrowed).** The only vulnerable path
|
||
is `load-file-error` → `DID_THROW_ITEM_INVALID` with `status: undefined`. For
|
||
local file selection, the exact path to `undefined` status isn't identified.
|
||
The most likely trigger is a **Firefox-specific XHR abort edge case** in the
|
||
existing cover file's `server.load`, racing with adding a new local file.
|
||
|
||
## Recommended next step
|
||
|
||
Add `console.log` instrumentation to `server.load`'s onload/onerror and a global
|
||
`FilePond:error` / `window.error` trap, then reproduce in Firefox. If
|
||
`server.load onload` fires immediately before the crash, the race is confirmed
|
||
and the fix is **Option B** (custom `fetch`-based `server.load` that never
|
||
routes server responses through the LOAD_FILE filter chain).
|