Validate iCalendar syntax before composing MIME - #71
Conversation
The partial scanner accepted malformed property names and parameter values while transports asserted a supported MIME method. Parse every content line using the RFC 5545 grammar before walking components, so construction and transport composition refuse invalid syntax alike. Preserve folding, empty parameter values, and the existing method and TypeError contracts. Check Unicode before unfolding so a fold cannot hide unpaired surrogates in the transmitted content. Add regression coverage and update the unreleased calendar notes. Restore the OpenTelemetry fragment's PR reference already present in the materialized changelog so Sacho synchronization preserves it. Fixes #70 #69 Assisted-by: Codex:gpt-6-astra Assisted-by: Claude Code:claude-fable-5-1
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
+ Coverage 80.49% 80.57% +0.08%
==========================================
Files 32 32
Lines 4880 4886 +6
Branches 1003 1016 +13
==========================================
+ Hits 3928 3937 +9
+ Misses 749 744 -5
- Partials 203 205 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe calendar module now parses every content line with RFC 5545 grammar checks. It validates names, parameters, quoted values, control characters, surrogates, folding, component structure, and supported methods. Documentation and changelogs describe the validation scope. Tests cover parsing, message creation, attachment creation, and JMAP, Mailgun, and SMTP conversion paths. Priority: ➖ Normal — Schedule the RFC 5545 validation change because it broadly hardens calendar parsing across MIME composition and message converters while addressing a medium-severity issue. Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Calendar content is now rejected when malformed before it is composed into messages, while valid folded and parameterized content remains supported. The change has coverage across core message creation and outbound conversion paths and is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The old scanner could derive a MIME method from malformed iCalendar syntax. A single RFC 5545 content line parser now validates every unfolded line before the component walk, keeping name, quoting, and character rules in one place.
Since #69 has not shipped, validation also covers uninterpreted properties such as
SUMMARY;BROKEN:Lunch. Property-specific semantics remain the caller's responsibility; valid empty parameter values and original folding are preserved. The existing changelog fragments describe the final behavior.Deno/Node.js/Bun tests, repository checks, the docs build, and Sacho validation passed. Tests requiring external services were skipped.
Fixes #70.