Skip to content

fix(md): keep the image of an inline SVG data URI - #4925

Closed
simpleqt wants to merge 1 commit into
docling-project:mainfrom
simpleqt:fix/md-inline-svg-image
Closed

simpleqt wants to merge 1 commit into
docling-project:mainfrom
simpleqt:fix/md-inline-svg-image

Conversation

@simpleqt

Copy link
Copy Markdown
Contributor

Keep the image of an inline SVG data URI

Fixes #4742.

With fetch_images=True, an inline data:image/png is decoded, but an inline
data:image/svg+xml becomes a picture with no image. ImageResourceLoader.create_image_ref
hands the decoded bytes to PIL.Image.open(), Pillow has no SVG decoder, the
UnidentifiedImageError is caught, the loader warns and returns None — so the
payload is dropped:

picture #/pictures/0: image=(120.0, 80.0)          # PNG
picture #/pictures/1: image=None                   # SVG
UserWarning: Could not process an image from data:image/svg+xml;base64,…:
             cannot identify image file

The loader now catches that error and, when the source is a data URI declaring
image/svg+xml, returns an ImageRef that keeps the decoded payload:
mimetype="image/svg+xml", dpi=72, size taken from the SVG's width/height
(falling back to its viewBox), and uri set to the original data URI, which the
Markdown and HTML serializers already emit as-is. Both pictures then report
image=(120.0, 80.0), and no warning is raised.

The size attributes are read with regular expressions rather than an XML parser,
so a malformed or hostile document (entity expansion included) cannot affect the
loader. Nothing else changes: PNG/JPEG take the byte-identical Pillow path, any
other undecodable payload still warns and leaves a placeholder
(data:image/png;base64,aW52YWxpZA== behaves as before, which an existing test
pins), and local/remote .svg sources are still skipped.

Testing

Two tests in tests/test_backend_markdown_images.py:

  • test_inline_svg_data_uri_keeps_the_image (parametrized over fetch_images) —
    checks no warning, that the PNG still yields real pixels through Pillow, that
    the SVG keeps its payload, size and URI, and that the payload survives both an
    ImageRefMode.EMBEDDED export and a model_validate(export_to_dict()) round trip
  • test_inline_svg_data_uri_size_comes_from_the_view_box

On the base commit the inline-SVG tests fail (the warning fires, and
assert None is not None for the missing image); with the fix the module is
28 passed, and test_backend_markdown_images.py +
test_image_resource_loader.py + test_backend_markdown.py are 97 passed.
ruff at both the version pyproject.toml pins (0.15.12) and the pre-commit rev
(0.16.10) is clean, including the import-sort form CI uses for a
--no-install-project install.

A trade-off worth your judgement

Turning an SVG into pixels needs an SVG renderer, which this change deliberately
does not add (that is a dependency-packaging decision). So for these refs
ImageRef.pil_image / PictureItem.get_image() raise UnidentifiedImageError
rather than returning None — the difference matters to consumers that need
pixels, such as picture-enrichment models. Everything else, including Markdown and
HTML export and JSON round trips, now preserves the SVG payload. If you would
rather the loader rasterize instead, this change is the wrong shape and I would
withdraw it; @iluv7's comment on the issue describes that alternative. The shared
loader means the HTML path benefits too: <img src="data:image/svg+xml;base64,…">
now yields ('image/svg+xml', (40.0, 20.0)).

AI assistance: this change was produced with AI assistance (an autonomous coding
agent) from the reporter's issue; the analysis, the reproduction, the
before/after test runs and the lint runs were executed locally, and a human
reviewed the result before submission.

With `fetch_images=True`, an inline `data:image/svg+xml` image became a picture
without an image: the loader handed the decoded SVG to `PIL.Image.open()`, which
has no SVG decoder, caught `UnidentifiedImageError` and returned `None`, so the
payload was lost.

Keep the decoded payload of such a data URI as the image resource. The ref
carries the declared media type, the size the SVG declares (its `width` and
`height`, or its `viewBox`), and the original data URI, which the Markdown and
HTML serializers emit as-is, so the SVG survives exports and round trips.

Raster payloads are untouched: PNG and JPEG still go through Pillow, and any
other payload that Pillow cannot identify still warns and leaves a placeholder.
Rendering an inline SVG into pixels needs an SVG renderer, so
`ImageRef.pil_image` stays unavailable for these refs.

Signed-off-by: simpleqt <2015900818@qq.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 02:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @simpleqt, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@github-actions github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) bug Something isn't working enhancement New feature or request markdown issue related to markdown backend labels Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

AI triage (advisory)

Checked commit 9d28881b19. The analysis read the diff only. It did not run code from this pull request.

The PR keeps the decoded payload of an inline data:image/svg+xml image when fetch_images=True. It builds an ImageRef with the SVG media type, a size read from width/height or viewBox, and the original data URI. It adds two tests.

Topic labels added: bug, markdown, enhancement.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code
PR State Signal Assessment
#4912 open changes the same code related: Both PRs edit image_resource_loader.py, but at different places. PR 4912 changes how file:// base paths resolve. This PR changes how inline SVG data URIs load. The two changes do not conflict.

The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI first review (advisory): blocker found

This review covers the full diff at 9d28881b19. It read the code and did not run it.

The SVG ImageRef has no pixels. ImageRef.pil_image raises UnidentifiedImageError for it, and get_image() raises with it. Before this PR the SVG picture had image=None and these paths skipped it. Check that exports in REFERENCED mode and picture enrichment still work for a Markdown file that holds an inline SVG.

A maintainer makes the merge decision. This review never approves the pull request.

# of the picture. Any other payload keeps the warning.
if (svg_ref := _inline_svg_image_ref(src_url, img_data)) is None:
raise
return svg_ref

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: SVG ImageRef makes PictureItem.get_image() raise instead of returning None

Trigger: Markdown or HTML input with ![x](data:image/svg+xml;base64,...) and fetch_images=True. This is now the default path for such input.

Cause: The new ImageRef has uri=data:image/svg+xml;... and no _pil. FloatingItem.get_image() returns self.image.pil_image. For a data: URI, ImageRef.pil_image calls PILImage.open(BytesIO(decoded)). It has no try/except, so it raises UnidentifiedImageError. This happens in docling_core/types/doc/common/reference.py (the pil_image property) and in docling_core/types/doc/items/node.py (FloatingItem.get_image).

Wrong result: Before this PR the picture had image=None. get_image() then returned None for a picture with no provenance, and callers skipped it. Now two paths crash:

  • DoclingDocument._with_pictures_refs calls item.get_image(doc=self) with no guard. It runs for export_to_markdown, export_to_html and save_as_* with image_mode=ImageRefMode.REFERENCED. A document with one inline SVG can no longer be exported in this mode.
  • BaseItemAndImageEnrichmentModel.prepare_element in docling/models/base_model.py:197 calls element.get_image(conv_res.document). The picture description, classification and similar enrichment stages will crash on the SVG picture.

The docstring says pil_image is "not available" for such a ref. In fact it raises, and the new code does not guard the callers.

Test gap: The tests use only ImageRefMode.EMBEDDED and export_to_dict. Neither calls pil_image. No test covers REFERENCED mode or get_image() on the SVG picture.

Fix: Do not hand out an ImageRef that pil_image cannot open. Options:

  • Keep image=None and store the SVG payload elsewhere, for example in the picture meta.
  • Rasterize the SVG so the ref has real pixels.
  • Change docling_core so that pil_image returns None for non-raster mimetypes.

Add a regression test that exports with ImageRefMode.REFERENCED and calls get_image(doc) on the SVG pictu…

@github-actions github-actions Bot added the ai:review-changes AI first review suggests changes label Oct 21, 2026
@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
docling/backend/utils/image_resource_loader.py 86.48% 3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@simpleqt

simpleqt commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Withdrawing: this change trades silent data loss for a crash

I am closing this PR. Independent checking found a regression I introduced, and the fix belongs in docling-core rather than here.

What is still true. The reported bug is real: with fetch_images=True an inline data:image/svg+xml decoded fine but create_image_ref handed the bytes to PIL.Image.open(), the UnidentifiedImageError was swallowed, and the picture lost its image.

What is wrong with my fix. It handed back an ImageRef that Pillow cannot open. FloatingItem.get_image() is documented and typed as returning PILImage.Image | None, but it does return self.image.pil_image with no guard, and ImageRef.pil_image calls PIL.Image.open(BytesIO(decoded)) for a data: URI without a try. Before this PR the SVG picture had image=None and callers skipped it; now they raise. Measured on my branch vs the base commit, same probe:

scenario base this PR
PictureItem.get_image(doc) on the SVG picture returns None raises UnidentifiedImageError
export_to_markdown(image_mode=ImageRefMode.REFERENCED) works works
export_to_html(image_mode=ImageRefMode.REFERENCED) works works
PNG-only control, REFERENCED export works works

So the export path I worried about is fine, but get_image() — used by BaseItemAndImageEnrichmentModel.prepare_element (picture description, classification) — now raises for this picture instead of returning None. That is a worse failure than a missing image and it is a regression against the base commit, which is enough to withdraw. My own PR body said this shape was wrong and should be withdrawn; that was correct.

Where the fix belongs. The contract violation is in docling-core: get_image() promises None when no image is available and instead propagates the decoder's exception. Any ImageRef that cannot be decoded does this today, including a corrupt data: URI or a file:// URI that no longer exists — the inline-SVG case only makes it reachable from Markdown/HTML, which is what surfaced it. I have opened docling-project/docling-core#916 for that. Once get_image() returns None for an unopenable ref, this PR's approach becomes viable and I am happy to reopen it.

On the AI reviewer's blocker. The advisory review was right on the substance and I should have tested get_image() and REFERENCED mode myself before submitting; my tests only covered EMBEDDED export and a JSON round trip. The patch-coverage warning (86.5%, 5 lines) pointed at the same gap.

AI assistance: this retraction was produced with AI assistance (an autonomous coding agent); the reproduction above was executed locally against both this branch and the base commit.

Update: the root-cause fix is open as docling-project/docling-core#916.

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

Labels

ai:ci-safe AI triage found no CI-safety concern (advisory) ai:review-changes AI first review suggests changes bug Something isn't working enhancement New feature or request markdown issue related to markdown backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Markdown: an inline data:image/svg+xml image becomes a picture with no image

2 participants