-
Notifications
You must be signed in to change notification settings - Fork 149
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
VALIDATOR-368: allowed angle brackets in email addresses #140
VALIDATOR-368: allowed angle brackets in email addresses #140
Conversation
this PR was done 70 days ago. It awaits review and merge |
I should be able to come back around to this component is a week or two. I am working on releasing Commons IO ATM... |
Hi, any idea when this fix can be reviewed/merged? thx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this changes the behaviour, I think it needs to be optional.
One way to do that would be to provide a lenient matcher that strips off the comment and angle brackets and then calls the existing validator with the embedded email address.
Rather than add yet another ctor parameter, it might make sense as a subclass.
private static final String IP_DOMAIN_REGEX = "^\\[(.*)\\]$"; | ||
private static final String USER_REGEX = "^" + WORD + "(\\." + WORD + ")*$"; | ||
private static final String FULL_NAME_EMAIL_REGEX = "^(.+)<" + EMAIL_CHARACTERS + ">$"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why capture the leading comment?
It is not needed.
@@ -407,6 +407,8 @@ public void testEmailUserName() { | |||
assertFalse(validator.isValid("Abc@def@example.com")); | |||
|
|||
assertTrue(validator.isValid("space\\ monkey@example.com")); | |||
|
|||
assertTrue(validator.isValid("Abigail Jones <abigail@example.com>")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some tests to show that user part and domain part are properly validated
@@ -179,15 +182,28 @@ public boolean isValid(final String email) { | |||
|
|||
// Check the whole email address structure | |||
final Matcher emailMatcher = EMAIL_PATTERN.matcher(email); | |||
if (!emailMatcher.matches()) { | |||
final Matcher fullnameEmailMatcher = EMAIL_WITH_FULL_NAME_PATTERN.matcher(email); | |||
final boolean fullnameEmailMatches = fullnameEmailMatcher.matches(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure you need to keep this variable
this PR has been open for half a year |
There are some review comments that need to be addressed before it can be considered for inclusion in the code base. |
We should really find a solution for this - constructor parameter || lenient matcher I don't mind. Any solution helps |
Contributions are still welcome. It does not look to be particularly difficult, but someone has to do the work... |
VALIDATOR-368: allowed angle brackets in email addresses