Skip to content

Comments

Eml-to-pdf bug fixes: removal of incompatible fonts, removal of emoji in favor of @, jakarta-mail dependency handling improvements#3770

Merged
Frooodle merged 16 commits intoStirling-Tools:mainfrom
balazs-szucs:update-eml-to-pdf-to-support-only-general-fonts
Jun 19, 2025
Merged

Eml-to-pdf bug fixes: removal of incompatible fonts, removal of emoji in favor of @, jakarta-mail dependency handling improvements#3770
Frooodle merged 16 commits intoStirling-Tools:mainfrom
balazs-szucs:update-eml-to-pdf-to-support-only-general-fonts

Conversation

@balazs-szucs
Copy link
Collaborator

Description of Changes

This pull request introduces enhancements and code cleanup to the EmlToPdf utility class, focusing on improving email-to-PDF conversion, handling embedded images, and simplifying the codebase. Key changes include better handling of inline images, enhanced Jakarta Mail dependency checks, and refactoring for improved readability and maintainability.

Enhancements to Email-to-PDF Conversion:

  • Added support for processing inline images (cid: references) by converting them into data URIs for proper inline display.
  • Improved attachment handling to always include embedded images regardless of size, ensuring inline display functionality.
  • Enhanced email HTML generation to process inline images and include them in the email body.

Attachment Handling Enhancements:

  • Replaced the attachment icon placeholder (icon or 📎 emoji) with a new marker (@) for consistency across the application (non-fat images did not support the emoji, however @ is supported accross the board.)
  • Updated the annotation logic to use AttachmentMarkerPositionFinder instead of EmojiPositionFinder, aligning with the new attachment marker system.

Jakarta Mail Dependency Handling:

  • Added detailed checks for core Jakarta Mail classes to determine availability in different environments (e.g., Docker).
  • Introduced validation for Jakarta Mail multipart and part types to prevent processing invalid objects.
  • Explicitly parse in the classes:
  • jakarta.mail.internet.MimeMessage – Core email message parsing
  • jakarta.mail.Session – Email session management
  • jakarta.mail.internet.MimeUtility – MIME encoding/decoding utilities
  • jakarta.mail.internet.MimePart – Individual MIME parts (attachments, body parts)
  • jakarta.mail.internet.MimeMultipart – Multi-part MIME messages
  • jakarta.mail.Multipart – Base multipart interface
  • jakarta.mail.Part – Base part interface

Code Cleanup and Refactoring:

  • Simplified utility classes (StyleConstants, MimeConstants, FileSizeConstants) by removing unnecessary constructors and unused constants.
  • Updated log messages for clarity, such as distinguishing between general content processing errors and multipart-specific issues.

Checklist

General

Documentation

UI Changes (if applicable)

  • Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR)

Testing (if applicable)

  • I have tested my changes locally. Refer to the Testing Guide for more details.

Copilot AI review requested due to automatic review settings June 18, 2025 23:19
@dosubot dosubot bot added the size:L This PR changes 100-499 lines ignoring generated files. label Jun 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the EmlToPdf utility to address inline image processing, replace unsupported emoji representations with an "@" marker, and improve Jakarta Mail dependency handling for enhanced email-to-PDF conversion.

  • Updates style and MIME constants to simplify font and attachment marker management
  • Enhances inline image processing, logging, and reflection-based constructor usage
  • Refactors attachment and multipart processing with additional Jakarta Mail type checks
Comments suppressed due to low confidence (2)

common/src/main/java/stirling/software/common/util/EmlToPdf.java:1693

  • The original EmojiPositionFinder constructor declared throwing IOException due to PDFTextStripper's constructor. Verify that removing the throws clause in AttachmentMarkerPositionFinder does not lead to unhandled IO issues or compilation errors with PDFBox.
        public AttachmentMarkerPositionFinder() {

common/src/main/java/stirling/software/common/util/EmlToPdf.java:183

  • Changing validateEmlInput to no longer throw IOException (and instead relying on IllegalArgumentException) could affect callers expecting IO-related exceptions. Confirm that downstream error handling is adjusted accordingly.
    private static void validateEmlInput(byte[] emlBytes) {

@github-actions github-actions bot added the Java Pull requests that update Java code label Jun 18, 2025
@dosubot dosubot bot added the enhancement New feature or request label Jun 18, 2025
@balazs-szucs balazs-szucs changed the title Eml-to-pdf bug fixes: removal of incompatible fonts (of non fat images), removal of emoji in favor of @, jakarta-mail dependency handling improvements Eml-to-pdf bug fixes: removal of incompatible fonts, removal of emoji in favor of @, jakarta-mail dependency handling improvements Jun 18, 2025
@Frooodle
Copy link
Member

what is it missing that causes the Jakarta Mail libraries to be missing? what system dep?

@balazs-szucs
Copy link
Collaborator Author

balazs-szucs commented Jun 19, 2025

I do think that the font was my own fault, but Jakarta Mail problem was partly missing version.properties. I added better fallback to this, and kind more explicit checks. I tried to be more explicit in how the especially the content part handled e.g in processMultipartAdvanced/Image handling just in case and that seem to solve the other part of it. In my testing, when I added logging there it seemed like it wasn't even called or reached by the code when in docker. Not 100% sure why.

Anyhow, I spent considerable time testing this with all sorts of edge cases, and I am confident about this.

@balazs-szucs
Copy link
Collaborator Author

balazs-szucs commented Jun 19, 2025

Do you think it would be wise to add junit Test to this? I think that would levitate some concerns , and making emails for testing that would fail without proper dependencies wouldn't be that hard. But I didn't check this beforehand so this is just a guess.

@github-actions github-actions bot added the Back End Issues related to back-end development label Jun 19, 2025
@stirlingbot stirlingbot bot removed enhancement New feature or request Java Pull requests that update Java code Back End Issues related to back-end development labels Jun 19, 2025
@Frooodle
Copy link
Member

JUnits are welcome to add but not a requirement for the repo, although we should start requiring that tbf...

@balazs-szucs
Copy link
Collaborator Author

Makes sense. I wouldn't mix Junit test with this per se, but I'll work on that then. Sorry for inconvenience on this.

@Frooodle
Copy link
Member

Ill merge this code as is, junits can be separate

@Frooodle Frooodle merged commit 9923411 into Stirling-Tools:main Jun 19, 2025
11 checks passed
@Ludy87 Ludy87 added enhancement New feature or request Java Pull requests that update Java code Back End Issues related to back-end development labels Jun 19, 2025
@balazs-szucs balazs-szucs deleted the update-eml-to-pdf-to-support-only-general-fonts branch June 23, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Back End Issues related to back-end development enhancement New feature or request Java Pull requests that update Java code size:L This PR changes 100-499 lines ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants