Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ParseException with mail received over internet via SMTP #143

Closed
hohwille opened this issue Mar 14, 2024 · 11 comments
Closed

ParseException with mail received over internet via SMTP #143

hohwille opened this issue Mar 14, 2024 · 11 comments
Labels
duplicate This issue or pull request already exists

Comments

@hohwille
Copy link

Describe the bug
When I read emails that have been sent over the internet and arrived their destination from EML format using JakarataMail, for some edge-cases, I get strange errors like this:

jakarta.mail.internet.ParseException: In parameter list <;
	creation-date=Mon, 25 Dec 2017 02:48:38 GMT;
	modification-date=Mon, 25 Dec 2017 02:48:38 GMT>, expected ';', got ","
	at jakarta.mail.internet.ParameterList.<init>(ParameterList.java:299)

To Reproduce
Steps to reproduce the behavior:

  1. Create an EML message with a MIME part containing this header:
Content-Disposition: attachment;
	creation-date=Mon, 25 Dec 2017 02:48:38 GMT;
	modification-date=Mon, 25 Dec 2017 02:48:38 GMT
  1. Read that mail using JakartaMail
  2. See error cited above

Expected behavior
I do not know what the initial design decisions from Sun may have been when all this code was initially born, but from my PoV reading a mail with JakartaMail should be as robust as possible. Also retrieving attributes (using getFrom(), getTo(), getDisposition(), etc.) should ideally give me back what has been in the mail.
In a perfect world, validation should be separate from reading and accessing the data.
I am very aware that the API of JakartaMail cannot be changed very easily (also using java.util.Date seems to be a crime nowadays after having JSR310). But at least avoiding unnecessary ParseErrors should be a reasonable goal.
In the concrete example it would be easy to tokenize using the expected semicolon without complaining about the coma in the date value.
I do not want to judge if the value of the cited Content-Disposition header is valid according to relevant RFCs or not.
Maybe it is not using the correct date/time format.
However, the mail clients and relay servers out there in the wild may just do such things and it would be awesome if JakartaMail does not get in the way here.

Environment:

  • OS: Windows 11
  • Java 17.0.10
  • jakarta.mail-api 2.1.2
@jmehrens
Copy link
Contributor

Do you know what email client generated the email via X-Mailer header or User-Agent header?

@hohwille
Copy link
Author

X-MimeOLE: Produced By Microsoft MimeOLE V6.1.7601.23651

Looks like Outlook-Express:
https://learn.microsoft.com/en-us/previous-versions/windows/desktop/oe/oe-mimeolecreatemessage

@jmehrens
Copy link
Contributor

I'll have to run your test case but, it seems that setting system property (not session property) mail.mime.parameters.strict=false is what you want to do in your use case.

If set to false, when reading a message, parameter values in header fields such as Content-Type and Content-Disposition are allowed to contain whitespace and other special characters without being quoted; the parameter value ends at the next semicolon. If set to true (the default), parameter values are required to conform to the MIME specification and must be quoted if they contain whitespace or special characters.

@hohwille
Copy link
Author

hohwille commented Apr 2, 2024

@jmehrens thank you so much for your response and the tip that I tried:

      Properties props = new Properties();
      // Don't parse addresses with respect to RFC822. take them as is.
      props.setProperty("mail.mime.address.strict", "false");
      props.setProperty("mail.mime.parameters.strict", "false");
      props.setProperty("mail.mime.decodeparameters.strict", "false");
      props.setProperty("mail.mime.decodetext.strict", "false");
      Session s = Session.getDefaultInstance(props);
      return new MimeMessage(s, mailData);
      ...

However, I still get the same exception. I have verified it in debug mode and all mails are parsed using this single code function so I can confirm this is not yet the solution to close this issue.
Are there more such properties that I should try? Cannot find a comprehensive documentation about these properties. Just grabbed them from the JavaDoc.

@jmehrens
Copy link
Contributor

jmehrens commented Apr 2, 2024

My mistake. The mail.mime.address.strict is documented as session property. The rest are system properties. In your example you set everything as a session property so the properties don't do anything. I would set mail.mime.address.strict as a system property too.

Here is the source:
https://github.com/jakartaee/mail-api/blob/bf2bfc18c0d2b9b3f57417203c2a462a3a94a7de/api/src/main/java/jakarta/mail/internet/ParameterList.java#L126

I think you'll want to set mail.mime.parameters.strict and mail.mime.windowsfilenames as system properties for the jvm and re-test.

@hohwille
Copy link
Author

hohwille commented Apr 4, 2024

@jmehrens thank you very much for pointing this out and for the link to the source. Very helpful.
Now I use spring-boot @ConfigurationProperties class to map all properties from mail.mime. and set them automatically as system properties and to the session properties.

@Named
@ConfigurationProperties("mail")
public class MailMimeConfigProperties {

  private Map<String, String> mime;

  private Properties properties;

  public Map<String, String> getMime() {
    return this.mime;
  }

  public void setMime(Map<String, String> mime) {
    this.mime = mime;
  }

  public Properties getProperties() {

    if (this.properties == null) {
      Properties p = new Properties();
      for (Entry<String, String> entry : mime.entrySet()) {
        String key = "mail.mime." + entry.getKey();
        String value = entry.getValue();
        p.setProperty(key, value);
        System.setProperty(key, value);
        System.out.println("Set " + key + "=" + value + " for session and system property");
      }
      this.properties = p;
    }
    return this.properties;
  }
}

And in my application.properties I have:

# configure jakarta mail (https://github.com/eclipse-ee4j/angus-mail/issues/143)
mail.mime.address.strict=false
mail.mime.parameters.strict=false
mail.mime.decodeparameters.strict=false
mail.mime.decodetext.strict=false
mail.mime.windowsfilenames=true

And to verify I see this output:

Set mail.mime.address.strict=false for session and system property
Set mail.mime.parameters.strict=false for session and system property
Set mail.mime.decodeparameters.strict=false for session and system property
Set mail.mime.decodetext.strict=false for session and system property
Set mail.mime.windowsfilenames=true for session and system property

However, I still get the same exception when parsing the mail so it seems these properties do not solve the actual problem.

@jmehrens
Copy link
Contributor

jmehrens commented Apr 4, 2024

There is a danger setting system properties in code in that they have to be set before the class loads. Try setting system properties on the command line or if you are using a web container like Tomcat you'll have to consult the documentation on setting system properties.

I should be able to verify your test case in a week or so and come up with plan for this ticket. Thanks for testing.

@hohwille
Copy link
Author

hohwille commented Apr 8, 2024

Sorry for the late response.
@jmehrens you are perfectly correct: When setting up the system properties as JVM args it works:

java -Dmail.mime.decodeparameters.strict=false -Dmail.mime.parameters.strict=false -Dmail.mime.address.strict=false -Dmail.mime.decodetext.strict=false ...

So that is great news. The code implementation already can handle the problem I was reporting.

Feel free to take this as a feature request to consider allowing to override the system properties also as session properties.
But apart from that, all is fine from my PoV and then this issue can be closed.
I can also make spring-boot to create my configuration early on and eager setting the system properties so it happens before jakarta mail classes are actually loaded. But ability to configure/override via session properties would be a perfect solution to avoid potential confusion like I had on my way.

@jmehrens
Copy link
Contributor

jmehrens commented Apr 8, 2024

That is good news. We have another ticket for system vs. session properties so that issue will still be worked on but I will close this as a duplicate

@jmehrens
Copy link
Contributor

jmehrens commented Apr 8, 2024

Dupe of jakartaee/mail-api#513

@jmehrens jmehrens closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2024
@jmehrens jmehrens added the duplicate This issue or pull request already exists label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants