Skip to content

fix(image): a malformed data URI on an inline <img> aborts the entire document #228

Description

@amitsharma-turbodocx

Change type

Planned — a non-trivial fix

Risk / impact

Medium — no data loss, but a single bad <img> in user-supplied HTML currently costs the caller the whole document rather than one image.

Details — description & full context

Summary

createMediaFile() throws Invalid base64 string on a data URI it cannot parse. On the buildImage path that throw is caught and the image is skipped; on the buildRun path it is not caught, so the rejection propagates out of HTMLtoDOCX() and the caller gets no document at all.

The result is that the same malformed input behaves differently depending on where the <img> sits in the markup:

Markup Outcome
<p><img src="data:image/png;base64,"></p> image skipped, document generated
<p><span><img src="data:image/png;base64,"></span></p> throws Invalid base64 string, whole document lost
<p><span><img src="data:image/png,notbase64"></span></p> throws, whole document lost
<p><span><img src=""></span></p> image skipped, document generated

Wrapping the image in any inline element is enough to flip the behaviour, because that routes it through buildRun instead of buildImage.

Repro

const HTMLtoDOCX = require('@turbodocx/html-to-docx')

// generates fine — image skipped
await HTMLtoDOCX('<p><img src="data:image/png;base64,"/></p>', null, {})

// rejects with: Invalid base64 string
await HTMLtoDOCX('<p><span><img src="data:image/png;base64,"/></span></p>', null, {})

Cause

parseDataUrl() matches /^data:([A-Za-z-+/]+);base64,(.+)$/. An empty payload has nothing for (.+) to match, so it returns null and createMediaFile() throws:

https://github.com/TurboDocx/html-to-docx/blob/main/src/docx-document.js#L513-L517

The two callers then diverge:

  • src/utils/image.js — buildImage wraps createMediaFile in try/catch and returns null, so the image is dropped and generation continues.
  • src/helpers/xml-builder.js:1121 — buildRun calls createMediaFile unguarded:
const base64Uri = decodeURIComponent(vNode.properties.src);
if (base64Uri) {
  response = await docxDocumentInstance.createMediaFile(base64Uri);   // <-- no try/catch
}

if (response) { ... }

The if (response) check below it already anticipates a missing response, so the surrounding code is written for a skip — only the throw is unhandled.

Scope

Long-standing and unrelated to any recent change. Reproduced identically on @turbodocx/html-to-docx@1.22.0 (which uses image-size) and on the probe-image-size branch, so it is not a regression from #227 — it surfaced while building that PR's test matrix.

It matters most for callers rendering HTML they do not control (CMS content, email bodies, API-supplied markup), where one truncated or hand-edited data URI takes down the entire generation.

Suggested fix

Wrap the createMediaFile call in buildRun in the same try/catch buildImage already uses — log a warning naming the source and return runFragment so the rest of the document survives. Consider extracting the shared "turn a source into a media file, or skip it" logic so the two paths cannot drift again.

Worth deciding as part of this: should parseDataUrl treat an empty payload as a parse failure returning null rather than letting createMediaFile throw? That would make the failure mode uniform at the source instead of relying on every caller to guard.

Testing & validation

  • CI passes on the PR
  • Tests added/updated for the change — cover both the buildImage and buildRun paths with an empty payload, a non-base64 payload, and a truncated one, asserting the document still generates

Rollback plan

Revert the merge commit. The change only widens an existing error path, so there is no state or output migration to undo.

Breaking change?

No — callers that currently receive a rejection would instead receive a document with that one image missing. Anyone relying on the throw to detect bad input would need to check the warning output instead, but that behaviour is already inconsistent between the two paths today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingchangeSOC 2 change management record

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions