Fix: render HTML instead of raw code on print#1851
Conversation
Bug ReportNo bugs found. Definition of Comments? Email us. This is the last day of your free trial. Subscribe at jazzberry.ai. |
WalkthroughThis change removes a locally defined Changes
Sequence Diagram(s)sequenceDiagram
participant MailDisplay
participant email-utils
MailDisplay->>email-utils: cleanHtml(decodedBody)
email-utils-->>MailDisplay: Returns sanitized HTML
MailDisplay->>User: Renders cleaned email content
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bug Report
Comments? Email us. This is the last day of your free trial. Subscribe at jazzberry.ai. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 1 file. Review in cubic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/mail-display.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (AGENT.md)
**/*.{js,jsx,ts,tsx}: Use 2-space indentation
Use single quotes
Limit lines to 100 characters in width
Semicolons are required
Files:
apps/mail/components/mail/mail-display.tsx
**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit Inference Engine (AGENT.md)
Use Prettier with sort-imports and Tailwind plugins
Files:
apps/mail/components/mail/mail-display.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (AGENT.md)
Enable TypeScript strict mode
Files:
apps/mail/components/mail/mail-display.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
apps/mail/components/mail/mail-display.tsx (3)
Learnt from: snehendu098
PR: #1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
Learnt from: retrogtx
PR: #1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: danteissaias
PR: #618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
🧬 Code Graph Analysis (1)
apps/mail/components/mail/mail-display.tsx (1)
apps/mail/lib/email-utils.ts (1)
cleanHtml(213-222)
🔇 Additional comments (1)
apps/mail/components/mail/mail-display.tsx (1)
58-58: Good call—one shared sanitizer beats many bespoke hacksPulling in
cleanHtmlaligns the module with our central XSS-defense strategy and deletes dead weight (escapeHtml). Nice move; less code, more safety.
Bug ReportName: XSS Vulnerability due to DOMPurify fallback No email content available '. While this prevents a crash, it means that unsanitized HTML from the email body will be rendered. Since the user could insert arbitrary HTML, this opens a vulnerability to XSS attacks. Since the email is replaced entirely, I am marking the severity as medium. The second commit removes the extra fallback, so if the cleanHtml function returns undefined or null, nothing will be rendered.Comments? Email us. This is the last day of your free trial. Subscribe at jazzberry.ai. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/mail-display.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (AGENT.md)
**/*.{js,jsx,ts,tsx}: Use 2-space indentation
Use single quotes
Limit lines to 100 characters in width
Semicolons are required
Files:
apps/mail/components/mail/mail-display.tsx
**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit Inference Engine (AGENT.md)
Use Prettier with sort-imports and Tailwind plugins
Files:
apps/mail/components/mail/mail-display.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (AGENT.md)
Enable TypeScript strict mode
Files:
apps/mail/components/mail/mail-display.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
apps/mail/components/mail/mail-display.tsx (4)
Learnt from: snehendu098
PR: #1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
Learnt from: danteissaias
PR: #618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
Learnt from: retrogtx
PR: #1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: retrogtx
PR: #1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
🔇 Additional comments (1)
apps/mail/components/mail/mail-display.tsx (1)
56-60: Centralized HTML sanitization locked in, looks good 🚀
Ran a repo‐wide search—no lingeringescapeHtmlcalls. Clean, consistent, and DRY. Onward to a leaner codebase (and Mars).
Bug ReportName: Potential XSS vulnerability due to outdated DOMPurify version and lack of configuration check Comments? Email us. |
|
Hi @MrgSub, could you please take a look at this PR when you get a chance? Let me know if any changes are needed. Thanks! |
|
Lil bit of lore https://x.com/reactreaper/status/1952029007400354009 |
3 participants
Description
Fixes the issue where clicking the "Print" button in email displays raw HTML instead of the rendered email output.
Fixes the issue #1850
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
The existing
cleanHtmlutility was reused to sanitize and render the HTML properly before printing.Screenshots/Recordings
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by cubic
Fixed the print button so it now shows the rendered email content instead of raw HTML code.
Summary by CodeRabbit