feat(eml): read RFC 5322 email messages - #164
Conversation
Hand-rolled MIME walker on std + encoding_rs (already a dependency), per the crate's existing style of small purpose-built parsers over heavy general-purpose crates. No new dependencies. - RFC 822 header parse with continuation-line unfolding - multipart traversal, depth-capped, preferring text/plain - quoted-printable and base64 transfer-decoding - RFC 2047 encoded-word headers (both B and Q schemes) - charset via encoding_rs, mirroring csv.rs's decode pattern - Subject as H1, From/To/Cc/Date as a metadata paragraph - html-only bodies error as Unsupported rather than emitting markup Verified against 142 real .eml files: 141 convert, median 4.0ms. The one failure is a calendar invite whose text/plain part is empty and whose content is text/calendar -- reported honestly, not guessed. 10 unit tests, including a regression for encoded-word payloads whose own =XX escapes put a literal "?=" inside the word.
Node/wasm/python enum mirrors and the CLI's format list, so the workspace compiles and CI's clippy -D warnings passes on all crates. Also collapses two nested ifs clippy flagged.
Seven handmade fixtures covering the shapes a real corpus shows: multipart/alternative, base64 + latin-1, RFC 2047 encoded words, nested multipart/mixed with an attachment, folded headers, html-only, and a declared boundary that never appears. Fixtures found four bugs, all fixed here: - a whitespace-only line (one space, not empty) did not break a paragraph, so it rendered as a hard break with nothing after it; 3267 occurrences across a 142-message corpus, now zero - non-ASCII header bytes went through from_utf8_lossy, replacing latin-1 subjects with U+FFFD - a declared boundary that never appears errored instead of recovering as flat text - the CRLF preceding a boundary was kept in the part body Also adds the fixtures_detect_from_content entry: eml carries no signature, so it resolves by extension like csv.
Mirrors csv.rs: convert arbitrary bytes as Eml and assert only that it never panics, hangs, or exhausts memory. Two seeds. A flat plaintext message, and a multipart/alternative whose text/plain part is quoted-printable latin-1 with an RFC 2047 encoded subject, so mutation reaches the boundary walk, the transfer-decoders and the encoded-word reader rather than stopping at the header block. 3,088,842 executions, no crashes, no hangs, no leaks.
3b6fabc to
9eb447f
Compare
There was a problem hiding this comment.
1 issue found and verified against the latest diff
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/eml.rs">
<violation number="1" location="src/formats/eml.rs:344">
P2: Malformed base64 with misplaced or incomplete padding is accepted as valid and can produce corrupted body text. Validate padding and the final quartet before returning `Some`; return `None` for malformed payloads so the existing recovery path runs.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| let mut acc: u32 = 0; | ||
| let mut bits = 0u32; | ||
| for &b in input { | ||
| if b.is_ascii_whitespace() || b == b'=' { |
There was a problem hiding this comment.
P2: Malformed base64 with misplaced or incomplete padding is accepted as valid and can produce corrupted body text. Validate padding and the final quartet before returning Some; return None for malformed payloads so the existing recovery path runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/eml.rs, line 344:
<comment>Malformed base64 with misplaced or incomplete padding is accepted as valid and can produce corrupted body text. Validate padding and the final quartet before returning `Some`; return `None` for malformed payloads so the existing recovery path runs.</comment>
<file context>
@@ -0,0 +1,541 @@
+ let mut acc: u32 = 0;
+ let mut bits = 0u32;
+ for &b in input {
+ if b.is_ascii_whitespace() || b == b'=' {
+ continue;
+ }
</file context>
Three correctness bugs the 142-message corpus never exercised, found in review. Each has a regression test that fails without its fix. - A text/plain part marked `Content-Disposition: attachment` was eligible as the message body, and the text/plain pass runs before the nested multipart pass, so a multipart/mixed of [alternative, text attachment] returned the attachment instead of the message. RFC 2183 disposition now excludes attachments from body selection. - A delimiter was matched with starts_with, so a body line sharing the boundary as a prefix (`--abcExtra`) was read as a delimiter and the rest of the message was dropped with no error. RFC 2046 5.1.1: the line is the boundary exactly, optionally closed with `--`, then only whitespace. - Adjacent encoded-words kept the whitespace between them. RFC 2047 6.2 makes that a separator for the encoding, not text: `=?..?Q?Hello?= =?..?Q?World?=` is "HelloWorld". Whitespace between a word and ordinary text is still preserved. Also names EML alongside CSV wherever the docs called CSV the only signature-less format: the Format/from_bytes/to_markdown_bytes doc comments, the CLI's stdin help, and the README's detection paragraph and parser diagram.
|
Thanks — six of the seven are real and are fixed in c47d71d, each with a regression test that I checked fails against the previous commit rather than passing vacuously. The two that mattered were genuine correctness bugs, and both are cases my 142-message corpus simply never contained:
The three P3 doc points are fixed too — EML is now named alongside CSV in the One I do not think is valid: the base64 padding finding (P2, Re-fuzzed after the changes: 1,898,113 executions, no crashes, no hangs, no leaks. Full suite is 312 passing, clippy clean on the workspace and the wasm target, fmt clean. |
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- `is_delimiter` accepts the trailing whitespace RFC 2046 5.1.1 permits, but the close check still tested the raw line, so `--B-- ` opened a part instead of closing the multipart and the epilogue was parsed. It now reports which delimiter form it matched. - `decode_words` dropped the whitespace before a `=?` token before knowing whether that token decoded, so a valid word followed by a malformed one lost the space between them. The gap is now resolved after the candidate decodes. Regression test for each, both checked to fail against the previous commit.
|
Both valid, both mine, both fixed in af00979.
Worth noting for anyone reading the history: my first regression test for the padded-close case passed against the unfixed code. The fixture put the Re-fuzzed after these changes: 1,652,818 executions, no crashes, no hangs, no leaks. Suite is 314 passing, clippy clean on the workspace and the wasm target, fmt clean. The base64 padding item is still open from my side — I could not construct an input where lenient |
Closes half of #128:
.emlonly..msgis #160, from @vaibhavdabas16 — independent frontends, either order.Shape
Hand-rolled MIME on
std+encoding_rs, no new dependencies. Followscsv.rs:parse(bytes) -> Result<Document, ConvertError>, produces the document model, does not touch the Markdown writer.text/plainencoding_rs, mirroringcsv.rs::decodedetect.rsentry, per the CSV precedent for signature-less text formatsRegistered in the node, wasm and python enums and in
node/cli.jsFORMATS.A body that is
text/htmlwith notext/plainalternative returnsUnsupportedrather than emitting markup as prose. That case wants #52 or #53 landing first; I did not want to guess at HTML here.Validation
142 real
.emlfiles, Outlook/Exchange via Mimecast: 141 convert, median 4.0 ms.The one failure is a calendar invite whose
text/plainpart is 2 bytes and whose content istext/calendar. It returnsUnsupportedrather than emitting the 2 bytes. Correct as far as I can tell, but it is a judgement call — happy to extract it instead if you would rather.Running the whole corpus, rather than spot-checking, is what made this worth trusting. It surfaced an RFC 2047 bug I would not have invented a test for:
decode_one_wordlooked for the terminator?=from the start of the word, but a payload's own=XXescapes can put a literal?=inside it, so the search landed early and parsing bailed, leaving raw encoded words in the subject. Fixed by locating the terminator after both?separators, with a regression test. Post-fix the corpus has zero undecoded escapes and zero U+FFFD.That corpus is private, so the fixtures are synthetic: seven handmade files under
tests/fixtures/eml/, plus thefixtures_detect_from_contententry. Writing them turned up four more bugs I had missed against real mail, the worst being that a whitespace-only line (one space, not empty) did not break a paragraph, so it rendered as a hard break with nothing after it. 3267 occurrences across the corpus, now zero.Fuzzed as well, since the parser walks attacker-shaped bytes: 3,088,842 executions over the two seeds, no crashes, no hangs, no leaks.
Checks, on this branch against v0.2.4:
cargo test --locked— 309 passed, 1 ignored (13 new unit tests)cargo clippy --workspace --all-targets --all-features -- -D warningscargo clippy -p anydoc-wasm --target wasm32-unknown-unknown -- -D warningscargo fmt --all --checkcargo +nightly fuzz run eml— 3,088,842 executions, cleanTwo notes for review
detect.rs. I left.emlon extension-only detection, reading the CSV precedent in the README ("CSV has no such marker, so the extension or an explicit format names it instead") as the rule for signature-less text formats. #160 chose a content-sniff entry for.msg, which is a different case — OLE has a real signature. If you would rather.emlsniffed too, say so and I will add it; I did not want to invent a heuristic thatdetect.rswarns against.Shared helper. If both this and #160 land, each frontend carries its own envelope-and-body rendering. @vaibhavdabas16 offered to factor that out as a follow-up once the second one is in, and that is the right shape — he has the clearer view of both. I am happy to take it instead if he would rather not.
Summary by cubic
Adds RFC 5322/MIME
.emlconversion, closing the.emlhalf of #128. Previously unsupported, messages now convert to Markdown with the Subject as an H1, sender metadata, and atext/plainbody preferred overtext/html.Behavior
Unsupportedfor HTML-only messages.Validation
Written for commit af00979. Summary will update on new commits.