T-1655 — raw-HTML images with unquoted attribute values (<img src=cat.png>) were never routed through the audited prism-doc://img/?src= scheme handler, so they were dropped. The fix widens the scan to every name=value attribute form and replaces two backtracking regexes with linear scanners. Reviewed against origin/main after four prior review rounds.
src/srcset are now mediated; the value-in-value hazard (alt="src=evil.png") is closed by scanning whole attributes rather than anchoring on src.<img starts, against 11.4 s for the shape the old regex choked on.splitSrcsetCandidates (this file, untouched): 1.98 s at 128 KB. HTMLImageParser.voidElementRegex (upstream, at parse time): 11.4 s at 72 KB. Both pre-existing; both make the new CHANGELOG claim over-broad."; the unquoted value class excludes ", so the widening opens no new injection carrier. The known quote-breakout (T-1942) is byte-for-byte identical to main.WebMediaBehaviourTests/imageActivatedPosts() fails identically on origin/main@a12e6ca; it exercises a native .image block that never reaches this rewriter.specs/bugfixes/<name>/report.md, but still describes the round-1 regex and states the tag-level regex was left unchanged — it was replaced in round 3.Needs fixes
Nothing in the changed lines is wrong. The scanner is correct, the fix is real, the tests are behavioural, SwiftLint is clean, every directly-related suite passes, and the diff still merges cleanly after origin/main moved mid-review. Four prior rounds already settled the grammar, the equivalence proof, and the DoS fix; I re-verified the parts that matter rather than re-litigating them.
What holds it back is one claim this PR adds that I verified to be false. The new CHANGELOG entry tells users that malformed image tags “no longer stall the document” and that finding them “costs time in proportion to the document's length rather than its square”. That is true of the two regexes this PR replaced, and false of the document pipeline. I measured two other quadratic loops still reachable from any opened document — one in this very file, one upstream at parse time — each at a clean 4× per doubling. Both are pre-existing and correctly out of scope for the fix, but neither is filed, and the release note currently reads as though the problem class is closed.
The required actions are the author's calls, not mine: file the two follow-up tickets, then decide whether the CHANGELOG sentence narrows to the rewriter or waits for those fixes. I deliberately made no edits — including to the CHANGELOG — because the correct wording depends on that scope decision, and a reviewer should not both make that call and sign it off.
34760ce Fix T-1655: Mediate unquoted src/srcset in raw HTML images d8d2896 Fix T-1655 review: allow `=` in unquoted attribute values f03fb76 Fix T-1655 review: make the raw-HTML image scan linear working-tree No changes applied by this review Prism shows markdown files, and a markdown file is allowed to contain raw snippets of HTML — including images written as <img src=cat.png>.
Prism never lets a document fetch an image directly. Every image address is first rewritten to point at Prism's own internal handler, which checks that the file is somewhere the document is actually allowed to read from. Think of it as a mailroom: nothing gets delivered straight to the recipient, everything goes through the desk that checks the address first.
The code that rewrote those addresses looked for a quote mark right after the equals sign — it only recognised src="cat.png". But HTML also lets you leave the quotes off entirely: src=cat.png is perfectly valid. Those addresses were never rewritten, so they never got a mailroom stamp, and a later cleaning step threw them away as unrecognised. The image simply vanished from the page.
This change teaches the code to read all three ways HTML lets you write an attribute value: double-quoted, single-quoted, and bare.
Two things were wrong. The visible one: images written the bare way disappeared with no explanation, while the identical image with quotes around it rendered fine. The invisible one: a bare address pointing at a website slipped past the mailroom entirely. It never actually loaded — a separate safety net (the page's content security policy) blocked it — but the mailroom was supposed to be the thing that caught it, and it wasn't catching it.
name=value pairs inside an HTML tag, like src=cat.png or alt="a cat".Three files. prism/Services/WebRendering/HTMLImageSourceRewriter.swift carries the whole fix (+205/-37); prismTests/WebRendering/BlockHTMLEmitterMediaTests.swift adds 12 regression tests including two wall-clock guards; CHANGELOG.md gets one entry.
The rewriter runs on each raw-HTML block before HTMLSanitizer, because the sanitizer's allowlist strips relative URLs but permits the prism-doc: scheme — so rewriting first is what lets a mediated reference survive the clean.
The old code was two regexes. The tag-level one (<(?:img|source)\b[^>]*>) found each start tag; the attribute-level one (\b(src|srcset)\s*=\s*(["'])([\s\S]*?)\2) found the values inside it. The second required a quote after =, which is the bug.
The naive fix — bolt an unquoted alternative onto the same src-anchored pattern — is unsafe, and this is the load-bearing insight of the change. Because that pattern starts at src, it would also fire on a src= sequence sitting inside another attribute's value: <img alt="src=evil.png" src=cat.png> would splice quotes into the middle of alt and reshape the tag. The quoted-only form was accidentally immune because a quote had to follow.
So the scan was inverted: it now finds every name=value attribute, left to right, non-overlapping, and consumes each quoted value as a unit. Attributes that aren't src/srcset are located only to be stepped over and re-emitted byte-for-byte. A src= inside a value is therefore already inside a consumed span and can never be mistaken for an attribute of its own.
Both regexes were then replaced by hand-written linear scanners, because generalising the attribute pattern introduced a quadratic backtracking case (below), and the tag pattern turned out to have had one all along.
A SwiftSoup DOM round-trip is the obvious alternative and is documented at the top of the file as already rejected: the bundled SwiftSoup drops the children of a <figure>/<picture> when re-serializing a mutated void <img> child, so embedded images silently lost their src. Since T-1681 there is a second reason — every SwiftSoup call now serialises behind a shared lock, so a DOM round-trip here would contend with every concurrent sanitize on the off-main emit path.
Hand-writing a tokenizer on an audited security-adjacent path is the real cost. It is mitigated three ways: the tag-start regex keeps ICU's \b rather than reimplementing a Unicode word boundary; the whitespace class is derived from CharacterSet minus U+200B to match ICU's \s exactly; and the scanner's equivalence to the regex it replaced was checked by differential fuzzing in an earlier round.
One deliberate behaviour change falls out: data-src is no longer mediated. The old \b-anchored pattern matched the src tail of data-src and rewrote it. Matching whole attribute names stops that — correctly, since data-src isn't a source Prism honours and the sanitizer strips it either way.
Two independent quadratic blowups were removed, and I re-measured both.
Attribute scan. The generalised pattern ([^\s"'=<>/`]+)\s*=\s*(?:"([\s\S]*?)"|'([\s\S]*?)'|([^\s"'<>`]+)) is quadratic on an unterminated quoted value: the name alternative is retried at every character of the unbroken run, each retry scanning forward for a = that isn't there. 16 KB unterminated base64 measured 8.3 s in round 3.
The replacement is a UTF-16 scan whose correctness rests on two arguments, both stated in the doc comment and both sound. First, the name is the maximal run of name characters, and backtracking to a shorter name can never help — the run only ends at a character that cannot begin \s*=. Second, when an attempt starting at a run fails, every start inside that run fails identically because they share the same continuation, so the scan resumes at nameEnd rather than position+1. That is what makes it linear rather than merely non-backtracking. Termination is unconditional: position strictly increases on all three exits, and nameEnd ≥ position+1 because the loop is only entered when the character at position is a name unit.
Tag extents. <(?:img|source)\b[^>]*> runs [^>]* to end-of-input from every start before failing. tagRanges keeps the regex to the fixed-length prefix and finds the extent by scanning to the first >. Linearity follows from disjointness: the search window for accepted start i is [s_i+len, c_i], cursor becomes c_i+1, and the location ≥ cursor guard forces s_{i+1} > c_i, so accepted windows are disjoint and sum to ≤ n. The break when no > remains is sound because if none follows start p, none follows any later start. Measured 0.004 s on 720 KB versus 11.4 s for the old shape at 72 KB.
Whitespace parity. The claim that CharacterSet.whitespacesAndNewlines minus U+200B equals ICU's \s was swept across all 65,536 BMP code points against NSRegularExpression: zero divergences, and U+200B is the sole mismatch without the subtraction.
Contained. One production caller (BlockHTMLEmitter.rewriteSanitizedImageSources), used only by emitHTML. The nonisolated statics are immutable lets, safe for the off-MainActor emit path T-1681 introduced.
Worth knowing: the srcset half of this file is defence-in-depth only. HTMLSanitizer deliberately doesn't allowlist srcset on img (it can't protocol-check multi-URL syntax) and doesn't add source at all, so on the raw-HTML path every candidate this code mediates is subsequently deleted. That's pre-existing and correct, but it means some of the new tests pin behaviour that cannot affect a rendered document.
Structurally this is now the third hand-rolled HTML tokenizer in prism/Services, alongside DetailsTokenizer and HTMLImageParser's XMLParser wrapper. It is the best of the three — DetailsTokenizer.checkOpenAttribute still answers “is open an attribute here?” with a substring heuristic that returns true for <details class="a open b"> — which makes it the natural home for shared logic later.
The problem class is not closed, and the CHANGELOG says it is. I measured two further quadratics reachable from any opened document. splitSrcsetCandidates, in this same file and untouched by the diff, calls current.trimmingCharacters(...).lowercased().hasPrefix("data:") on every character while inDataURL is false — ΣO(i) with two String allocations per character. Measured 0.0097 / 0.043 / 0.178 / 0.506 / 1.98 s at 8/16/32/64/128 KB. Already reachable on main via quoted srcset; the unquoted form adds a second door to the same room at identical cost. And HTMLImageParser.voidElementRegex ((?i)<(img|source|br)(\s[^>]*?|)(?<!/)\s*>) runs unconditionally at Step 2 of parse on every HTML block up to 1 MB, from MarkdownBlockParser:535 — i.e. before this rewriter ever runs. Measured 0.15 / 0.86 / 3.03 / 11.42 s at 9/18/36/72 KB of "<img " + "a"×40 repeated.
That second one is the sharp edge: the PR's own guard uses String(repeating: "<img", 16_000) with no space after img, which fails \s[^>]*? immediately and sidesteps the upstream regex entirely. Add one space — the more natural way to write a half-finished tag — and the document still hangs, at parse, for the reasons the CHANGELOG says are fixed.
The perf guards don't test what they're named for. A single absolute budget at one input size tests “not catastrophically slow at 16 KB”, not linearity; a reintroduced quadratic that happens to be fast at 16 KB passes. Measuring at N and 2N and asserting a ratio ceiling would pin the complexity class.
Known and out of scope (T-1942). rewriteImageSrc returns data: URIs verbatim, and rewriteTag re-emits hard double-quoted, so a " inside a single-quoted data: value breaks out into a live attribute. Verified byte-for-byte identical to main, and doubly contained (sanitizer drops on*, script-src 'none'). The srcset descriptor path (rewriteSrcset re-joins descriptors verbatim) has the same shape and is worth folding into that ticket.
Undocumented but correct edge. / is deliberately absent from the unquoted-value terminators, so <img src=cat.png/> yields cat.png/ — spec-correct per the HTML5 attribute-value-unquoted state, matching what SwiftSoup sees downstream, but producing a mediated URL that won't resolve. Not a regression (the image was dropped outright before), but the decision is recorded nowhere and no test pins it.
HTMLImageSourceRewriter.swift
Why it matters. This is the fix. It closes the reported bug (unquoted src dropped) and a mediation gap (unquoted remote src reached the DOM un-mediated, stopped only by the CSP). It also silently fixes a pre-existing bug where the old pattern rewrote a src= sequence found inside another attribute's quoted value.
What to look at. HTMLImageSourceRewriter.swift:177-212 (attributes(in:)), :216-230 (attributeValue)
HTMLImageSourceRewriter.swift
Why it matters. A malformed raw-HTML image tag is trivially reachable from any opened document, so a quadratic here is a DoS: 8.3s on a 16KB unterminated quoted value, ~6s on 64KB of unclosed tag starts, both growing 4x per doubling. The tag-extent one was pre-existing on main.
What to look at. HTMLImageSourceRewriter.swift:88-105 (tagRanges), :36-38 (tagStartRegex)
HTMLImageSourceRewriter.swift
Why it matters. Excluding `=` truncated the value at the first one, mediating `image.php?x` and leaving `=1&y=2` dangling as raw text after the re-emitted closing quote. Also broke base64 padding in unquoted data: URIs.
What to look at. HTMLImageSourceRewriter.swift:238-255 (isUnquotedValueUnit), tests at BlockHTMLEmitterMediaTests.swift:214-232
CHANGELOG.md
Why it matters. This is the one thing I'd hold the merge on. The sentence is true of the two regexes replaced here and false of the document pipeline: two other quadratics remain, one in this same file and one upstream at parse time, both measured at a clean 4x per doubling and both reachable from any opened document.
What to look at. CHANGELOG.md, [Unreleased] > Fixed, T-1655 entry, final sentence
BlockHTMLEmitterMediaTests.swift
Why it matters. Coverage is genuinely behavioural — every test drives the public rewrite() or the full BlockHTMLEmitter.emit(), none reaches into the private scanner. The emit-level tests assert the property that actually matters: un-mediated relative URLs do not survive sanitization.
What to look at. BlockHTMLEmitterMediaTests.swift:145-317
Stated at the top of the file: the bundled SwiftSoup corrupts serialization of a <figure>/<picture> containing a mutated void <img> child — re-serializing the parent drops its children, so embedded images silently lost their src. Verified still true; HTMLSanitizer is the only SwiftSoup entry point. HTMLImageParser's XMLParser is also unusable here for an independent reason: unquoted attribute values are illegal XML, so it could never see <img src=cat.png> at all.
A reason the header does not mention, worth adding: since T-1681 every SwiftSoup call runs behind a shared Mutex, so a DOM round-trip here would serialise against every concurrent sanitize on the off-main emit path.
Only the cheap half of the tag match was hand-written; the Unicode word boundary stayed in the regex where the unbounded quantifier — the actual source of the backtracking — was removed. Stated rationale at :31-35: reproducing ICU's boundary by hand would be a fidelity risk on an audited path. This is the right split, and it is why the tag-scan change is easy to argue correct.
Chosen to keep the scan byte-identical to the regex it replaced rather than approximating. I swept all 65,536 BMP code points against NSRegularExpression: zero divergences, and U+200B is the sole mismatch without the subtraction. Claim verified exactly as written.
The old \b(src|srcset) pattern matched the src tail of data-src (the \b sits between - and s), so main rewrote data-src="cat.png" into a mediated reference. Matching whole attribute names stops that. The new behaviour is correct — data-src is not a source Prism honours, and the sanitizer strips it either way — and it is covered by lookalikeAttributeNotMediated. But the CHANGELOG covers only the value case (alt="src=…"), not the attribute-name case, so a changelog reader would not learn of it.
Explicitly held out of scope in the PR body, and unchanged from main. Concretely: <img alt="x>y" src=https://evil.example/track.png> is cut at the inner >, so the scan sees an unterminated quote and bails, leaving the src un-mediated. The real tokenizer does not terminate inside a quoted value and sees a live src, which survives the sanitizer's http/https allowlist. The CSP (img-src prism-doc: data:) blocks the fetch — so it reaches the page but not the network. Pre-existing, fails safe, correctly out of scope.
Safe for the mediation path: rewriteImageSrc routes through URLComponents.queryItems, which I confirmed percent-encodes " → %22, <, > and whitespace, so a mediated value cannot emit a quote. It is not safe for the data: passthrough, which returns the URI verbatim — that is the known quote-breakout filed as T-1942, and I verified it is byte-for-byte identical to main. The unquoted widening adds no new carrier, since " terminates an unquoted value.
The PR body says so explicitly (“The harness blocked writing specs/bugfixes/unquoted-img-src/report.md, so the bugfix report lives here”), so the omission is deliberate rather than an oversight. The problem is that the stand-in was never refreshed after rounds 3 and 4: it still presents the round-1 regex as the fix, and still says “Scope deliberately unchanged: the tag-level regex (<(?:img|source)\b[^>]*>) still ends a tag at the first >” — that regex was replaced in f03fb76. Against ~60 sibling directories under specs/bugfixes/, the two most valuable artifacts of this work (the 8.3s measurement and the differential-fuzzing equivalence argument) now survive only as source comments.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | CHANGELOG.md — T-1655 entry, final sentence | The entry tells users malformed image tags 'no longer stall the document' and that finding them 'costs time in proportion to the document's length rather than its square'. True of the two regexes replaced here; false of the document pipeline. HTMLImageParser.voidElementRegex ((?i)<(img|source|br)(\s[^>]*?|)(?<!/)\s*>) runs unconditionally at Step 2 of HTMLImageParser.parse on every HTML block up to 1 MB, called from MarkdownBlockParser:535 — before this rewriter runs. Measured on input ("<img " + "a"x40) repeated: 0.15s / 0.86s / 3.03s / 11.42s at 9/18/36/72 KB, a clean 4x per doubling. The PR's own guard uses String(repeating: "<img", 16_000) with no space, which fails \s[^>]*? immediately and sidesteps it entirely. | REPORTED, NOT FIXED. Author's call, and deliberately not mine to make: either narrow the sentence to the rewriter specifically, or file the upstream fix and keep the claim. Recommend a Transit ticket for HTMLImageParser.voidElementRegex (and htmlCommentRegex at :74-76, same lazy [\s\S]*? shape). Verified independently by compiling the shipped regex and measuring. |
| major | HTMLImageSourceRewriter.swift:298 — splitSrcsetCandidates | PRE-EXISTING, untouched by this diff (grep of the diff for splitSrcsetCandidates returns 0 lines). Inside the per-character loop, `current.trimmingCharacters(in: .whitespaces).lowercased().hasPrefix("data:")` runs while inDataURL is false — for a non-data candidate that is every character, allocating a trimmed and a lowercased copy of the whole accumulated string each time. Sum O(i) = O(m^2). Measured on the verbatim shipped function: 0.0097 / 0.043 / 0.178 / 0.506 / 1.98 s at 8/16/32/64/128 KB; extrapolates to ~2 min at 1 MB. Already reachable on main via quoted srcset; the unquoted form this PR adds is a second door at identical cost, not a new or worse one. Same file, same off-MainActor emit path, same DoS class the PR set out to eliminate. | REPORTED, NOT FIXED — out of scope for T-1655 and not a regression, but it should be filed before this merges, given the CHANGELOG markets this file as de-quadratic-ised. The fix is small and local: once the trimmed prefix exceeds 5 characters without matching 'data:', it never can, so a latch retires the check after a bounded prefix. Secondary, same cause: :301-303 re-evaluates trimmingCharacters().isEmpty on every whitespace char inside a data URL even after sawWhitespaceInData is true. |
| minor | PR #319 body — the bugfix report of record | The PR body explicitly substitutes for specs/bugfixes/<name>/report.md but was not refreshed after rounds 3-4. It presents the round-1 regex as the fix (that regex no longer exists), and states 'Scope deliberately unchanged: the tag-level regex (<(?:img|source)\b[^>]*>) still ends a tag at the first >' — that regex was replaced in f03fb76. The 8.3s DoS measurement and the differential-fuzzing equivalence argument appear nowhere in the repo. | REPORTED, NOT FIXED. Refresh the PR body to describe the shipped scanner, or land the report under specs/bugfixes/ now that the harness constraint may no longer apply. |
| minor | BlockHTMLEmitterMediaTests.swift:286-317 — perf guards | Both tests are named '...ScaleLinearly' but a single absolute budget at one input size cannot test linearity — it tests 'not catastrophically slow at 16/64 KB'. A reintroduced quadratic that happens to be fast at 16 KB passes. Repo precedent for wall-clock tests is solid (ten *PerformanceTests.swift files, one using an identical 2s budget), but convention puts them in their own file and expresses CI slack as a named multiplier (NotesPerformanceTests:29, InlineNotesExportPerformanceTests:12) rather than folding it into a comment. | REPORTED, NOT FIXED (test-logic change, out of bounds for this review). Suggest measuring at N and 2N and asserting a ratio ceiling (~3x) alongside the absolute budget — that pins the complexity class, and at that ceiling it will not flake. |
| minor | HTMLImageSourceRewriter.swift — missing grammar coverage | Four branches the new scanner introduces have no test: uppercase unquoted name (<img SRC=cat.png>, exercising .lowercased() at :205 on the unquoted path); whitespace around = in the unquoted form (<img src = cat.png>, the skip loops at :190 and :196); both src and srcset unquoted on one tag (the per-tag rationale at :61-63 is tested only for quoted values); and self-closing unquoted (<img src=cat.png/>). The last is the notable one: / is deliberately absent from unquotedValueTerminators, so the value is cat.png/ — spec-correct per the HTML5 attribute-value-unquoted state and matching what SwiftSoup sees downstream, but it yields a mediated URL that will not resolve. Not a regression (the image was dropped outright before), but the decision is recorded nowhere. | REPORTED, NOT FIXED (test-logic change, out of bounds). Four one-line #expects; the self-closing one should be added specifically to pin the decision, with the HTML5-state reference in the comment. |
| minor | HTMLImageSourceRewriter.swift:41,117,121 — mediated-name encoding | The set of mediated attribute names is encoded twice in two different string forms three lines apart: a Set<String> at :41 and an == "srcset" comparison at :121. Adding a third name to the Set would silently route it down the src branch. A `private enum MediatedAttribute: String { case src, srcset }` with an exhaustive switch deletes the Set and makes the compiler enforce the pairing. | REPORTED, NOT FIXED (production change, out of bounds). Cosmetic; no current defect. |
| minor | HTMLImageSourceRewriter.swift:111-130, :202-208 — lost fast path and eager allocation | Two related costs from `found` now holding every attribute rather than only src/srcset. (1) `guard !found.isEmpty else { return tag }` at :112 used to mean 'there is a src/srcset here'; now <img alt="x" width="10"> takes the full substring-and-concatenate path to produce a byte-identical string. (2) attributes(in:) eagerly materialises a .lowercased() name AND a value String per attribute, then rewriteTag discards all but one or two — 30 allocations for a 15-attribute tag where 2 are wanted. Filtering to mediated attributes before the guard fixes both and removes the non-advancing `continue` (which IS correct — I traced both branches, no gap, no duplication — but needs an inline comment to be legible, which is itself a smell). Separately, per-code-unit cost measured ~60ns ASCII / ~118ns non-ASCII from characterAtIndex: message sends plus CharacterSet.contains; an ASCII fast path would cut most of it. | REPORTED, NOT FIXED (production change, out of bounds). Linear either way, so no correctness or complexity impact — but this runs per raw-HTML block on the off-MainActor emit path T-1681 just optimised. |
| nit | HTMLImageSourceRewriter.swift:93, :98, :70, :114 | Four free micro-wins in the new code. :93 tagStartRegex.matches() materialises an NSTextCheckingResult per <img occurrence before any is used, including all those after the break point — O(#starts) heap objects for a scan that may break on the first iteration; firstMatch(in:range:) driven from cursor is a drop-in with the same linear time at O(1) memory. :98 range(of: ">") lacks options: .literal, so it takes the canonical-equivalence path rather than the memchr-class one. :70 and :114 lack output.reserveCapacity(). The `nsHTML as String` re-bridge at :93 was measured free (native Swift storage) but reads as a cost. | REPORTED, NOT FIXED (production change, out of bounds). |
| nit | HTMLImageSourceRewriter.swift:50-56, :133-141, :216 — constant/idiom drift | Four character-class constants in three shapes: equalsSign as a bare unichar via UInt8(ascii:); quotes as a Set<unichar> used positively; nameTerminators and unquotedValueTerminators as Sets used negatively behind is*Unit helpers — and two construction idioms. nameTerminators' doc comment is restated verbatim by isNameUnit's at :232-233. attributeValue returns a tuple while the collection it feeds uses a struct. Measured Set<unichar>+CharacterSet.contains against a raw switch (3M iterations: 0.354s vs 0.329s) — no perf difference, so this is style only. | REPORTED, NOT FIXED. Reads as three sittings, which is exactly what it was. |
| nit | prism/Services — tokenizer proliferation | Third hand-rolled HTML tokenizer in this directory. DetailsTokenizer.findOpenTagEnd (:70-79) is the same scan-to-first-'>' that tagRanges now reimplements, and checkOpenAttribute (:93-124) answers 'is `open` an attribute name here' with a substring heuristic that is demonstrably wrong where the new scanner is right: <details class="a open b"> returns true, so the section renders expanded. Also worth a comment on rewriteSrcset: HTMLSanitizer deliberately does not allowlist srcset on img (:180-186) and does not add `source` (:167-175), so every srcset candidate this file mediates on the raw-HTML path is subsequently deleted — the code is defence-in-depth only, and some new tests pin behaviour that cannot reach a rendered document. | REPORTED, NOT FIXED. Natural follow-up: promote tagRanges/attributes(in:) into a shared HTMLTagScanner and have DetailsTokenizer ask it for `open`, which fixes the class="a open" false positive for free. |
Click to expand.
diff --git a/prism/Services/WebRendering/HTMLImageSourceRewriter.swift b/prism/Services/WebRendering/HTMLImageSourceRewriter.swiftindex 4f56939..fb89e75 100644--- a/prism/Services/WebRendering/HTMLImageSourceRewriter.swift+++ b/prism/Services/WebRendering/HTMLImageSourceRewriter.swift@@ -13,7 +13,7 @@ // an allowed protocol, so rewriting first lets the mediated reference survive the clean. // // Implementation note: this rewrites the `src`/`srcset` ATTRIBUTE VALUES with a targeted-// regex over the raw string, rather than a SwiftSoup DOM round-trip. A DOM round-trip was+// scan over the raw string, rather than a SwiftSoup DOM round-trip. A DOM round-trip was // the obvious approach, but the bundled SwiftSoup corrupts the serialization of a // `<figure>`/`<picture>` that contains a mutated void `<img>` child (re-serializing the // parent drops its children), so embedded images silently lost their src. The regex only@@ -25,21 +25,35 @@ import Foundation nonisolated enum HTMLImageSourceRewriter { - /// Matches a whole `<img>` or `<source>` start tag.- private static let tagRegex: NSRegularExpression? = {- try? NSRegularExpression(pattern: #"<(?:img|source)\b[^>]*>"#, options: [.caseInsensitive])+ /// Matches the START of an `<img>`/`<source>` tag only — the tag's extent is found by a+ /// scan (see `tagRanges(in:)`), not by the regex.+ ///+ /// The obvious `<(?:img|source)\b[^>]*>` is quadratic on a fragment carrying many tag+ /// starts and no `>` at all (`<img<img<img…`): `[^>]*` runs to the end of the input from+ /// every start before failing. Keeping the regex to the fixed-length prefix leaves no+ /// unbounded quantifier to backtrack over, while preserving `\b` exactly — reproducing+ /// ICU's Unicode word boundary by hand would be a fidelity risk on an audited path.+ private static let tagStartRegex: NSRegularExpression? = {+ try? NSRegularExpression(pattern: #"<(?:img|source)\b"#, options: [.caseInsensitive]) }() - /// Matches a single- OR double-quoted `src`/`srcset` attribute. Captures (1) name,- /// (2) the quote character, (3) the value (matched lazily up to the matching quote via- /// the `\2` backreference). Both quote styles must be matched: a single-quoted- /// `src='…'` left unrewritten would survive the sanitizer's allowlist (which permits- /// `prism-doc:` but strips relative URLs) and bypass scheme-handler mediation.- private static let attributeRegex: NSRegularExpression? = {- try? NSRegularExpression(- pattern: #"\b(src|srcset)\s*=\s*(["'])([\s\S]*?)\2"#, options: [.caseInsensitive]- )- }()+ /// The attribute names carrying an image reference that must be mediated.+ private static let mediatedAttributes: Set<String> = ["src", "srcset"]++ /// Whitespace as ICU's `\s` defines it — `CharacterSet.whitespacesAndNewlines` agrees on+ /// every code point except U+200B (ZERO WIDTH SPACE), which ICU does not treat as+ /// whitespace. Subtracting it keeps the scan below byte-identical to the regex it+ /// replaced.+ private static let attributeWhitespace: CharacterSet = CharacterSet.whitespacesAndNewlines+ .subtracting(CharacterSet(charactersIn: "\u{200B}"))++ private static let equalsSign = unichar(UInt8(ascii: "="))+ private static let quotes: Set<unichar> = Set("\"'".utf16)+ /// Characters that end an attribute name, besides whitespace.+ private static let nameTerminators: Set<unichar> = Set("\"'=<>/`".utf16)+ /// Characters that end an unquoted attribute value, besides whitespace. Note `=` is+ /// absent — see `isUnquotedValueUnit`.+ private static let unquotedValueTerminators: Set<unichar> = Set("\"'<>`".utf16) /// Rewrites `src` and every `srcset` candidate in `html` using `rewrite`. The `rewrite` /// closure owns the `data:` passthrough policy (it returns data URIs unchanged).@@ -48,49 +62,203 @@ nonisolated enum HTMLImageSourceRewriter { /// it — so an `<img>` carrying BOTH a `src` and a `srcset` has both mediated (a single /// global attribute scan would have stopped after the first match per tag). static func rewrite(_ html: String, using rewrite: (String) -> String) -> String {- guard !html.isEmpty, let tagRegex, let attributeRegex else { return html }+ guard !html.isEmpty else { return html } let nsHTML = html as NSString- let tagMatches = tagRegex.matches(in: html, range: NSRange(location: 0, length: nsHTML.length))- guard !tagMatches.isEmpty else { return html }+ let tags = tagRanges(in: nsHTML)+ guard !tags.isEmpty else { return html } var output = "" var cursor = 0- for tagMatch in tagMatches {- output += nsHTML.substring(with: NSRange(location: cursor, length: tagMatch.range.location - cursor))- let tag = nsHTML.substring(with: tagMatch.range)- output += rewriteTag(tag, attributeRegex: attributeRegex, using: rewrite)- cursor = tagMatch.range.location + tagMatch.range.length+ for tagRange in tags {+ output += nsHTML.substring(with: NSRange(location: cursor, length: tagRange.location - cursor))+ output += rewriteTag(nsHTML.substring(with: tagRange), using: rewrite)+ cursor = tagRange.location + tagRange.length } output += nsHTML.substring(from: cursor) return output } - /// Rewrites every src/srcset attribute value inside a single start tag.- private static func rewriteTag(- _ tag: String, attributeRegex: NSRegularExpression, using rewrite: (String) -> String- ) -> String {+ /// The ranges of the `<img>`/`<source>` start tags in `nsHTML`, non-overlapping and in+ /// document order — the extents `<(?:img|source)\b[^>]*>` would have matched, found in+ /// linear time.+ ///+ /// A tag runs from its start to the first following `>`. Once no `>` remains, no later+ /// start can match either, so the scan stops rather than re-scanning to the end of the+ /// input once per start (the quadratic case the regex had).+ private static func tagRanges(in nsHTML: NSString) -> [NSRange] {+ guard let tagStartRegex else { return [] }+ let length = nsHTML.length+ var ranges: [NSRange] = []+ var cursor = 0+ let starts = tagStartRegex.matches(in: nsHTML as String, range: NSRange(location: 0, length: length))+ for start in starts {+ // Matches are non-overlapping: a start inside an already-consumed tag is skipped.+ guard start.range.location >= cursor else { continue }+ let searchFrom = start.range.location + start.range.length+ let close = nsHTML.range(of: ">", range: NSRange(location: searchFrom, length: length - searchFrom))+ guard close.location != NSNotFound else { break }+ let end = close.location + close.length+ ranges.append(NSRange(location: start.range.location, length: end - start.range.location))+ cursor = end+ }+ return ranges+ }++ /// Rewrites every src/srcset attribute value inside a single start tag, leaving every+ /// other attribute exactly as written.+ private static func rewriteTag(_ tag: String, using rewrite: (String) -> String) -> String { let nsTag = tag as NSString- let matches = attributeRegex.matches(in: tag, range: NSRange(location: 0, length: nsTag.length))- guard !matches.isEmpty else { return tag }+ let found = attributes(in: nsTag)+ guard !found.isEmpty else { return tag } var output = "" var cursor = 0- for match in matches {- output += nsTag.substring(with: NSRange(location: cursor, length: match.range.location - cursor))- let attribute = nsTag.substring(with: match.range(at: 1)).lowercased()- let value = nsTag.substring(with: match.range(at: 3))- let rewritten = attribute == "srcset"- ? rewriteSrcset(value, using: rewrite)- : rewrite(value)+ for attribute in found {+ guard mediatedAttributes.contains(attribute.name) else {+ continue // Not an image reference: leave it in place, copied with the next span.+ }+ output += nsTag.substring(with: NSRange(location: cursor, length: attribute.range.location - cursor))+ let rewritten = attribute.name == "srcset"+ ? rewriteSrcset(attribute.value, using: rewrite)+ : rewrite(attribute.value) // Re-emit double-quoted regardless of the source quote style: the rewritten // value is a mediated prism-doc:/data: URL (no embedded double quote).- output += "\(attribute)=\"\(rewritten)\""- cursor = match.range.location + match.range.length+ output += "\(attribute.name)=\"\(rewritten)\""+ cursor = attribute.range.location + attribute.range.length } output += nsTag.substring(from: cursor) return output } + /// One `name=value` attribute located inside a start tag.+ private struct Attribute {+ /// The whole `name=value` span, including any quotes around the value.+ let range: NSRange+ /// The attribute name, lowercased.+ let name: String+ /// The value with any surrounding quotes removed.+ let value: String+ }++ /// Finds every `name=value` attribute in a start tag, in the three value forms HTML+ /// allows: double-quoted, single-quoted, and unquoted (T-1655).+ ///+ /// Every quote style must be handled, including unquoted: `<img src=cat.png>` left+ /// unrewritten reaches the sanitizer as a bare relative URL, which the allowlist strips+ /// (the image loses its src and never renders), while a bare `http(s)` one would survive+ /// the clean un-mediated — bypassing scheme-handler mediation with only the CSP's+ /// `img-src prism-doc: data:` left to stop the fetch.+ ///+ /// Scanning *whole attributes* rather than just `src`/`srcset` is what makes the+ /// unquoted form safe to support: because the scan consumes each quoted value as a unit,+ /// a `src=` sequence sitting INSIDE another attribute's value (`alt="src=evil.png"`) is+ /// never mistaken for an attribute of its own. A `src`-anchored match with an unquoted+ /// alternative would fire there and splice quotes into the middle of that value,+ /// reshaping the tag. Non-`src`/`srcset` attributes are located only to be skipped over;+ /// they are re-emitted byte-for-byte.+ ///+ /// **Why this is a scan and not a regex.** The equivalent pattern —+ /// `([^\s"'=<>/`]+)\s*=\s*(?:"([\s\S]*?)"|'([\s\S]*?)'|([^\s"'<>`]+))` — is quadratic on+ /// an unterminated quoted value: the name alternative is retried at every character of+ /// the unbroken run, each retry scanning forward for a `=` that is not there. A single+ /// `<img>` with a ~16 KB base64 `src` and a missing closing quote took **8.3 s**; the+ /// scan below does it in 0.2 ms and stays linear to 1 MB. Since a malformed raw-HTML+ /// image tag is trivially reachable from any opened document, the regex form was a DoS.+ ///+ /// The scan reproduces that pattern's grammar and its leftmost-first, non-overlapping+ /// match semantics exactly (verified by differential fuzzing against it):+ ///+ /// - A name is the *maximal* run of name characters. Backtracking to a shorter name can+ /// never help, because the run only ends at a character that cannot begin `\s*=`.+ /// - When an attempt starting at a run fails, every start *inside* that run fails too —+ /// they share the same continuation — so the scan resumes at the end of the run.+ /// - A quoted value ends at the first matching quote (the regex's lazy `[\s\S]*?`); an+ /// unterminated one fails the whole attempt, exactly as the alternation did.+ private static func attributes(in nsTag: NSString) -> [Attribute] {+ let length = nsTag.length+ var attributes: [Attribute] = []+ var position = 0+ while position < length {+ guard isNameUnit(nsTag.character(at: position)) else {+ position += 1+ continue+ }+ var nameEnd = position+ while nameEnd < length, isNameUnit(nsTag.character(at: nameEnd)) { nameEnd += 1 }++ var index = nameEnd+ while index < length, isWhitespaceUnit(nsTag.character(at: index)) { index += 1 }+ guard index < length, nsTag.character(at: index) == equalsSign else {+ position = nameEnd+ continue+ }+ index += 1+ while index < length, isWhitespaceUnit(nsTag.character(at: index)) { index += 1 }++ guard let value = attributeValue(in: nsTag, from: index) else {+ position = nameEnd+ continue+ }+ attributes.append(+ Attribute(+ range: NSRange(location: position, length: value.end - position),+ name: nsTag.substring(with: NSRange(location: position, length: nameEnd - position)).lowercased(),+ value: value.text+ )+ )+ position = value.end+ }+ return attributes+ }++ /// The attribute value starting at `start`, plus the offset just past it. Returns `nil`+ /// when no value form applies — an unterminated quote, or an empty unquoted run.+ private static func attributeValue(in nsTag: NSString, from start: Int) -> (text: String, end: Int)? {+ let length = nsTag.length+ guard start < length else { return nil }+ let first = nsTag.character(at: start)+ if quotes.contains(first) {+ var index = start + 1+ while index < length, nsTag.character(at: index) != first { index += 1 }+ guard index < length else { return nil }+ return (nsTag.substring(with: NSRange(location: start + 1, length: index - start - 1)), index + 1)+ }+ var index = start+ while index < length, isUnquotedValueUnit(nsTag.character(at: index)) { index += 1 }+ guard index > start else { return nil }+ return (nsTag.substring(with: NSRange(location: start, length: index - start)), index)+ }++ /// An attribute-name character: anything but whitespace, a quote, `=`, `<`, `>`, `/`, or+ /// a backtick.+ private static func isNameUnit(_ unit: unichar) -> Bool {+ !nameTerminators.contains(unit) && !isWhitespaceUnit(unit)+ }++ /// An unquoted-value character. `=` is deliberately allowed: per the HTML5 tokenizer's+ /// unquoted-attribute-value state, only whitespace and `>` terminate the value, so `=`+ /// is an ordinary value character (query strings, `data:` base64 padding). Excluding it+ /// truncated the value at the first `=` and left the remainder dangling as raw text+ /// after the re-emitted closing quote.+ ///+ /// The class is otherwise an intentionally *conservative subset* of that tokenizer+ /// state, not a faithful match: `"`, `'`, `<`, and a backtick also terminate here,+ /// where the real tokenizer would keep them (it flags them parse errors and continues).+ /// That is what keeps the "`src=` inside another attribute's value" protection intact,+ /// together with the quoted forms being tried first and the name class still excluding+ /// `=` — so an unquoted `alt=hello-src=evil.png` is consumed whole as one non-mediated+ /// `alt`, which is what the downstream SwiftSoup parse sees too. Where the subset+ /// truncates early, the remainder is left un-mediated and the sanitizer strips it; it is+ /// never reattached to a `src`, because every rewritten value is re-emitted hard-quoted.+ private static func isUnquotedValueUnit(_ unit: unichar) -> Bool {+ !unquotedValueTerminators.contains(unit) && !isWhitespaceUnit(unit)+ }++ private static func isWhitespaceUnit(_ unit: unichar) -> Bool {+ guard let scalar = Unicode.Scalar(UInt32(unit)) else { return false } // Lone surrogate.+ return attributeWhitespace.contains(scalar)+ }+ /// Rewrites every candidate in a `srcset` value, preserving descriptors (`1x`, `480w`). /// A `srcset` is a comma-separated list of `url [descriptor]` entries — but a `data:` /// URL legitimately contains commas (`data:image/png;base64,AAA=`), so candidates arediff --git a/prismTests/WebRendering/BlockHTMLEmitterMediaTests.swift b/prismTests/WebRendering/BlockHTMLEmitterMediaTests.swiftindex 688ba98..6d1b841 100644--- a/prismTests/WebRendering/BlockHTMLEmitterMediaTests.swift+++ b/prismTests/WebRendering/BlockHTMLEmitterMediaTests.swift@@ -141,6 +141,180 @@ struct RawHTMLImageRewriteTests { // mediated, double-quoted reference). #expect(!rewritten.contains("src='cat.png'")) }++ // MARK: Unquoted attribute values (T-1655)++ @Test("Unquoted src is mediated, not dropped (T-1655)")+ func unquotedSrcMediated() {+ // HTML permits unquoted attribute values. An unmatched `src=cat.png` reaches the+ // sanitizer as a relative URL, which the allowlist strips — so the image silently+ // loses its src and never renders.+ let raw = "<img src=cat.png alt=cat>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ #expect(rewritten.contains("src=\"prism-doc://img/?src=cat.png\""), "mediated: \(rewritten)")+ #expect(!rewritten.contains("<img src=cat.png"), "bare unquoted src gone: \(rewritten)")+ // Neighbouring unquoted attributes are left alone.+ #expect(rewritten.contains("alt=cat"))+ }++ @Test("Unquoted srcset candidates are mediated (T-1655)")+ func unquotedSrcsetMediated() {+ // An unquoted srcset cannot contain spaces, so it is a bare comma-separated list.+ let raw = "<source srcset=hero.webp><img srcset=a.png,b.png>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ let count = rewritten.components(separatedBy: "prism-doc://img/?src=").count - 1+ #expect(count == 3, "3 unquoted srcset candidates mediated: \(rewritten)")+ #expect(!rewritten.contains("srcset=hero.webp"))+ #expect(!rewritten.contains("srcset=a.png"))+ }++ @Test("Unquoted src survives sanitization once mediated (T-1655)")+ func unquotedSrcSurvivesEmit() {+ let doc = BlockHTMLEmitter.emit(blocks: [.html(content: "<img src=cat.png alt=cat>")],+ footnotes: .empty, settings: RenderSettings())+ #expect(doc.html.contains("prism-doc://img/?src="))+ #expect(!doc.html.contains("src=\"cat.png\""))+ }++ @Test("A `src=` inside another attribute's quoted value is not rewritten (T-1655)")+ func srcInsideQuotedValueNotRewritten() {+ // Widening the match to unquoted values must not let a `src=` sequence *inside*+ // another attribute's value be treated as an attribute of its own: doing so would+ // splice quotes into the middle of that value and reshape the tag.+ let raw = "<img alt=\"src=evil.png\" src=cat.png>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ #expect(rewritten.contains("alt=\"src=evil.png\""), "alt untouched: \(rewritten)")+ let count = rewritten.components(separatedBy: "prism-doc://img/?src=").count - 1+ #expect(count == 1, "only the real src mediated: \(rewritten)")+ }++ @Test("Non-src attributes ending in `src` are left un-mediated and stripped (T-1655)")+ func lookalikeAttributeNotMediated() {+ // `data-src` is not an image source the renderer honours; it must not be promoted+ // into a mediated reference, and it must not survive sanitization either.+ let raw = "<img data-src=cat.png alt=x>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ #expect(!rewritten.contains("prism-doc://img/?src="), "data-src not mediated: \(rewritten)")+ let doc = BlockHTMLEmitter.emit(blocks: [.html(content: raw)],+ footnotes: .empty, settings: RenderSettings())+ #expect(!doc.html.contains("cat.png"), "un-mediated relative URL does not survive sanitization")+ }++ @Test("Unquoted remote src is mediated rather than reaching the page raw (T-1655)")+ func unquotedRemoteSrcMediated() {+ // The sanitizer allowlists http/https on img@src, so an unrewritten unquoted+ // remote src would survive the clean un-mediated (only the CSP would stop the+ // fetch). Mediation must happen in the rewriter.+ let raw = "<img src=https://example.com/a.png>"+ let doc = BlockHTMLEmitter.emit(blocks: [.html(content: raw)],+ footnotes: .empty, settings: RenderSettings())+ #expect(doc.html.contains("prism-doc://img/?src="))+ #expect(!doc.html.contains("src=\"https://example.com/a.png\""))+ }++ @Test("Unquoted src carrying a query string is mediated whole, not truncated at `=`")+ func unquotedSrcWithQueryStringNotTruncated() {+ // Per the HTML5 tokenizer, `=` is an ordinary character inside an unquoted value —+ // only whitespace and `>` terminate it. A value grammar that excluded `=` stopped at+ // the first one, mediating `image.php?x` and leaving `=1&y=2` dangling as raw text+ // after the re-emitted closing quote.+ let raw = "<img src=image.php?x=1&y=2 alt=foo>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ // The whole value is mediated: `=` and `&` percent-encode into the src query item,+ // so the mediated URL round-trips to the original path in the scheme handler.+ #expect(rewritten.contains("src=\"prism-doc://img/?src=image.php?x%3D1%26y%3D2\""),+ "full value mediated: \(rewritten)")+ // Nothing of the original value is left loose in the tag.+ #expect(!rewritten.contains("=1&y=2"), "no dangling remainder: \(rewritten)")+ #expect(rewritten.contains("alt=foo"), "following attribute intact: \(rewritten)")+ }++ @Test("Unquoted srcset candidates carrying query strings are mediated whole")+ func unquotedSrcsetWithQueryStringNotTruncated() {+ let raw = "<img srcset=a.php?x=1,b.php?y=2>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ let count = rewritten.components(separatedBy: "prism-doc://img/?src=").count - 1+ #expect(count == 2, "both candidates mediated: \(rewritten)")+ #expect(rewritten.contains("a.php?x%3D1"), "first candidate whole: \(rewritten)")+ #expect(rewritten.contains("b.php?y%3D2"), "second candidate whole: \(rewritten)")+ #expect(!rewritten.contains("=1,b.php"), "no dangling remainder: \(rewritten)")+ }++ @Test("Unquoted data: src keeps its base64 `=` padding")+ func unquotedDataURIPaddingPreserved() {+ // base64 padding routinely ends a data URI in `=`; truncating there produced an+ // undecodable payload plus a stray `=` after the closing quote.+ let raw = "<img src=data:image/png;base64,iVBORw0KGgo= alt=x>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ // data: is passthrough policy (the closure returns it unchanged), but the value must+ // still be captured whole and re-emitted quoted.+ #expect(rewritten.contains("src=\"data:image/png;base64,iVBORw0KGgo=\""),+ "padding preserved: \(rewritten)")+ #expect(rewritten.contains("alt=x"), "following attribute intact: \(rewritten)")+ }++ @Test("An unquoted lookalike value containing `src=` is still not mediated")+ func unquotedSrcInsideAnotherValueNotMediated() {+ // Guards the `=`-permitting value grammar: `alt=hello-src=evil.png` is ONE alt+ // attribute per the HTML5 tokenizer (no whitespace ends the value), so there is no+ // image source to mediate and the tag must be re-emitted byte-for-byte.+ let raw = "<img alt=hello-src=evil.png>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ #expect(!rewritten.contains("prism-doc://img/?src="), "nothing mediated: \(rewritten)")+ #expect(rewritten == raw, "tag unchanged: \(rewritten)")+ // It stays inert text in the alt value — never a src the page would fetch.+ let doc = BlockHTMLEmitter.emit(blocks: [.html(content: raw)],+ footnotes: .empty, settings: RenderSettings())+ #expect(!doc.html.contains("src=\"evil.png\""), "not a live src: \(doc.html)")+ #expect(!doc.html.contains("prism-doc://img/?src=evil.png"), "not mediated: \(doc.html)")+ }++ @Test("An unterminated quoted src is left alone and does not survive sanitization")+ func unterminatedQuotedSrcFailsSafe() {+ // A typo'd tag whose quoted value never closes has no attribute the scan can accept+ // for `src`, so the tag is re-emitted byte-for-byte and the un-mediated relative URL+ // is stripped by the sanitizer. Fails safe — nothing un-mediated reaches the page.+ let raw = "<img src=\"cat.png alt=x>"+ let rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ #expect(rewritten == raw, "tag unchanged: \(rewritten)")+ let doc = BlockHTMLEmitter.emit(blocks: [.html(content: raw)],+ footnotes: .empty, settings: RenderSettings())+ #expect(!doc.html.contains("cat.png"), "un-mediated relative URL does not survive: \(doc.html)")+ }++ @Test("A large unterminated quoted src does not blow up the attribute scan (T-1655)")+ func unterminatedQuotedSrcScalesLinearly() {+ // Regression guard for a quadratic blow-up: the attribute scan used to be a regex+ // whose name alternative was retried at every character of the unbroken run, each+ // retry scanning forward for a `=` that was not there. This exact input took 8.3s+ // (and grew ~4x per doubling — minutes at 50-100 KB), hanging that document's render+ // and pegging a core off a single malformed raw-HTML tag. The scan is linear: ~0.2ms.+ //+ // The budget is deliberately ~4 orders of magnitude above the real cost so a loaded+ // machine cannot make it flake, while still being 4x below the pre-fix time.+ let payload = String(repeating: "AB", count: 8_000) // 16 KB, unbroken, no whitespace.+ let raw = "<img src=\"data:image/png;base64,\(payload) alt=cat>"+ var rewritten = ""+ let elapsed = ContinuousClock().measure {+ rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ }+ #expect(elapsed < .seconds(2), "attribute scan must stay linear, took \(elapsed)")+ #expect(rewritten == raw, "unterminated value left alone")+ }++ @Test("Many unclosed tag starts do not blow up the tag scan (T-1655)")+ func unclosedTagStartsScaleLinearly() {+ // The companion blow-up on the tag scan: `<(?:img|source)\b[^>]*>` ran `[^>]*` to the+ // end of the input from every one of thousands of starts before failing (~6s here,+ // quadratic). Once no `>` remains, no later start can match either, so the scan stops.+ let raw = String(repeating: "<img", count: 16_000) // 64 KB of starts, no `>` at all.+ var rewritten = ""+ let elapsed = ContinuousClock().measure {+ rewritten = HTMLImageSourceRewriter.rewrite(raw, using: BlockHTMLEmitter.rewriteImageSrc)+ }+ #expect(elapsed < .seconds(2), "tag scan must stay linear, took \(elapsed)")+ #expect(rewritten == raw, "no complete tag, nothing rewritten")+ } } // MARK: - Sanitized raw HTML embedding — DELIBERATE CHANGE group (Req 1.8)diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 215a46a..ede9bd4 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Adding a note to text selected after a footnote badge now quotes the text you actually selected (T-1876). In a paragraph like `before[^1] after`, selecting `after` quoted words from near the start of the paragraph instead, and saving stored a wrong source range, which the note then carried into relocation and inline-note export. The rendered document's text-to-source map now keeps its offsets in the block's own coordinates across any number of footnote badges. Footnotes inside list items and table cells are tracked separately. - The saved reading position is no longer corrupted by content the document does not show (T-1851). Hidden content measures as sitting exactly at the top of the window, which beat every genuinely visible block, so the document reported a block the reader could not see as the block being read. That happened with any heading collapsed, and — because the carrier holding a document's YAML frontmatter is hidden the same way — on every document that starts with frontmatter, collapsed or not. Reopening the document, returning from raw source, or recovering from a rendering-process restart then landed on the wrong block or did nothing at all. Content the document does not lay out is now skipped when working out the reading position.+- Images written in raw HTML with unquoted attribute values (`<img src=cat.png>`, `<source srcset=hero.webp>`) now render like their quoted equivalents (T-1655). Only quoted `src`/`srcset` values were routed through Prism's image handler, so an unquoted relative address was stripped as unrecognised and the image disappeared, while an unquoted remote address reached the page without going through the handler at all — the content security policy still blocked that fetch, so nothing was ever loaded off-device. Every quoting style is now mediated, and text that merely looks like `src=` inside another attribute's value is left untouched. Malformed image tags — an address whose opening quote is never closed, or a run of half-written tags — no longer stall the document: finding them now costs time in proportion to the document's length rather than its square, so a 16 KB broken tag takes a fraction of a millisecond instead of eight seconds. - Opening or reloading a large or HTML-heavy document no longer freezes the UI (T-1681). Since the WebKit rendering cutover, the full document HTML — including a SwiftSoup sanitisation pass per raw-HTML block and per inline HTML run — was built synchronously on the main thread on every serve, so a big or markup-dense document blocked the app while it rendered, and re-built the HTML on every reload. The build now runs off the main thread and its result is cached per parse: the UI stays responsive, and reloads (external file change, URL refresh, WebContent-process recovery, the iOS folder-access retry) reuse the cached HTML instead of re-emitting. Very HTML-dense documents can still take a noticeable moment to appear; making that incremental is tracked separately. - Search matches are highlighted in the rendered document again (T-1680). Since the WebKit rendering cutover, searching counted matches and navigated natively but the page never showed a highlight — the native→web search-state feed was never connected. Matches now light up as you type, the current match gets its distinct emphasis and scrolls into view when navigating (including when a result is picked from the iPhone search overlay), footnote badges whose content matches are marked, and dismissing search clears the highlights. - Navigation and display state reach the rendered document again (T-1719). The rendering-engine cutover left core behaviours attached to a retired scroll surface, so they silently stopped running: tapping a table-of-contents entry, a note, or a search result now scrolls the document again; the iPhone bottom toolbar hides when scrolling down and returns when scrolling up; table display-mode choices and expanded/collapsed `<details>` sections now survive a WebKit process recovery, the raw/rendered toggle, and the image-access re-fetch exactly as left (including collapsing a section that was open by default; reloading changed file content still re-seeds them from the document, as designed); and the macOS View-menu scroll commands (Page Up/Down, Top, Bottom) work on the rendered document. A new `WebDocumentStateSynchronizer` owns keeping the page in sync with native state independent of any view being mounted, backed by production-assembly regression suites (`specs/bugfixes/webkit-state-integration/report.md`).WebMediaBehaviourTests/imageActivatedPosts() fails on this branch. I built a detached worktree at origin/main@a12e6ca and ran the same suite: identical failure, same timings. It drives a native MarkdownBlock.image through the live WebKit harness — a path that never touches HTMLImageSourceRewriter, which only runs on .html blocks. Not this branch.
make lint: 0 violations, 0 serious, 495 files.origin/main advanced from a12e6ca to 024f354 mid-review; I re-fetched, and the diff is unchanged and still merges without conflict.Per repo notes, CI here does not build or run tests, so green checks are not validation — everything above was run locally.
Settled in rounds 1-4 and re-checked only where it intersected a new finding: the =-in-unquoted-value fix, quoted-value-first ordering blocking the alt="src=evil.png" hazard, URLComponents mediation percent-encoding = and & losslessly, the data-src behaviour change, scanner/regex differential equivalence, and the 2s guard budgets.
The worktree is clean; git status reports no modifications. Every finding above is reported rather than fixed, including the CHANGELOG sentence — its correct wording depends on whether the two remaining quadratics get filed and fixed or the claim gets narrowed, and that scope call belongs to the author. A reviewer should not both make that call and sign it off.