Skip to content

Silent data loss via WOPI PutFile on corrupted force-save (CONVERT_CORRUPTED) — unfixable host-side #42

Description

@moodyjmz

Summary

When a document's server-side conversion fails during a force-save (CONVERT_CORRUPTED, the class of failure Euro-Office/sdkjs#80 caused), DocumentServer's WOPI push path (processWopiPutFile) still fires and pushes the corrupted/empty output to the WOPI host. Unlike the non-WOPI callback path, this is unfixable on the host side — WOPI's PutFile request carries no status field at all, so a WOPI host has no way to distinguish a genuine save from a corrupted one, even in principle.

This is the same underlying failure class as eurooffice-nextcloud#160, but worse: the callback-based connector at least receives a status: 7 (CorruptedForce) signal and currently discards it (fixable connector-side). A WOPI host is never told at all — there is nothing to discard. The fix can only live here, in canvasservice.js.

Root cause

DocService/sources/canvasservice.js:1040-1042:

let isError = constants.NO_ERROR != statusInfo;
const isErrorCorrupted = constants.CONVERT_CORRUPTED == statusInfo;
const savePathDoc = saveKey + '/' + cmd.getOutputPath();

(constants.CONVERT_CORRUPTED is -86Common/sources/constants.js:233 — matching the x2t ExitCode: 86 seen in the sdkjs#80 crash class.)

For the non-WOPI callback path, canvasservice.js:1136-1171 correctly ends up sending statusErr (CorruptedForce, 7) to the connector — isError stays true through the whole block and gets applied at line 1171, overriding the statusOk set at line 1165. (That part is working as intended; see the design note below.)

For the WOPI path, no equivalent check exists. Both call sites gate purely on outputSfc.getUrl():

// canvasservice.js:1188-1197 (isSfcm / force-save branch)
if (wopiParams) {
  if (outputSfc.getUrl()) {
    ...
    replyStr = yield processWopiPutFile(ctx, docId, wopiParams, savePathDoc, userLastChangeId, true, isAutoSave, false);
  }
// canvasservice.js:1232-1237 (regular save-on-exit branch)
if (wopiParams) {
  if (outputSfc.getUrl()) {
    replyStr = yield processWopiPutFile(ctx, docId, wopiParams, savePathDoc, userLastChangeId, !notModified, false, true);
  }

outputSfc.getUrl() is truthy in exactly the failure case this issue is about — the isErrorCorrupted bypass at line 1136 exists specifically so a URL to the (corrupted) output can still be attached for the callback flow's benefit (see below). Neither WOPI call site checks isError or isErrorCorrupted before pushing.

processWopiPutFile (line 1357) then unconditionally streams savePathDoc — the corrupted output — to wopiClient.putFile(), which POSTs it directly to the WOPI host with no accompanying status:

function* processWopiPutFile(ctx, docId, wopiParams, savePathDoc, userLastChangeId, isModifiedByUser, isAutosave, isExitSave) {
  ...
  const postRes = yield wopiClient.putFile(ctx, wopiParams, null, streamObj.readStream, metadata.ContentLength, ...);

Why this can't be fixed host-side

The WOPI PutFile operation (MS-WOPI spec) is just bytes over HTTP — there's no status/error channel analogous to the callback protocol's status field. A WOPI host implementation, however careful, receives a well-formed HTTP PUT with a plausible byte count and nothing to indicate the content is actually the unconverted/empty template rather than the user's edit. Compare to the callback path, where eurooffice-nextcloud#160 is a real, fixable bug — the connector receives status: 7 and should act on it, but at least the information exists at the host boundary.

Design intent vs. the gap

The isErrorCorrupted bypass at line 1136 looks deliberate for the callback path: send status: 7, but also attach a URL to the partial/corrupted output in case the host wants to inspect or keep it — the decision of what to do with a corrupted result is left to the host, informed by the status. That design assumes the host always gets the status alongside the URL. WOPI's PutFile breaks that assumption: the URL-equivalent (the byte stream) travels alone, with no status ever accompanying it.

Suggested direction

Gate both WOPI call sites (canvasservice.js:1188 and :1232) on !isErrorCorrupted (or !isError) in addition to outputSfc.getUrl(), so a corrupted conversion never reaches processWopiPutFile. On the corrupted branch, log at error level and skip the WOPI push entirely — there's no host-side signal to substitute for "don't do this."

Relevance

eo-nc (the callback-based Nextcloud connector) is being retired in favor of a WOPI-based integration. This means the replacement integration path inherits an exposure that is structurally worse than the one being retired, and one with no possible host-side mitigation — it has to land here before or alongside that migration.

Verification status

This is a code-level finding (control flow + constant values traced and cross-checked), not yet reproduced against a live WOPI push. A quick attempt to reproduce via document-server-integration's reference WOPI host (/example/wopi-new under a packaged v9.3.4 image with WOPI_ENABLED=true) hit a 504 through nginx before reaching the point of testing the actual push — that's a separate wiring issue in the packaged image worth a look on its own, not a comment on whether this finding is real. Confirming this live (e.g. via a debug build with WOPI properly reachable, forcing CONVERT_CORRUPTED, and inspecting what actually lands at a WOPI host's PutFile endpoint) would be worth doing before treating this as fully closed.

Related

eurooffice-nextcloud#160 — same underlying trigger (CONVERT_CORRUPTED / sdkjs#80 crash class), callback-path symptom, connector-side fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions