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

EmailFormatSpecifier not validating in 0.5.0beta4 #12

Closed
dsjellz opened this issue Aug 15, 2012 · 6 comments
Closed

EmailFormatSpecifier not validating in 0.5.0beta4 #12

dsjellz opened this issue Aug 15, 2012 · 6 comments

Comments

@dsjellz
Copy link

dsjellz commented Aug 15, 2012

in version 0.5.0beta4 the EmailFormatSpecifier is not properly checking if the email address is valid. According to the javadoc for InternetAddress: http://javamail.kenai.com/nonav/javadocs/javax/mail/internet/InternetAddress.html#InternetAddress(java.lang.String)

the constructor being used defaults strict mode to false. In order for to enforce RFC822 syntax, the second parameter in the constructor needs to be set to true. ie:

new InternetAddress(instance.textValue(), true);

@fge
Copy link
Collaborator

fge commented Aug 16, 2012

Good catch!

Just to be sure, can you give an example of a mail address which passes validation and should not pass it? Just for tests, and adding an appropriate comment in the code? I'd appreciate a lot!

@dsjellz
Copy link
Author

dsjellz commented Aug 16, 2012

Sure. while using 'new InternetAddress(instance.textValue())' the address 'testaddress123' passes as a valid address.
However, if we change this to 'new InternetAddress(instance.textValue(), true)', validation will fail as expected.

@fge
Copy link
Collaborator

fge commented Aug 16, 2012

Hmmwait, RFC 822 does not specify anywhere that an email should have a domain part! As such, this "true" parameter does not really enforce RFC 822, just a "canonical" form for an email address.

As such, I do not see it as a bug. I know that 90+% people now think that "testaddress123" is not a valid email address -- in which they are mistaken.

The "workaround" is to:

{
    "format": "email",
    "pattern": "@"
}

since @ is not a valid character in a username part.

That is how I read the RFC. If an authoritative source on that tells otherwise, I'd like a link to it.

@fge
Copy link
Collaborator

fge commented Aug 16, 2012

OK, since then I have reconsidered the matter at hand.

The problem you raise not only applies to the "email" format specifier, but also the "hostname" format specifier. In a similar vein, strictly speaking, "foo" is a valid hostname -- and email address.

However, I can only admit that most, if not all, use cases of JSON Schema will instinctively expect fully qualified hostnames and/or email addresses. I will therefore switch the default behaviour accordingly.

Expect a commit soon. And sorry for my bluntness ;)

@fge
Copy link
Collaborator

fge commented Aug 17, 2012

Could you review the aforementioned commit? Tests have been adapted accordingly.

While this makes me uneasy wrt strict RFC conformance, you have me convinced that this will turn out to be the better choice.

fge added a commit that referenced this issue Aug 17, 2012
…CHANGE)

If RFCs are strictly observed, emails may have no domain parts and host names
may not be fully qualified. However, this contradicts the general perception,
and 99+% use cases in the process.

From now on, this implementation expects hostnames to be fully qualified, and
email addresses to have a domain part.

This fixes issue #12.

v2: also modify javadoc for HostnameFormatSpecifier

Signed-off-by: Francis Galiegue <fgaliegue@gmail.com>
@fge
Copy link
Collaborator

fge commented Aug 17, 2012

Done.

@fge fge closed this as completed Aug 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants