Skip to content

docs(di): cross-reference upstream enough_mail fix for #232 crash - #237

Merged
guettli merged 2 commits into
mainfrom
issue-235-track-upstream-qp-decode-fix
Jul 16, 2026
Merged

docs(di): cross-reference upstream enough_mail fix for #232 crash#237
guettli merged 2 commits into
mainfrom
issue-235-track-upstream-qp-decode-fix

Conversation

@guettlibot

@guettlibot guettlibot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Open in AgentLoop UI

Closes #235 — Does it make sense to open a PR in the upstream repo?

Summary

Answers #235 ("does it make sense to open a PR in the upstream repo?") with yes, and here it is: Enough-Software/enough_mail#283. That PR guards QuotedPrintableMailCodec.decodeText against the RangeError that took the whole app down in #232.

The only code change in this repo is a 3-line comment update in lib/di.dart next to the defensive try/catch added in #234, so future readers can find the upstream tracking PR and know why the guard stays even after we bump the dependency (it still covers network/DB prefetch failures).

Why no dependency bump

Deliberate — the plan on #235 keeps that as a separate follow-up gated on the upstream PR merging and a new enough_mail release. Once the fix ships to pub.dev, a small follow-up PR here can bump enough_mail: ^2.1.7 → ^2.1.8 (or whatever the release ends up being).

Upstream fix summary

  • Root cause: cleaned.substring(i + 1, i + 3) was called unconditionally on = (line 199 upstream). A trailing = / =X in the outer branch, or an off-by-one in the inner while-loop guard, could take i + 3 past cleaned.length.
  • Fix: preserve the malformed tail verbatim, matching the existing int.tryParse == null branch and RFC 2045 §6.7.
  • Tests: added 4 regression cases under a new malformed truncated =XY escapes are preserved verbatim group; the full upstream suite (420 tests) passes.

Test plan

  • dart format --language-version=3.3 --set-exit-if-changed --output=none lib/di.dart — no formatting changes.
  • CI green on this PR (comment-only change; unit + integration tests should be untouched).

🤖 Generated with Claude Code

The RangeError from QuotedPrintableMailCodec.decodeText that triggered
the crash in #232 is fixed upstream in Enough-Software/enough_mail#283.
Note the reference next to the defensive try/catch so a future reader
knows the underlying bug is being tracked upstream and knows why the
guard stays even after the dependency bumps: it still covers
network/DB failures in the opportunistic prefetch path.

Closes #235
The long-term IMAP/JMAP fuzz test at step 6 (JMAP mutations) has been
flaking on CI with sporadic 'Mismatch after JMAP mutations!' —
EmailFieldMismatch.flagged on a random message, sometimes accompanied by
missingInA/missingInB when the moveEmail hasn't landed on the IMAP side
yet. Both this PR's initial run and the rerun hit it with different
random IDs, and main is intermittently affected too.

The single-round sync at line 369-370 assumes Stalwart bumps the IMAP
HIGHESTMODSEQ on the same tick that the JMAP mutation lands. It doesn't
always; CONDSTORE then skips the mailbox and the assertion fires. Wrap
the pair in a 2-round loop so the second pass picks up the delayed
HIGHESTMODSEQ bump, mirroring the same pattern already used elsewhere.

This is a straight cherry-pick of the test-only change from the
abandoned PR #204 / commit 8f0b834 ("fix(jmap/tests): retry transport
errors and double-sync after JMAP mutations"). That PR was closed
without merging so this fix never landed; the sibling
jmap_client.dart / account_comparison_stalwart_test.dart pieces of
8f0b834 are deliberately not cherry-picked here to keep the scope of
this docs-tracking PR minimal — they can land separately if the
transport-retry problem is still occurring.

Unblocks CI on #237.
@guettlibot

Copy link
Copy Markdown
Collaborator Author

Update: CI failed twice on test/backend/long_term_fuzz_test.dart:376 ("Mismatch after JMAP mutations!" — random EmailFieldMismatch.flagged / missingInA / missingInB depending on the random seed), which is unrelated to the doc-comment change in this PR.

The exact stabilization for that step lives in commit 8f0b834 on the abandoned PR #204. That PR was closed without merging, so the flake never got fixed on main.

Cherry-picked just the test-only portion (the 2-round _syncAllMailboxes loop after JMAP mutations) into this branch as commit 45a434a. Deliberately did not cherry-pick the sibling jmap_client.dart / account_comparison_stalwart_test.dart pieces of that commit — those touch product code and belong in their own PR if the transport-retry problem still occurs. Happy to open a separate issue for it.

@guettli
guettli merged commit 7cebe0e into main Jul 16, 2026
1 check passed
@guettli
guettli deleted the issue-235-track-upstream-qp-decode-fix branch July 16, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does it make sense to open a PR in the upstream repo?

2 participants