Fix: Calendar event HTML description rendered as plain text#4558
Merged
hoangdat merged 5 commits intoMay 28, 2026
Merged
Conversation
Signed-off-by: dab246 <tdvu@linagora.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds AutolinkTextNodeTransformer (DOM-level URL/email linkification), refactors SanitizeAutolinkFilter to a simpler linkify-and-escape flow, introduces TransformConfiguration.forCalendarEvent(), updates SingleEmailController to use that factory, and updates/expands tests across transformer, local transform, repository, and controller layers to match new sanitization and passthrough behavior. Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
hoangdat
reviewed
May 25, 2026
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4558. |
Member
|
@coderabbitai review this please |
Contributor
|
✅ Actions performedReview triggered.
|
Contributor
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
test/features/email/data/repository/calendar_event_repository_impl_test.dart (1)
153-161: ⚡ Quick winAssert the forwarded
TransformConfigurationexplicitly.Current delegation checks use
anyfor the second argument, so a wrong config could still pass. Verify the exacttransformConfigurationobject in these assertions.Suggested test tightening
Future<void> expectDelegated({ required String raw, required String sanitized, }) async { when(htmlDatasource.transformHtmlEmailContent(any, any)) .thenAnswer((_) async => sanitized); final result = await transformSingle(CalendarEvent(description: raw)); - verify(htmlDatasource.transformHtmlEmailContent(raw, any)).called(1); + verify(htmlDatasource.transformHtmlEmailContent(raw, transformConfiguration)).called(1); expect(result.first.calendarEventList.first.description, sanitized); } ... test('should call htmlDataSource with the raw description', () async { @@ - verify(htmlDatasource.transformHtmlEmailContent(rawDescription, any)).called(1); + verify(htmlDatasource.transformHtmlEmailContent(rawDescription, transformConfiguration)).called(1); });Also applies to: 195-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/email/data/repository/calendar_event_repository_impl_test.dart` around lines 153 - 161, The test helper expectDelegated currently verifies htmlDatasource.transformHtmlEmailContent(raw, any) which allows any TransformConfiguration; change the assertions to verify the exact TransformConfiguration being forwarded by constructing or capturing the expected TransformConfiguration and using that in verify (e.g., verify(htmlDatasource.transformHtmlEmailContent(raw, expectedConfig)) or use a capture/argThat predicate) so the transformSingle(CalendarEvent...) call is asserted to pass the precise TransformConfiguration; apply the same tightening for the other occurrences around lines 195-206 where htmlDatasource.transformHtmlEmailContent is verified.core/lib/presentation/utils/html_transformer/text/sanitize_autolink_unescape_html_transformer.dart (2)
9-12: ⚡ Quick winAdd method documentation to clarify parameter usage.
The
htmlEscapeparameter is passed toSanitizeAutolinkFilterbut text content is not escaped due toescapeHtml: false. Without documentation, it's unclear why the parameter is passed if escaping is disabled. Add a comment or method doc clarifying thathtmlEscapeis still used internally by the filter for building link tags, just not for escaping text content.📝 Suggested documentation
`@override` + /// Processes text by sanitizing autolinks without HTML-escaping the content. + /// + /// The [htmlEscape] parameter is passed to [SanitizeAutolinkFilter] for + /// building link tags, but text content is preserved as-is (not escaped) + /// due to `escapeHtml: false`. String process(String text, HtmlEscape htmlEscape) { return SanitizeAutolinkFilter(htmlEscape, escapeHtml: false).process(text); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/lib/presentation/utils/html_transformer/text/sanitize_autolink_unescape_html_transformer.dart` around lines 9 - 12, Add a concise method/doc comment to the process(String text, HtmlEscape htmlEscape) method explaining that although escapeHtml is set to false when constructing SanitizeAutolinkFilter, the htmlEscape parameter is still required and passed through because the SanitizeAutolinkFilter uses it internally when building link tags (not for escaping plain text content); reference the process method name, the htmlEscape parameter, SanitizeAutolinkFilter, and the escapeHtml flag in the comment so readers know why htmlEscape is provided despite escaping being disabled.
6-7: ⚡ Quick winAdd class-level documentation.
The class lacks documentation explaining its purpose, security implications, and how it differs from similar transformers. This is important context for a security-sensitive component that deliberately disables HTML escaping.
Consider adding DartDoc:
/// A text transformer that sanitizes autolinks while preserving HTML markup. /// /// This transformer delegates to [SanitizeAutolinkFilter] with `escapeHtml: false`, /// which means HTML tags in the input text are preserved as-is rather than being /// HTML-escaped. This allows rich-text content (like calendar event descriptions) /// to render as formatted HTML. /// /// **Security**: This transformer does NOT provide XSS protection. It must be /// followed by [StandardizeHtmlSanitizingTransformers] or equivalent to remove /// dangerous HTML elements, attributes, and JavaScript. class SanitizeAutolinkUnescapeHtmlTransformer extends TextTransformer {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/lib/presentation/utils/html_transformer/text/sanitize_autolink_unescape_html_transformer.dart` around lines 6 - 7, Add a DartDoc comment to the SanitizeAutolinkUnescapeHtmlTransformer class that explains its purpose (sanitizes autolinks while preserving HTML), notes it delegates to SanitizeAutolinkFilter with escapeHtml: false, and clearly states the security implications (it does NOT provide XSS protection and must be followed by StandardizeHtmlSanitizingTransformers or equivalent to remove dangerous tags/attributes/JS); reference the class name SanitizeAutolinkUnescapeHtmlTransformer and the related SanitizeAutolinkFilter and StandardizeHtmlSanitizingTransformers in the comment so future maintainers know usage and safety requirements.lib/features/email/presentation/controller/single_email_controller.dart (1)
1257-1267: ⚡ Quick winConsider extracting the transformer configuration to a reusable method.
A previous review comment suggested extracting this transformer list to a specific method that can be reused in tests. This would improve maintainability and ensure consistency between production and test configurations.
♻️ Suggested refactoring
Add a static factory method to create the configuration:
static TransformConfiguration createCalendarEventTransformConfiguration() { return TransformConfiguration.create( customTextTransformers: const [ SanitizeAutolinkUnescapeHtmlTransformer(), StandardizeHtmlSanitizingTransformers(), NewLineTransformer(), ], customDomTransformers: [ SanitizeHyperLinkTagInHtmlTransformer(), ] ); }Then use it here:
void _parseCalendarEventAction({ required AccountId accountId, required Set<Id> blobIds, }) { log("SingleEmailController::_parseCalendarEventAction:blobIds: $blobIds"); consumeState(_parseCalendarEventInteractor!.execute( accountId, blobIds, - TransformConfiguration.create( - customTextTransformers: const [ - SanitizeAutolinkUnescapeHtmlTransformer(), - StandardizeHtmlSanitizingTransformers(), - NewLineTransformer(), - ], - customDomTransformers: [ - SanitizeHyperLinkTagInHtmlTransformer(), - ] - ) + createCalendarEventTransformConfiguration() )); }And reuse the same method in test files for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/email/presentation/controller/single_email_controller.dart` around lines 1257 - 1267, Extract the inline TransformConfiguration.create call into a reusable static factory method (e.g. createCalendarEventTransformConfiguration) that returns TransformConfiguration.create with the same customTextTransformers (SanitizeAutolinkUnescapeHtmlTransformer, StandardizeHtmlSanitizingTransformers, NewLineTransformer) and customDomTransformers (SanitizeHyperLinkTagInHtmlTransformer); replace the inline call with a call to that new static method in single_email_controller and import/use the same factory in tests so production and test transformer lists remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@core/lib/presentation/utils/html_transformer/text/sanitize_autolink_unescape_html_transformer.dart`:
- Around line 9-12: Add a concise method/doc comment to the process(String text,
HtmlEscape htmlEscape) method explaining that although escapeHtml is set to
false when constructing SanitizeAutolinkFilter, the htmlEscape parameter is
still required and passed through because the SanitizeAutolinkFilter uses it
internally when building link tags (not for escaping plain text content);
reference the process method name, the htmlEscape parameter,
SanitizeAutolinkFilter, and the escapeHtml flag in the comment so readers know
why htmlEscape is provided despite escaping being disabled.
- Around line 6-7: Add a DartDoc comment to the
SanitizeAutolinkUnescapeHtmlTransformer class that explains its purpose
(sanitizes autolinks while preserving HTML), notes it delegates to
SanitizeAutolinkFilter with escapeHtml: false, and clearly states the security
implications (it does NOT provide XSS protection and must be followed by
StandardizeHtmlSanitizingTransformers or equivalent to remove dangerous
tags/attributes/JS); reference the class name
SanitizeAutolinkUnescapeHtmlTransformer and the related SanitizeAutolinkFilter
and StandardizeHtmlSanitizingTransformers in the comment so future maintainers
know usage and safety requirements.
In `@lib/features/email/presentation/controller/single_email_controller.dart`:
- Around line 1257-1267: Extract the inline TransformConfiguration.create call
into a reusable static factory method (e.g.
createCalendarEventTransformConfiguration) that returns
TransformConfiguration.create with the same customTextTransformers
(SanitizeAutolinkUnescapeHtmlTransformer, StandardizeHtmlSanitizingTransformers,
NewLineTransformer) and customDomTransformers
(SanitizeHyperLinkTagInHtmlTransformer); replace the inline call with a call to
that new static method in single_email_controller and import/use the same
factory in tests so production and test transformer lists remain identical.
In
`@test/features/email/data/repository/calendar_event_repository_impl_test.dart`:
- Around line 153-161: The test helper expectDelegated currently verifies
htmlDatasource.transformHtmlEmailContent(raw, any) which allows any
TransformConfiguration; change the assertions to verify the exact
TransformConfiguration being forwarded by constructing or capturing the expected
TransformConfiguration and using that in verify (e.g.,
verify(htmlDatasource.transformHtmlEmailContent(raw, expectedConfig)) or use a
capture/argThat predicate) so the transformSingle(CalendarEvent...) call is
asserted to pass the precise TransformConfiguration; apply the same tightening
for the other occurrences around lines 195-206 where
htmlDatasource.transformHtmlEmailContent is verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ace5a593-95b7-4122-a9d8-4ac5d608fc7c
📒 Files selected for processing (5)
core/lib/presentation/utils/html_transformer/text/sanitize_autolink_unescape_html_transformer.dartlib/features/email/presentation/controller/single_email_controller.darttest/features/email/data/local/calendar_event_description_transform_test.darttest/features/email/data/repository/calendar_event_repository_impl_test.darttest/features/email/presentation/controller/single_email_controller_test.dart
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
hoangdat
reviewed
May 27, 2026
| }); | ||
| }); | ||
|
|
||
| group('HTML passthrough — HTML descriptions rendered as formatted content', () { |
Member
There was a problem hiding this comment.
how about the case
test('SHOULD preserve an existing HTML link destination and text', () async {
// Real-world calendar invites embed join links as HTML <a href>.
const description = '<a href="https://example.com/meeting">Join Zoom</a>';
final result = await transformDescription(description);
// The original destination URL must survive intact.
expect(result, contains('href="https://example.com/meeting"'));
// The visible link text must survive.
expect(result, contains('Join Zoom'));
// The markup must not be mangled into garbage by the linkifier.
expect(result, isNot(contains('<ahref')));
});
hoangdat
reviewed
May 27, 2026
| }); | ||
| }); | ||
|
|
||
| group('HTML passthrough — HTML descriptions rendered as formatted content', () { |
Member
There was a problem hiding this comment.
more coverage test
test('SHOULD NOT corrupt an HTML link whose visible text is also a URL', () async {
// href and anchor text both contain a URL — the linkifier must not
// double-process the attribute and break the tag.
const description =
'<a href="https://example.com/a">https://example.com/a</a>';
final result = await transformDescription(description);
expect(result, contains('href="https://example.com/a"'));
expect(result, isNot(contains('<ahref')));
});
test('SHOULD preserve a URL sitting in an <img> src attribute without mangling text', () async {
// URL lives in the src attribute, not body text. <img> may be stripped
// by the sanitizer, but the surrounding description must stay intact.
const description =
'<img src="https://cdn.example.com/a.png"> Team photo';
final result = await transformDescription(description);
expect(result, contains('Team photo'));
expect(result, isNot(contains('<ahref')));
});
test('SHOULD autolink a bare URL that appears inside an HTML element', () async {
// Positive guard: a URL in text content (not an attribute) must still
// become a clickable link, even when wrapped in an element.
const description =
'<p>Join the call at https://meet.example.com/room/9</p>';
final result = await transformDescription(description);
expect(result, contains('href="https://meet.example.com/room/9"'));
expect(result, contains('<p>'));
});Signed-off-by: dab246 <tdvu@linagora.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart (2)
39-39: 💤 Low valueLogging every processed result may be too verbose for production.
The current
log()call will output the full processed HTML for every calendar event description. This could generate significant log volume when users view multiple calendar events. Consider usinglogTrace()(for diagnostic purposes only) or removing this log entirely after initial validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart` at line 39, Replace the verbose public log in SanitizeAutolinkFilter::process that prints the full processed HTML by either removing it or switching it to a diagnostic-only call such as logTrace() (or the project's equivalent trace-level logger), so that the HTML payload is not always emitted in production; locate the line that calls log('SanitizeAutolinkFilter::process:htmlTextBuffer = $result') inside the process method and change it to a trace/debug-level log or delete the call depending on whether you want retained diagnostic visibility.
25-27: 💤 Low valueConsider
\"inside quoted HTML attribute values in_htmlTagPattern
_htmlTagPatternhandles quoted attributes by forbidding the delimiting quote character inside the value, so it won’t correctly match malformed inputs liketitle="He said \"hello\""as a single tag. Existing tests already cover the intended>case via>and the fallback for unclosed tags (core/test/sanitize_autolink_filter_test.dart). A repo-wide search also found no\"sequences in test/data. Add a small test or document that only standard HTML escaping (e.g.,") is supported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart` around lines 25 - 27, The _htmlTagPattern regex does not allow backslash-escaped quote characters inside quoted attribute values (e.g., title="He said \"hello\""), so either update the pattern or explicitly document/testing the supported behavior: either (A) broaden _htmlTagPattern to accept backslash-escaped quotes inside double- and single-quoted attributes (so it treats \" and \' as part of the value) by altering the RegExp in sanitize_autolink_filter.dart, or (B) add a small unit test in core/test/sanitize_autolink_filter_test.dart and a short comment above the static _htmlTagPattern explaining that only standard HTML escaping (e.g., ") is supported and backslash-escaped quotes are not recognized; pick one approach and implement it consistently (update the RegExp if you need to accept \" sequences, or add the test+doc if you want to explicitly reject them).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart`:
- Line 39: Replace the verbose public log in SanitizeAutolinkFilter::process
that prints the full processed HTML by either removing it or switching it to a
diagnostic-only call such as logTrace() (or the project's equivalent trace-level
logger), so that the HTML payload is not always emitted in production; locate
the line that calls log('SanitizeAutolinkFilter::process:htmlTextBuffer =
$result') inside the process method and change it to a trace/debug-level log or
delete the call depending on whether you want retained diagnostic visibility.
- Around line 25-27: The _htmlTagPattern regex does not allow backslash-escaped
quote characters inside quoted attribute values (e.g., title="He said
\"hello\""), so either update the pattern or explicitly document/testing the
supported behavior: either (A) broaden _htmlTagPattern to accept
backslash-escaped quotes inside double- and single-quoted attributes (so it
treats \" and \' as part of the value) by altering the RegExp in
sanitize_autolink_filter.dart, or (B) add a small unit test in
core/test/sanitize_autolink_filter_test.dart and a short comment above the
static _htmlTagPattern explaining that only standard HTML escaping (e.g.,
") is supported and backslash-escaped quotes are not recognized; pick one
approach and implement it consistently (update the RegExp if you need to accept
\" sequences, or add the test+doc if you want to explicitly reject them).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cb0c434-d236-492a-9d9d-670fc074a10f
📒 Files selected for processing (3)
core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dartcore/test/sanitize_autolink_filter_test.darttest/features/email/data/local/calendar_event_description_transform_test.dart
hoangdat
reviewed
May 27, 2026
|
|
||
| // Matches a complete HTML tag, correctly handling quoted attribute values | ||
| // that may contain the > character (e.g. title="a > b"). | ||
| static final _htmlTagPattern = RegExp(r'''<(?:[^>"']*|"[^"]*"|'[^']*')*>'''); |
Member
There was a problem hiding this comment.
it is ReDoS, should to find other approach to handle it. How about using DOM transformer?
Member
Author
There was a problem hiding this comment.
Oke, we will use new transformer for auto link
…plain text Signed-off-by: dab246 <tdvu@linagora.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart (1)
26-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid returning raw input when sanitization fails.
If
linkify()throws, this returns unsanitizedinputText, which can reintroduce raw HTML/script payloads in a failure path. Return an escaped fallback instead.🔧 Proposed fix
String process(String inputText) { try { if (inputText.isEmpty) return ''; final result = _linkifyText(inputText); log('SanitizeAutolinkFilter::process:htmlTextBuffer = $result'); return result; - } catch (e) { + } catch (e) { logWarning('$runtimeType::process:Exception = $e'); - return inputText; + return htmlEscape.convert(inputText); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart` around lines 26 - 37, process currently returns the raw unsanitized inputText on exceptions from _linkifyText, which can reintroduce unsafe HTML; change the catch in SanitizeAutolinkFilter::process to return an escaped fallback (e.g., run inputText through the project's HTML-escape utility or encoder) instead of inputText, and keep or augment the logWarning('$runtimeType::process:Exception = $e') so the error is recorded while ensuring the returned string is safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart`:
- Around line 30-33: The current SanitizeAutolinkFilter::process implementation
logs the full transformed content stored in result (produced by _linkifyText),
which can include sensitive user data; change the logging to avoid emitting the
payload — either remove the log or replace it with non-sensitive metadata such
as result.length or a boolean (e.g., isEmpty) and, if detailed content is
needed, gate it behind a diagnostic/debug flag that is off in production. Ensure
the log message no longer includes htmlTextBuffer or result contents but
provides minimal context (e.g., "SanitizeAutolinkFilter::process: length=...")
and keep _linkifyText usage unchanged.
In `@core/test/utils/autolink_text_node_transformer_test.dart`:
- Around line 581-588: The test currently only asserts that
AutolinkTextNodeTransformer exists; update it to also assert ordering by
importing SanitizeHyperLinkTagInHtmlTransformer, find its index (e.g.,
sanitizeIdx = transformers.indexWhere((t) => t is
SanitizeHyperLinkTagInHtmlTransformer)), and add an expectation that autolinkIdx
is lessThan(sanitizeIdx) so AutolinkTextNodeTransformer comes before
SanitizeHyperLinkTagInHtmlTransformer in
TransformConfiguration.forCalendarEvent().domTransformers.
---
Outside diff comments:
In `@core/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dart`:
- Around line 26-37: process currently returns the raw unsanitized inputText on
exceptions from _linkifyText, which can reintroduce unsafe HTML; change the
catch in SanitizeAutolinkFilter::process to return an escaped fallback (e.g.,
run inputText through the project's HTML-escape utility or encoder) instead of
inputText, and keep or augment the logWarning('$runtimeType::process:Exception =
$e') so the error is recorded while ensuring the returned string is safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8ec236c-d9b5-436d-a4ca-9b702b967d8d
📒 Files selected for processing (6)
core/lib/presentation/utils/html_transformer/dom/autolink_text_node_transformer.dartcore/lib/presentation/utils/html_transformer/sanitize_autolink_filter.dartcore/lib/presentation/utils/html_transformer/transform_configuration.dartcore/test/sanitize_autolink_filter_test.dartcore/test/utils/autolink_text_node_transformer_test.darttest/features/email/data/local/calendar_event_description_transform_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- core/lib/presentation/utils/html_transformer/transform_configuration.dart
…red as plain text Signed-off-by: dab246 <tdvu@linagora.com>
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
hoangdat
approved these changes
May 28, 2026
hoangdat
deleted the
fix/calendar-event-html-description-rendered-as-plain-text
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Calendar event (ICS) descriptions that contain HTML markup were displayed as raw, unrendered text in the email viewer. Instead of seeing formatted content, users saw the literal HTML tags — e.g. a description like:
was shown as the visible characters
<p>We are delighted to extend a warm invitation...</p>instead of a rendered paragraph.Root Cause
The transformer pipeline in
_parseCalendarEventActionwas treating HTML descriptions as plain text:SanitizeAutolinkHtmlTransformerswithescapeHtml: truecallshtmlEscape.convert()on the raw text before any DOM parsing, converting<and>to</>. This is correct for plain-text emails, but ICS descriptions are typically HTML — so the markup was rendered as visible escaped characters instead of formatted content.Fix
Introduce
AutolinkTextNodeTransformer— a DOM transformer that walks the already-parsed DOM to linkify bare URLs and email addresses in text nodes — and replace the plain-text pipeline with an HTML-aware named factory:AutolinkTextNodeTransformerwalks the parsed DOM tree rather than using a regex over raw text, so tag boundaries come from the document structure. This eliminates any ReDoS risk and avoids double-linkifying content inside existing<a>,<script>,<style>, or<textarea>elements.SanitizeAutolinkFilterwas also refactored alongside this change: theescapeHtmlflag was removed (it always HTML-escapes text elements now), and_linkifyOption/_linkifierwere madestaticto avoid re-allocating per call.Security
XSS is blocked by
StandardizeHtmlSanitizingTransformers, which strips<script>tags entirely, removes event-handler attributes (onerror,onclick, …), and stripsjavascript:URIs fromhrefattributes. This is verified by the updated tests insingle_email_controller_test.dartandcalendar_event_description_transform_test.dart.Note: the XSS neutralisation behaviour changed. The old pipeline rendered
<script>alert(1)</script>as visible escaped text (<script>…). The new pipeline removes the element entirely. Both outcomes are safe; the new one produces cleaner output.Summary by CodeRabbit
New Features
Improvements
Tests