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

IllegalArgumentException when validating email address ending with "High Octet Preset" control character #39

Closed
PascalSchumacher opened this issue Nov 16, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@PascalSchumacher
Copy link

Using JMail 1.3.2 to validate an email address ending with a "High Octet Preset" control character, e.g. JMail.isValid("a@b.com\u0081"); fails with:

java.lang.IllegalArgumentException: java.text.ParseException: A prohibited code point was found in the inputcom�
	at java.base/java.net.IDN.toASCIIInternal(IDN.java:275)
	at java.base/java.net.IDN.toASCII(IDN.java:123)
	at com.sanctionco.jmail.JMail.isValidIdn(JMail.java:512)
	at com.sanctionco.jmail.JMail.lambda$tryParse$0(JMail.java:119)
	at java.base/java.util.Optional.filter(Optional.java:218)
	at com.sanctionco.jmail.JMail.tryParse(JMail.java:119)
	at com.sanctionco.jmail.JMail.isValid(JMail.java:67)
Caused by: java.text.ParseException: A prohibited code point was found in the inputcom�
	at java.base/jdk.internal.icu.text.StringPrep.prepare(StringPrep.java:448)
	at java.base/java.net.IDN.toASCIIInternal(IDN.java:273)
	... 74 more

I guess that is not the expected behavior, or am I wrong?

By the way: Thank you very much for developing JMail! It is the best Java E-Mail validation library.

@PascalSchumacher PascalSchumacher added the bug Something isn't working label Nov 16, 2021
@RohanNagar
Copy link
Owner

RohanNagar commented Nov 16, 2021

Thank you for the report @PascalSchumacher. You're right that this is not the expected behavior, I believe JMail should return false in this case as this would not be considered a valid email address.

@RohanNagar
Copy link
Owner

@PascalSchumacher please let me know if the fix in fbeecce looks good to you and I will publish a new version v1.3.3.

@PascalSchumacher
Copy link
Author

@RohanNagar Thanks for the quick fix. I always a bit nervous when I see a catch block catching Exception because it also catches Throwables like OutOfMemoryError. According to the Java Doc this method only throws IllegalArgumentExceptions so I believe catching that would be better. What do you think?

@RohanNagar
Copy link
Owner

@PascalSchumacher Good point. OutOfMemoryError actually would not be caught since it extends Error. If we did something like catch (Throwable t), then it would be caught.

However since this method only throws IllegalArgumentException it makes sense to only catch that. I've updated and released version 1.3.3, which should be available on maven central in about 10 minutes!

@PascalSchumacher
Copy link
Author

PascalSchumacher commented Nov 17, 2021

@PascalSchumacher Good point. OutOfMemoryError actually would not be caught since it extends Error. If we did something like catch (Throwable t), then it would be caught.

Yes, you are right. I do not know what I was thinking. Sorry!

Thank you very much for the new release! 👍 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants