Files
Fuchs_Intranet/Fuchs/Docs/Decisions/0018-invoice-editor-images-inlined-into-pdf.md
T
StefanandClaude Sonnet 5 f407341f5f Parallelize invoice image fetch, add telemetry, generalize HtmlConverter
- InvoiceService.ResolveInlineImagesAsync now fetches an invoice's
  distinct whitelisted images concurrently (SemaphoreSlim-capped at 4)
  instead of one at a time. It ran inline on every invoice render,
  including repeated draft-preview renders, so N sequential mfr-portal
  round-trips could otherwise add N x latency to a user-facing request.
- Add fuchs.invoice.inlineimages.fetches (counter, tagged by outcome:
  success/http_error/bad_content_type/oversized/exception) and
  fuchs.invoice.inlineimages.resolve.duration (histogram) to
  FuchsTelemetry, matching the project's observability convention for
  outbound-HTTP operations.
- MigraDocExtensions.HtmlConverter's pre-resolved-image parameter is
  renamed inlineImages -> resolvedImages with generic docs: the
  converter fetches nothing and knows nothing about invoices — which
  images to resolve, and any whitelist/network policy, is entirely the
  caller's concern (InvoiceService, ADR 0018 today; any future caller
  tomorrow).
- Add tests asserting fetches actually overlap (not just faster) and
  that the new counter/histogram are recorded.
- Update ADR 0018 to describe the concurrent fetch, telemetry, and the
  generalized converter parameter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-10 21:17:09 +02:00

5.8 KiB
Raw Blame History

status, date, applyTo, supersededBy
status date applyTo supersededBy
Accepted 2026-09-10
Fuchs/code/MigraDocExtensions.cs
Fuchs/code/FuchsPdf.cs
Fuchs/Services/InvoiceService.cs
Fuchs/Services/IInvoiceService.cs
Fuchs/Services/IPdfService.cs
Fuchs/Services/FuchsPdfService.cs

0018 — Invoice-text images are fetched at render time from a domain whitelist, not embedded as data URIs

Context

The invoice line-item description is TinyMCE-authored HTML (FdsInvoiceData/InvoiceBlock.Items item desc), and can contain <img src="https://portal.mobilefieldreport.com/mfr/..."> picture links pulled from the mfr ERP — the browser resolves and displays them in the editor. The PDF pipeline, however, renders through a hand-rolled HTML→MigraDoc subset converter (MigraDoc.Extensions.Html.HtmlConverter in MigraDocExtensions.cs), which had no img handler at all: every <img>, remote or embedded, was silently dropped, so images the editor showed never appeared on the actual invoice.

MigraDoc/PdfSharp does not fetch remote URLs itself, and the converter runs synchronously deep inside the largely-synchronous FuchsPdf static rendering pipeline. Fetching an arbitrary user-supplied URL server-side is also an SSRF surface — the invoice text is user/editor-controlled, so a naive "download whatever src says" would let it reach internal/private network addresses.

Decision

  • Invoice images are inlined into the PDF only when their src starts with the mfr REST root (InvoiceService.AllowedInlineImagePrefix, e.g. https://portal.mobilefieldreport.com/mfr/) — the only origin the invoice editor's picture links come from. This is derived from the same Fds:MFR_host configuration entry the MFR REST client itself uses (via MFR_RESTClient.MFRClientConfig.RestRoot), not a second hardcoded literal, so it can't drift if the configured mfr host ever changes per environment/tenant. Anything else (any other host, http://, a different path) is left unrendered, exactly as before this feature — no generic remote fetch.
  • The fetch happens once per invoice render, in InvoiceService.ResolveInlineImagesAsync (async, off a named IHttpClientFactory client "InvoiceInlineImages", 10s timeout), before the synchronous MigraDoc render starts. InvoiceService.GenerateInvoicePdf therefore became GenerateInvoicePdfAsync — every caller already ran in an async context. <img> sources are found by parsing the description HTML with HtmlAgilityPack (ResolveInlineImagesAsync) — the same library the HtmlConverter itself parses the identical text with — rather than a regex, so a decoy attribute next to the real src (e.g. data-src="...") can't be picked up in its place. Each fetched image is capped at 5MB, must report an image/* content-type, and the invoice fetches at most 20 distinct images; failures (unreachable, wrong type, oversized) are logged, counted (FuchsTelemetry.InvoiceInlineImageFetches, tagged by outcome), and just leave that one image out — they never fail the whole render. The distinct URLs fetch concurrently (capped at 4 in flight via a SemaphoreSlim, not one-at-a-time) since this runs inline on every render, including repeated draft-preview renders — total resolve time is recorded in FuchsTelemetry.InvoiceInlineImagesResolveDuration.
  • The resolved {src → "base64:..."} map flows down as a plain IReadOnlyDictionary<string,string>? parameter through IPdfService.ApplyInvoice → FuchsPdf.ApplyInvoice → Cell.AddHtml into the HtmlConverter, whose new img handler only emits an image for a src present in that map — it does no fetching itself and never sees the network. The converter itself stays generic (its constructor parameter is named resolvedImages, not invoice-specific) — resolving which images go in the map, and any network/whitelist policy, is entirely InvoiceService's concern, not the converter's. Image sizing mirrors the HTML: the handler reads width/height (attribute or inline style, as TinyMCE's resize handles write them) and converts CSS px → cm, capped to 7×10cm so a malformed/huge size can't blow out the invoice table layout.

Consequences

  • Any future host serving invoice-editor images must be added to the whitelist deliberately (a one-line prefix, or a small allowed-prefixes list if a second origin appears) — this is not meant to grow into a generic "fetch any URL" facility.
  • IInvoiceService.GenerateInvoicePdf → GenerateInvoicePdfAsync, and IInvoiceDraftService.RenderPreview → RenderPreviewAsync (same signature change, same reason) are breaking signature changes; anything implementing/mocking these interfaces needs updating.
  • Image src values not covered by the whitelist — including a stray non-mfr remote URL, or a raw data: URI a user might paste in — still render as nothing, matching the pre-existing (silent) drop behaviour. If embedded data: images need support later, that's a separate, simpler decision (no network fetch involved) and can reuse the same img handler contract.

Alternatives considered

  • Fetch inside the HTML converter itself (sync-over-async HttpClient.GetAsync(...).Result) — rejected: violates the project's no-sync-over-async rule and keeps SSRF-relevant network code inside a generic, reusable HTML converter instead of a single audited call site.
  • Fully embed images as data: base64 URIs at editor save time — rejected: doubles storage size per invoice edit, and the image is already resolved by the mfr portal itself; re-fetching a small number of images once at render time is simpler and keeps the source of truth external.
  • Allow any remote http(s):// host — rejected on SSRF grounds; every invoice image the editor can currently produce is from the mfr portal, so a whitelist costs nothing today.