Possible bugs on Email and Range Validators

fsoler · **Joined:** Tue Aug 06, 2013 11:32 am **Posts:** 2

Hi,
I'm pretty new using HV project but I have found two scenarios that could be bugs.

Scenario 1
------------
I tried to validate a Email with the @Email annotation, but this validator accept an email like this: "foo@bar@" or "foo@bar@@@@" as they split the string by '@' and check parts.length == 2. However split method does not return an empty string part at the end of the string. So the result of this operation

Code:

String[] parts = "foo@bar@".split('@')

is:

parts.length = 2
part[0] = foo
part[1] = bar

I suggest to check this case on validator to avoid accept wrong emails.

Scenario 2
------------
I add to a String field the @Range annotation and it returns false when the field value is an empty string. I will find this more intuitive if in this case it would return true.

Taking this scenario:

Code:

@Range(min = 0, max = 50)
String value;

If value is null, we cannot apply the range constraint, so you cannot assume that is a wrong value. Well, but if we have an empty string, we still cannot apply the range constraint as the value is not a number and the parse will fail and the validator will return false. Null value nor the empty string cannot be compared with values between 0 and 50 so I suggest in this case to add a check to Validator to avoid this behavior.

I tried to open both bugs on jira but I couldn't. I could attach examples if needed.
Thanks for the attention.

Gunnar · **Joined:** Sat Jan 24, 2009 12:46 pm **Posts:** 388

Hi,

Thanks for the detailed reports. Scenario 1 looks like a bug indeed, I've created https://hibernate.atlassian.net/browse/HV-810 for this. Maybe you would be interested in providing a fix for this issue? Btw. I'm not sure why you couldn't create in issue in JIRA; Do you have a user account?

Regarding scenario 2, I don't think that's a bug. As per the BV spec, null is considered by all validators as valid (except for the @NotNull validator of course). But you have to deal with all other values in the validator. That's what is happening here, an empty string (as well as Strings with non-digit characters) doesn't represent a number within the specified range and thus is invalid. You may add @NotEmpty or @Size(min = 1) to your attribute to make sure the string is not empty. Or you could create a composed constraint comprising @Range and @NotEmpty.

--Gunnar

fsoler · **Joined:** Tue Aug 06, 2013 11:32 am **Posts:** 2

Gunnar wrote:

Hi,

Thanks for the detailed reports. Scenario 1 looks like a bug indeed, I've created https://hibernate.atlassian.net/browse/HV-810 for this. Maybe you would be interested in providing a fix for this issue? Btw. I'm not sure why you couldn't create in issue in JIRA; Do you have a user account?

Regarding scenario 2, I don't think that's a bug. As per the BV spec, null is considered by all validators as valid (except for the @NotNull validator of course). But you have to deal with all other values in the validator. That's what is happening here, an empty string (as well as Strings with non-digit characters) doesn't represent a number within the specified range and thus is invalid. You may add @NotEmpty or @Size(min = 1) to your attribute to make sure the string is not empty. Or you could create a composed constraint comprising @Range and @NotEmpty.

--Gunnar

Hi,
thanks for the answer. You are right, I don't have an user account on JIRA.

A possible solution for Scenario 1 is to check that there aren't trailing 'separators'. Due to String.split method specification, they will ignore all of them. In this case "@" is the pattern for split, so we can use the String.endsWith method to ensure that. This is a simple solution and it will work for the EmailValidator:

Code:

...
if ( value == null || value.length() == 0 ) {
    return true;
}

if (value.toString().endsWith( "@" )) {
    return false;
}

// split email at '@' and consider local and domain part separately
String[] emailParts = value.toString().split( "@" );
...

But, as "@" is a regular expression that will be compiled by Pattern class, we could do it for any regular expression.

Code:

/**
 * Regular expression for email.
 */
private Pattern separator = java.util.regex.Pattern.compile( "@" );

private boolean isValid(final CharSequence value) {
    if ( value == null || value.length() == 0 ) {
            return true;
    }

    // Count the separator occurrences
    final Matcher matcher = separator.matcher(value);
    int matchCount = 0;
    while (matcher.find()) {
        matchCount++;
    }
    if (matchCount != 1) {
        return false;
    }

    // split email at '@' and consider local and domain part separately
    String[] emailParts = value.toString().split(separator.pattern());
    ...

The second solution works for every regular expression but maybe is not necessary in this case.

Regarding Scenario 2, I agree with you about an empty string is not a valid number, I will work around with a composed or a custom constraint.

Thanks for the explanation.

-- Francisco