feat: add MHTML support - #149
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
af6e811 to
049b63a
Compare
|
@cubic-dev-ai review this pull request |
@marcellmanfrin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/mhtml.rs">
<violation number="1" location="src/formats/mhtml.rs:152">
P3: The new `html_resource_base` re-parses the entire root HTML with `Html::parse_document`, and `collect_stylesheets_in_order` parses the same string again immediately after (plus a third parse inside `parse_text_with_context`). Parsing the document twice for resource-base extraction adds a redundant full-DOM-build of a potentially large HTML root. Parse the document once in `parse()` and pass the parsed root (or the extracted base) into both helpers to avoid the duplicate parse.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai Please re-review the two still-open older threads on this PR against the current head "f23fdfdddf6ec2de475c4727bb2455ec8b11a956":
The current head has passed the complete AnyDoc validation gate, and the real 505,883-byte Chrome/Blink MHTML fixture was also converted directly and validated successfully. Please verify whether these two findings still apply to the current head and resolve/update the old threads accordingly if they are now addressed. |
@marcellmanfrin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/mhtml.rs">
<violation number="1" location="src/formats/mhtml.rs:158">
P2: When an MHTML document contains `<base href="">`, relative embedded resources stop resolving against the root `Content-Location`. Treat an empty resolved base as absent so the root location remains the fallback.</violation>
<violation number="2" location="src/formats/mhtml.rs:418">
P2: When an embedded image or stylesheet URL includes a fragment, the lookup retains the fragment even though MIME parts are keyed by the underlying resource URL. Strip fragments for embedded-resource matching while preserving them for external image URLs.</violation>
<violation number="3" location="src/formats/mhtml.rs:424">
P2: When an MHTML root has no usable URL base, protocol-relative images are dropped because `is_absolute_uri` rejects `//host/path`. Preserve network-path references as external images, matching the standalone HTML frontend.</violation>
</file>
<file name="src/formats/html.rs">
<violation number="1" location="src/formats/html.rs:246">
P2: Alternating 257 heading tags triggers `max_xml_depth` before HTML5 parsing can repair them, because `close_implied_before_start` does not close prior `h1`–`h6` elements. Track heading tags as mutually implied-closing elements so malformed HTML is not rejected solely by the preflight stack approximation.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/mhtml.rs">
<violation number="1" location="src/formats/mhtml.rs:246">
P2: For real multipart content types this comparison is always false, so `mime_boundary_from_headers` now returns `None` for every part and the MIME preflight guards are never reached. `ContentType::ctype()` in mail-parser 0.11.8 returns the full type/subtype (e.g. "multipart/related"), not the primary type "multipart"; the primary type requires `get_type()`. Because the only callers are lines 142 and 185 of the preflight, this effectively disables the base64-decoder `max_entry_bytes` reserve check and the `max_mime_depth` nesting check for any multipart MHTML, re-enabling the resource-exhaustion path the guards were added to block. Use `content_type.is_multipart()` instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| // mail-parser only nests multipart media types. A `boundary` parameter on | ||
| // any other media type does not create nested MIME parts, so the preflight | ||
| // must not treat boundary-looking body text there as nested MIME. | ||
| if !content_type.ctype().eq_ignore_ascii_case("multipart") { |
There was a problem hiding this comment.
P2: For real multipart content types this comparison is always false, so mime_boundary_from_headers now returns None for every part and the MIME preflight guards are never reached. ContentType::ctype() in mail-parser 0.11.8 returns the full type/subtype (e.g. "multipart/related"), not the primary type "multipart"; the primary type requires get_type(). Because the only callers are lines 142 and 185 of the preflight, this effectively disables the base64-decoder max_entry_bytes reserve check and the max_mime_depth nesting check for any multipart MHTML, re-enabling the resource-exhaustion path the guards were added to block. Use content_type.is_multipart() instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/mhtml.rs, line 246:
<comment>For real multipart content types this comparison is always false, so `mime_boundary_from_headers` now returns `None` for every part and the MIME preflight guards are never reached. `ContentType::ctype()` in mail-parser 0.11.8 returns the full type/subtype (e.g. "multipart/related"), not the primary type "multipart"; the primary type requires `get_type()`. Because the only callers are lines 142 and 185 of the preflight, this effectively disables the base64-decoder `max_entry_bytes` reserve check and the `max_mime_depth` nesting check for any multipart MHTML, re-enabling the resource-exhaustion path the guards were added to block. Use `content_type.is_multipart()` instead.</comment>
<file context>
@@ -238,12 +238,15 @@ fn mime_header_body_start(bytes: &[u8], start: usize, end: usize) -> Option<(usi
+ // mail-parser only nests multipart media types. A `boundary` parameter on
+ // any other media type does not create nested MIME parts, so the preflight
+ // must not treat boundary-looking body text there as nested MIME.
+ if !content_type.ctype().eq_ignore_ascii_case("multipart") {
+ return None;
+ }
</file context>
| if !content_type.ctype().eq_ignore_ascii_case("multipart") { | |
| if !content_type.is_multipart() { |
|
@cubic-dev-ai ultrareview: focus on the MHTML MIME preflight resource limits (base64 decoder reserve checks, MIME nesting depth, and the multipart media-type guard in mime_boundary_from_headers), HTML/MHTML charset and encoding handling, and format detection precedence. |
@marcellmanfrin Couldn't start the ultrareview: PR author |
|
@cubic-dev-ai Please start a fresh AI code review of the current head |
@marcellmanfrin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 existing issue remains and 2 new issues found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/html.rs">
<violation number="1" location="src/formats/html.rs:76">
P3: When the HTML root is a frameset, this returns Document::default() and silently discards the embedded image `assets` and ordered stylesheets that were already collected and size-limit-checked by the caller. For MHTML this drops all embedded resources even though they were valid. Assign the collected assets (and stylesheets) to the returned document instead of discarding them.</violation>
</file>
<file name="src/lib.rs">
<violation number="1" location="src/lib.rs:50">
P2: Adding `Html` and `Mhtml` to this public enum breaks downstream users with exhaustive `match` expressions. Ship this behind a major-version release, or establish a non-exhaustive enum API before adding further variants.</violation>
</file>
Requires human review: Auto-approval blocked by 1 unresolved issue from a previous review of this commit.
Fix all with cubic | Re-trigger cubic
| /// EPUB 2 and 3 (`.epub`). | ||
| Epub, | ||
| /// Standalone HTML5 (`.html`, `.htm`). JavaScript is not executed. | ||
| Html, |
There was a problem hiding this comment.
P2: Adding Html and Mhtml to this public enum breaks downstream users with exhaustive match expressions. Ship this behind a major-version release, or establish a non-exhaustive enum API before adding further variants.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 50:
<comment>Adding `Html` and `Mhtml` to this public enum breaks downstream users with exhaustive `match` expressions. Ship this behind a major-version release, or establish a non-exhaustive enum API before adding further variants.</comment>
<file context>
@@ -46,6 +46,11 @@ pub enum Format {
/// EPUB 2 and 3 (`.epub`).
Epub,
+ /// Standalone HTML5 (`.html`, `.htm`). JavaScript is not executed.
+ Html,
+ /// MIME HTML aggregate (`.mhtml`, `.mht`). Resources are resolved only
+ /// from MIME parts embedded in the input; no network requests are made.
</file context>
| let body = match root.descendent_elements().find(|e| e.value().name() == "body") { | ||
| Some(body) => body, | ||
| None if root.descendent_elements().any(|e| e.value().name() == "frameset") => { | ||
| return Ok(Document::default()); |
There was a problem hiding this comment.
P3: When the HTML root is a frameset, this returns Document::default() and silently discards the embedded image assets and ordered stylesheets that were already collected and size-limit-checked by the caller. For MHTML this drops all embedded resources even though they were valid. Assign the collected assets (and stylesheets) to the returned document instead of discarding them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/html.rs, line 76:
<comment>When the HTML root is a frameset, this returns Document::default() and silently discards the embedded image `assets` and ordered stylesheets that were already collected and size-limit-checked by the caller. For MHTML this drops all embedded resources even though they were valid. Assign the collected assets (and stylesheets) to the returned document instead of discarding them.</comment>
<file context>
@@ -0,0 +1,543 @@
+ let body = match root.descendent_elements().find(|e| e.value().name() == "body") {
+ Some(body) => body,
+ None if root.descendent_elements().any(|e| e.value().name() == "frameset") => {
+ return Ok(Document::default());
+ }
+ None => {
</file context>
Cubic finding (PR 149, src/formats/html.rs): the frameset branch of
document_from_parsed_html returned Document::default(), silently
discarding the assets that had already been collected and checked
against the size limits. The loss is observable for MHTML archives
whose HTML root is a frameset document: embedded image parts vanished
from the resulting Document.
Return Document { assets, ..Document::default() } in the frameset
branch. Regression: an MHTML archive with a frameset root and one
image part retains the asset while still producing no blocks.
Cubic finding (PR 149, src/formats/mhtml.rs): MhtmlCtx::link_target and MhtmlCtx::anchor_id duplicated StandaloneCtx verbatim. Move both implementations into HtmlCtx as default trait methods so only the image_source hook (which genuinely differs per frontend) and frontends with scoped link semantics (EPUB chapters) carry their own implementations. Behavior is unchanged for every frontend.
|
Re: review thread on @cubic-dev-ai Please re-check this finding against the current head |
|
Re: review thread on Addressed by #154, which marks |
|
Re: review thread on @cubic-dev-ai Please re-check this finding against the current head
If the finding is addressed by this evidence, please resolve the thread. |
@marcellmanfrin I have started the AI code review. It will take a few minutes to complete. |
@marcellmanfrin You're right, and this finding looks wrong — I'm retracting it. Verified against mail-parser 0.11.8's actual API:
The code at the current head is doing the right thing, and the suggested alternative was wrong. Resolving the thread. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/mhtml.rs">
<violation number="1" location="src/formats/mhtml.rs:139">
P2: A base64 part followed by large-but-valid later parts can be spuriously rejected. The preflight computes the decoder reserve from the entire remaining message (`bytes.len() - body_start`), not just the current part's encoded region, then rejects when that exceeds MAX_ENTRY_BYTES. Because MAX_TOTAL_BYTES (512MB) is far larger and non-image/non-HTML parts are never otherwise size-checked, a valid archive (e.g. two ~70MB images) fails RESOURCE_LIMIT solely because of data after the first base64 part. Bound the reserve to the part's own body (up to its terminating boundary) rather than the whole remaining message.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| let headers = &bytes[..body_start]; | ||
|
|
||
| if headers_use_base64(headers) { | ||
| enforce_base64_parser_reserve(bytes.len().saturating_sub(body_start), max_entry_bytes)?; |
There was a problem hiding this comment.
P2: A base64 part followed by large-but-valid later parts can be spuriously rejected. The preflight computes the decoder reserve from the entire remaining message (bytes.len() - body_start), not just the current part's encoded region, then rejects when that exceeds MAX_ENTRY_BYTES. Because MAX_TOTAL_BYTES (512MB) is far larger and non-image/non-HTML parts are never otherwise size-checked, a valid archive (e.g. two ~70MB images) fails RESOURCE_LIMIT solely because of data after the first base64 part. Bound the reserve to the part's own body (up to its terminating boundary) rather than the whole remaining message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/mhtml.rs, line 139:
<comment>A base64 part followed by large-but-valid later parts can be spuriously rejected. The preflight computes the decoder reserve from the entire remaining message (`bytes.len() - body_start`), not just the current part's encoded region, then rejects when that exceeds MAX_ENTRY_BYTES. Because MAX_TOTAL_BYTES (512MB) is far larger and non-image/non-HTML parts are never otherwise size-checked, a valid archive (e.g. two ~70MB images) fails RESOURCE_LIMIT solely because of data after the first base64 part. Bound the reserve to the part's own body (up to its terminating boundary) rather than the whole remaining message.</comment>
<file context>
@@ -0,0 +1,641 @@
+ let headers = &bytes[..body_start];
+
+ if headers_use_base64(headers) {
+ enforce_base64_parser_reserve(bytes.len().saturating_sub(body_start), max_entry_bytes)?;
+ }
+
</file context>
Cubic findings (PR 149 review run ed78d83a, shared HTML frontend):
1. The complexity preflight did not model HTML5's implicit <p> closure
on block-level start tags, so repeated <p><div> pairs accumulated
phantom nesting depth: documents html5ever repairs into shallow trees
were rejected and the depth accounting diverged from the DOM the
limits are meant to model. Close an innermost open <p> when a
block-level start tag arrives; deeper arrangements remain
over-counted, keeping the preflight fail-closed.
2. href="#" produced LinkTarget::Anchor(""), which the Markdown
renderer cannot resolve, so the link was dropped to plain text.
Preserve the empty fragment as a relative "#" URL instead.
Regressions: 200 <p><div> pairs now convert (depth 200 < limit), 300
pairs are still rejected before DOM construction, and a bare-hash link
renders as [top](#).
Cubic findings (PR 149 review run ed78d83a): 1. (P1) mail-parser eagerly decodes part bodies while parsing, and the preflight did not bound quoted-printable parts at all, so an oversized QP part materialized the very allocation MAX_ENTRY_BYTES should reject. Bound the encoded region of QP parts in the preflight (quoted-printable never expands, so the encoded size upper-bounds the decoded allocation) and decode image parts through the same guarded path as the HTML root (transfer_decoded_part_bytes) so the limits are enforced before decoded bytes are used, independent of parser internals. 2. When the HTML part carries no absolute Content-Location and <base href> is relative, embedded relative images and stylesheets were never joined against that base because resolve_resource_reference required a scheme. Join base and reference as archive paths, mirroring the resolution applied when the resource index is built. Regressions: an over-limit QP part is rejected by the preflight (and a small one passes); a relative base resolves an embedded image through Content-Location.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Cubic (PR 149, src/formats/html.rs:308) claimed dialog and summary do not implicitly close an open <p>; html5ever's in-body mode (tree_builder/rules.rs) proves they do. Keep both and complete the preflight list to the parser's full close_p_element_in_button_scope set: center, dir, listing, plaintext, and search were missing. table is retained: html5ever closes <p> for <table> outside quirks mode, and the preflight assumes standards mode.
Cubic (PR 149, tests/mhtml.rs): with symmetric relative references on both sides, the original fixture matched even without base resolution. Use a ./ dot segment in the image reference: it only collapses to the part key through the relative base join plus path normalization. Verified red without the resolve_resource_reference change and green with it.
|
Re: review thread on @cubic-dev-ai Please resolve this thread: the finding is a false positive for mail-parser 0.11.8.
|
|
Re: review thread on Addressed by #154, which marks |
|
Re: review thread on This conservatism is deliberate. mail-parser 0.11.x reserves from |
|
Re: review thread on Fixed in this branch by commit da61872: the frameset branch of |
@marcellmanfrin I have started the AI code review. It will take a few minutes to complete. |
@marcellmanfrin Confirmed — this is the same To recap so it stays on record:
The code at the current head is doing the right thing. Thread resolved. If you want this documented so it doesn't get re-flagged, the fix note is in the learnings store. Otherwise, happy to move on. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
|
@cubic-dev-ai ultrareview: focus on the MHTML MIME preflight resource limits (base64 reserve, quoted-printable bounds, nesting depth, media-type guard), resource resolution (Content-ID/Content-Location/base href), and the HTML/MHTML interaction in the shared frontend. |
@marcellmanfrin Couldn't start the ultrareview: PR author |
There was a problem hiding this comment.
1 existing issue remains and 3 new issues found across 41 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/formats/html.rs">
<violation number="1" location="src/formats/html.rs:37">
P2: A maximum-size malformed or legacy-encoded HTML input can expand several-fold during `decode_html` before the resource limit is applied, causing avoidable memory exhaustion. Cap the decoded text allocation/output or reject inputs whose decoded representation exceeds the configured entry budget before parsing.</violation>
<violation number="2" location="src/formats/html.rs:278">
P2: When malformed HTML repeats `<button>` start tags without end tags, the HTML5 tree builder implicitly closes each previous button, but this preflight counts them as nested and returns `max_xml_depth` after 256 tags. Add `button` to the implied-close cases so browser-repairable documents are not rejected before DOM construction.</violation>
</file>
<file name="src/formats/mhtml.rs">
<violation number="1" location="src/formats/mhtml.rs:161">
P2: A boundary-looking string inside a MIME part can make the preflight scanner parse ordinary body text as a new part. Restrict boundary matches to delimiter lines (line start plus valid `CRLF`/closing suffix), matching the MIME parser’s boundary semantics.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Fix all with cubic | Re-trigger cubic
| open.pop(); | ||
| } | ||
|
|
||
| let implied = match name { |
There was a problem hiding this comment.
P2: When malformed HTML repeats <button> start tags without end tags, the HTML5 tree builder implicitly closes each previous button, but this preflight counts them as nested and returns max_xml_depth after 256 tags. Add button to the implied-close cases so browser-repairable documents are not rejected before DOM construction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/html.rs, line 278:
<comment>When malformed HTML repeats `<button>` start tags without end tags, the HTML5 tree builder implicitly closes each previous button, but this preflight counts them as nested and returns `max_xml_depth` after 256 tags. Add `button` to the implied-close cases so browser-repairable documents are not rejected before DOM construction.</comment>
<file context>
@@ -0,0 +1,582 @@
+ open.pop();
+ }
+
+ let implied = match name {
+ "li" => &["li"][..],
+ "p" => &["p"][..],
</file context>
| marker.extend_from_slice(boundary); | ||
|
|
||
| while offset < end { | ||
| let Some(marker_offset) = find_bytes(&bytes[offset..end], &marker) else { |
There was a problem hiding this comment.
P2: A boundary-looking string inside a MIME part can make the preflight scanner parse ordinary body text as a new part. Restrict boundary matches to delimiter lines (line start plus valid CRLF/closing suffix), matching the MIME parser’s boundary semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/mhtml.rs, line 161:
<comment>A boundary-looking string inside a MIME part can make the preflight scanner parse ordinary body text as a new part. Restrict boundary matches to delimiter lines (line start plus valid `CRLF`/closing suffix), matching the MIME parser’s boundary semantics.</comment>
<file context>
@@ -0,0 +1,688 @@
+ marker.extend_from_slice(boundary);
+
+ while offset < end {
+ let Some(marker_offset) = find_bytes(&bytes[offset..end], &marker) else {
+ break;
+ };
</file context>
| }); | ||
| } | ||
|
|
||
| let text = decode_html(bytes); |
There was a problem hiding this comment.
P2: A maximum-size malformed or legacy-encoded HTML input can expand several-fold during decode_html before the resource limit is applied, causing avoidable memory exhaustion. Cap the decoded text allocation/output or reject inputs whose decoded representation exceeds the configured entry budget before parsing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/html.rs, line 37:
<comment>A maximum-size malformed or legacy-encoded HTML input can expand several-fold during `decode_html` before the resource limit is applied, causing avoidable memory exhaustion. Cap the decoded text allocation/output or reject inputs whose decoded representation exceeds the configured entry budget before parsing.</comment>
<file context>
@@ -0,0 +1,582 @@
+ });
+ }
+
+ let text = decode_html(bytes);
+ parse_text_with_context(&text, None, &StandaloneCtx, Vec::new())
+}
</file context>
1 participant
Scope
Adds MHTML/MHT parsing and conversion, completing the MHTML portion of #52.
This head is explicitly reconciled with standalone HTML PR #147 head
c6b7bb18608d4bde426a71d9a5300070d8f653fc.The reconciled functional commit
32c647468193fbff6e4d21b07ab8395295e79e75is a direct child of that #147 head. Its tree is48566f9d8eee0e9836c7b48a05ea05c827a1a1ec, exactly the same tree as the previously validated/reviewed #149 head17fe665d8f921bb1a881bb33a5ec4b87ceb43dd5. The reconciliation therefore changes ancestry only; it does not change the MHTML/HTML code content.Summary
Format::Mhtmlwith.mhtml/.mhtsupportmultipart/relatedarchives without classifying ordinary multipart email as MHTMLstartContent-ID,Content-Location, relative URLs,<base href>, and case-insensitivecid:references#147 reconciliation and realistic corpus
The branches were reconciled semantically rather than by blindly taking either side's HTML implementation. The current #149 commit is now directly based on the validated #147 head, while retaining the exact content tree of the previous #149 head. This proves the latest #147 anchor-recovery change was already present semantically in #149 and no MHTML-specific behavior was lost during the ancestry rewrite.
The #149 tree includes the shared HTML list fixes from #147, its controlled/LibreOffice HTML corpus, HTML list/corpus regressions and snapshots, while preserving the MHTML-specific MIME, charset, resource-resolution, CSS, and asset behavior.
tests/mhtml_corpus.rsreuses the #147 fixtures. It validates byte-for-byte standalone HTML vs MHTML invariance where MIME should not change semantics, base64 UTF-8 roots, realistic linked CSS and embedded images, deliberate relative-image resolution through MHTMLContent-Location, and conservative handling of ordinary MIME messages.Review follow-up fixes
The current head includes the earlier review fixes plus the latest Cubic follow-up:
Content-IDhas the same bare text; Content-ID resources requirecid:semanticsContent-Locationare removed from the resource-index key, matching fragment-stripped lookup behavior<a>start tags without accumulating false nesting depthtests/html.rs, MHTML resource-resolution cases intests/mhtml.rs;tests/review_followup.rswas removedThe earlier resource-safety fix remains: decoded MHTML HTML roots run
preflight_html_complexitybefore the MHTML DOM is materialized.The previous Cubic review of the identical content tree completed with 0 new issues in the latest delta. Because the #147 reconciliation rewrote branch ancestry, Cubic intentionally skipped its automatic review of the rewritten head. A manual re-review was triggered for exact head
32c647468193fbff6e4d21b07ab8395295e79e75(check99186037778); it has reviewed all 4 files and is currently completing its final review processing.A separate P3 performance/maintainability thread about multiple HTML passes remains intentionally unchanged after audit: the bounded charset sniff, streaming pre-DOM complexity preflight, and actual HTML5 tree construction serve distinct purposes. Removing the full-document preflight would weaken pre-DOM resource enforcement; combining it with HTML5 tree construction would require a substantially larger parser/tree-sink refactor.
Fresh GitHub Actions validation
Validated functional SHA:
32c647468193fbff6e4d21b07ab8395295e79e75.Parent HTML SHA:
c6b7bb18608d4bde426a71d9a5300070d8f653fc(current validated head of #147).Temporary CI-only branch:
verify/mhtml-reconciled-c6b7bb1-full.Temporary CI-only commit used to launch the authoritative gate:
156d6d858e1d5349e8716e278b329830187a071f. This commit is not part of the PR code; its sole parent is the validated functional SHA above, and the workflow explicitly checks out that SHA detached before testing.Authoritative verification: run
33284379175, job99184901026— success.html,html_corpus,html_list,mhtml,mhtml_charset,mhtml_corpus)cargo fmt --all -- --check: passed-D warnings: passedcargo test --locked: passednpm ci, build, tests, and generated binding determinism: passedsite-packagesmodule, full unittest suite: passedThe current PR head is exactly the validated functional SHA above.
Real MHTML validation
The authoritative run reconstructed and SHA-256-verified the original 505,883-byte SEI/TRE-PB Chrome/Blink MHTML fixture and converted it directly.
387c6f2d7223da224a8f55962b97eac947734c97fc84888e1e5619e93745837cmhtml�,Ã, orÂmojibake markersf6f75cec7bc06afe038bd89cd7967e5af358ec3562553f5bfc5c47c0299eb80c