[jira] Created: (OFBIZ-668) UtilValidate.isEmail has incorrect logic

[Nicolas Malin (Jira)]

UtilValidate.isEmail has incorrect logic
----------------------------------------

                 Key: OFBIZ-668
                 URL: https://issues.apache.org/jira/browse/OFBIZ-668
             Project: OFBiz (The Open for Business Project)
          Issue Type: Bug
          Components: framework
    Affects Versions: SVN trunk
         Environment: all
            Reporter: John Martin
            Priority: Minor


Two problems:

1. The isEmail(String email, boolean requireDot) test for an empty/null string and will return true.  Obviously both these conditions should return false.

2. the isEmail(String email) default is to not test for a dot in the domain.  Being that most tests would be for real email address (not something like admin@localhost), that the default should be true.

I'm also suggesting that we wrapper the Java Commons email validator class which is much more precise

 org.apache.commons.validator.EmailValidator

http://www.koders.com/java/fid2F1364A91DBBCED0B3D0DB88F5AA0499FD29A77F.aspx

Any thoughts?

Will implement after comments.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato reassigned OFBIZ-668:
---------------------------------------

    Assignee: Jacopo Cappellato

> UtilValidate.isEmail has incorrect logic
> ----------------------------------------
>
>                 Key: OFBIZ-668
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-668
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>         Environment: all
>            Reporter: John Martin
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>
> Two problems:
> 1. The isEmail(String email, boolean requireDot) test for an empty/null string and will return true.  Obviously both these conditions should return false.
> 2. the isEmail(String email) default is to not test for a dot in the domain.  Being that most tests would be for real email address (not something like admin@localhost), that the default should be true.
> I'm also suggesting that we wrapper the Java Commons email validator class which is much more precise
>  org.apache.commons.validator.EmailValidator
> http://www.koders.com/java/fid2F1364A91DBBCED0B3D0DB88F5AA0499FD29A77F.aspx
> Any thoughts?
> Will implement after comments.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato updated OFBIZ-668:
------------------------------------

    Attachment: emailvalidation.patch

Can I commit the attached patch?
Summary:

1) replaced the logic inside the isEmail method with a call to org.apache.commons.validator.EmailValidator.isValid(...)

2) however I've left the code that returns true if the string is empty; here is the method as is now:

public static boolean isEmail(String s) {
    if (isEmpty(s)) return defaultEmptyOK;
    return EmailValidator.getInstance().isValid(s);
}

3) the only remarkable difference is that the original code was accepting (by default) a domain without dots (user@localhost); the new code requires a real domain ([hidden email])

Is this acceptable?

> UtilValidate.isEmail has incorrect logic
> ----------------------------------------
>
>                 Key: OFBIZ-668
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-668
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>         Environment: all
>            Reporter: John Martin
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>         Attachments: emailvalidation.patch
>
>
> Two problems:
> 1. The isEmail(String email, boolean requireDot) test for an empty/null string and will return true.  Obviously both these conditions should return false.
> 2. the isEmail(String email) default is to not test for a dot in the domain.  Being that most tests would be for real email address (not something like admin@localhost), that the default should be true.
> I'm also suggesting that we wrapper the Java Commons email validator class which is much more precise
>  org.apache.commons.validator.EmailValidator
> http://www.koders.com/java/fid2F1364A91DBBCED0B3D0DB88F5AA0499FD29A77F.aspx
> Any thoughts?
> Will implement after comments.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12533309 ]

David E. Jones commented on OFBIZ-668:
--------------------------------------

I think that's fine. It definitely needs it isEmpty check there so that, as with all other methos, isEmpty can be checked separately from the rest of this stuff.

As for the no dot thingy, basically for local and intranet domains only, I always thought that was a bit funny and usually not what people want, so I'm totally okay with this. Will be interesting to see what others think though.

> UtilValidate.isEmail has incorrect logic
> ----------------------------------------
>
>                 Key: OFBIZ-668
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-668
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>         Environment: all
>            Reporter: John Martin
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>         Attachments: emailvalidation.patch
>
>
> Two problems:
> 1. The isEmail(String email, boolean requireDot) test for an empty/null string and will return true.  Obviously both these conditions should return false.
> 2. the isEmail(String email) default is to not test for a dot in the domain.  Being that most tests would be for real email address (not something like admin@localhost), that the default should be true.
> I'm also suggesting that we wrapper the Java Commons email validator class which is much more precise
>  org.apache.commons.validator.EmailValidator
> http://www.koders.com/java/fid2F1364A91DBBCED0B3D0DB88F5AA0499FD29A77F.aspx
> Any thoughts?
> Will implement after comments.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato closed OFBIZ-668.
-----------------------------------

    Resolution: Fixed

I've committed the patch in rev. 583373

> UtilValidate.isEmail has incorrect logic
> ----------------------------------------
>
>                 Key: OFBIZ-668
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-668
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>         Environment: all
>            Reporter: John Martin
>            Assignee: Jacopo Cappellato
>            Priority: Minor
>         Attachments: emailvalidation.patch
>
>
> Two problems:
> 1. The isEmail(String email, boolean requireDot) test for an empty/null string and will return true.  Obviously both these conditions should return false.
> 2. the isEmail(String email) default is to not test for a dot in the domain.  Being that most tests would be for real email address (not something like admin@localhost), that the default should be true.
> I'm also suggesting that we wrapper the Java Commons email validator class which is much more precise
>  org.apache.commons.validator.EmailValidator
> http://www.koders.com/java/fid2F1364A91DBBCED0B3D0DB88F5AA0499FD29A77F.aspx
> Any thoughts?
> Will implement after comments.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.