Improve mail test coverage #3674

[JiriOndrusek]

fixes #3674

[aldettinger]

Is ssl support already covered ? If not, would there be any interest ? Maybe another PR anyway.

[aldettinger]

Also, it looks jakarta.mail is using Locale.ENGLISH a lot for instance charset.toLowerCase(Locale.ENGLISH). I wonder if it would be possible to run the tests in native mode once on a machine where the default Locale is not english to see whether there is an issue.

[ppalaga]

@JiriOndrusek if you choose to do any more changes, please rebase too.

[jamesnetherton]

Is ssl support already covered

IMO we should test all of the protocols supported by the component. Seems we just cover pop3 at the moment.

[JiriOndrusek]

I'll add test coverage for ssl and other protocols as well.

[aldettinger]

Jiri, more context on the instinct that we may have issues with locales.
There are clues that in native mode, only the default locale is embedded:
#1861

So, on a machine with another default locale, I wonder how all the english/us locale from jakarta.mail would behave.
Note that it's just instinct, not actual problem seen.

Now, about how to test with another default locale, one can try to play with -Duser.country=CN -Duser.language=zh but I'm not sure it would have same effect has a default locale coming from the environment. In which case maybe @zhfeng or @ffang may perharps try to run this PR locally if they have time.

[JiriOndrusek]

Still work in progress. I refactored test coverage to use greenmail (as docker image). SSL coverage is still missing.

[JiriOndrusek]

PR is complete. Tests cover all protocols + ssl. I used java mail properties to trust all certificates:

    props.put("mail.smtp.auth", "false");
    props.put("mail.smtp.ssl.enable", "true");
    props.put("mail.smtp.ssl.trust", "*");

(because you have to import the CA certificates into the JVM’s Java trust/key store files, - see camel documentation

In case that the security tests will be revisited, it would be helpful to wait for the new version of greenmail docker container (1.6.8+ or 2.0.0-alpha-3+) with following fix greenmail-mail-test/greenmail@37a09af#diff-5dfefe1b55f9a4d77b491f77357ebb0910cd2d088a51c46c0ec62a5aa8bc8b59R24-R31

[jamesnetherton]

Sorry - I had another scan through the PR and have some additional comments.

[JiriOndrusek]

@jamesnetherton It would be nice to get this PR tested on the machine with different default locale (as @aldettinger mentioned in older comment). Do we have this option on some of testing machines?

[jamesnetherton]

Do we have this option on some of testing machines?

Not really.....

You could either build your own VM and set an appropriate locale for testing. Or experiment with GitHub actions on your fork and change the system locale before the native test runs.

Probably best to capture this as a follow up issue as I'd like to get this PR merged and not leave it dangling much longer.

[JiriOndrusek]

yes, I agree to not delay this PR.