2015-01-26 12:47 GMT+01:00 Benedikt Ritter <brit...@apache.org>: > > > 2015-01-26 12:25 GMT+01:00 sebb <seb...@gmail.com>: > >> On 26 January 2015 at 07:30, Benedikt Ritter <brit...@apache.org> wrote: >> > Hello sebb, >> > >> > 2015-01-25 12:55 GMT+01:00 sebb <seb...@gmail.com>: >> > >> >> On 25 January 2015 at 11:12, sebb <seb...@gmail.com> wrote: >> >> > On 25 January 2015 at 10:58, Benedikt Ritter <brit...@apache.org> >> wrote: >> >> >> Hello sebb, >> >> >> >> >> >> 2015-01-24 13:16 GMT+01:00 sebb <seb...@gmail.com>: >> >> >> >> >> >>> On 24 January 2015 at 12:01, <brit...@apache.org> wrote: >> >> >>> > Author: britter >> >> >>> > Date: Sat Jan 24 12:01:20 2015 >> >> >>> > New Revision: 1654500 >> >> >>> > >> >> >>> > URL: http://svn.apache.org/r1654500 >> >> >>> > Log: >> >> >>> > VALIDATOR-358: Underscores in domain names are not supported. >> This >> >> fixes >> >> >>> #3 from github. Thanks to Nykolas Laurentino de Lima. >> >> >>> >> >> >>> -1 >> >> >>> >> >> >>> This is not supported by the RFCs I have seen. >> >> >>> >> >> >>> AFAICT underscore is supported in DNS labels. >> >> >>> >> >> >> >> >> >> I've looked at RCF2181 [1] - Clarifications to the DNS >> Specification, >> >> >> section 11 - Name syntax: >> >> >> >> >> >> "The DNS itself places only one restriction on the particular labels >> >> that >> >> >> can be used to identify resource records. That one restriction >> relates >> >> to >> >> >> the length of the label and the full name. [...] Implementations of >> the >> >> DNS >> >> >> protocols must not place any restrictions on the labels that can be >> >> used. >> >> >> In particular, DNS servers must not refuse to serve a zone because >> it >> >> >> contains labels that might not be acceptable to some DNS client >> >> programs." >> >> >> >> >> >> What am I missing? >> >> > >> >> > What I wrote below. >> >> > >> >> >> Benedikt >> >> >> >> >> >> [1] http://www.ietf.org/rfc/rfc2181.txt >> >> >> >> >> >> >> >> >>> >> >> >>> However that does not imply they are supported in hostnames and >> URLs. >> >> > >> >> > i.e. DNS is a lot more permissive than hostnames allow. >> >> > DNS is used for more than just hostname resolution. >> >> > >> >> > >> http://en.wikipedia.org/wiki/Hostname#Restrictions_on_valid_host_names >> >> >> >> RFC 1123 extends RFC 952 to allow a leading numeric: >> >> >> >> http://tools.ietf.org/html/rfc1123#page-13 >> >> >> >> so the syntax now allows letter-or-digit in first and last characters >> >> of a label. >> >> hyphens are additionally allowed in other positions >> >> [TLDs are not allowed to start with a digit; this means 1.2.3.4 must >> >> be a numeric IP address] >> >> >> >> I'm not aware of an RFC that relaxes the hostname syntax further, but >> >> I've not done an exhaustive search. >> >> >> > >> > I've reverted the commit. >> >> Thanks! >> >> > The initial PR on github [1] was about AWS host >> > names, which can contain underscores (but are not considered valid bei >> > validator). Maybe there is a different way we can fix that? >> >> If there really is a use case for this (and it seems there is) then it >> should be easy enough to add underscore as an _option_ when performing >> validation. >> Perhaps the OP can provide an updated patch? >> >> We need to be careful not to change the validation without considering >> the relevant RFCs. >> We should fix bugs where the code does not agree with the RFCs, but we >> should not deliberately allow forbidden syntax. >> Other users may be expecting strict adherence to the RFCs. >> > > Because of this I'm not sure whether DomainValidator is the correct place > for this at all. The github issue looks more like it needs a > "HostnameValidator" or "DnsValidator" or something like that. >
To make this clear :-) I agree with you, our routines should not violate rules defined in RFCs, not even optionally. > > >> >> > Benedikt >> > >> > [1] https://github.com/apache/commons-validator/pull/2 >> > >> > >> >> >> >> >>> >> >> >>> Unless a relevant RFC shows otherwise, this commit needs to be >> >> reworked. >> >> >>> >> >> >>> Either reverted entirely, or allowing underscore must be optional, >> and >> >> >>> not allowed by default. >> >> >>> >> >> >>> > Modified: >> >> >>> > commons/proper/validator/trunk/src/changes/changes.xml >> >> >>> > >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> >> >>> > >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/UrlValidator.java >> >> >>> > >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/EmailValidatorTest.java >> >> >>> > >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java >> >> >>> > >> >> >>> > Modified: commons/proper/validator/trunk/src/changes/changes.xml >> >> >>> > URL: >> >> >>> >> >> >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/changes/changes.xml?rev=1654500&r1=1654499&r2=1654500&view=diff >> >> >>> > >> >> >>> >> >> >> ============================================================================== >> >> >>> > --- commons/proper/validator/trunk/src/changes/changes.xml >> (original) >> >> >>> > +++ commons/proper/validator/trunk/src/changes/changes.xml Sat >> Jan 24 >> >> >>> 12:01:20 2015 >> >> >>> > @@ -43,6 +43,9 @@ The <action> type attribute can be add,u >> >> >>> > <body> >> >> >>> > >> >> >>> > <release version="1.5.0" date="tba" description="tba"> >> >> >>> > + <action issue="VALIDATOR-356" dev="britter" type="fix" >> >> >>> due-to="Nykolas Laurentino de Lima"> >> >> >>> > + Underscores in domain names are not supported >> >> >>> > + </action> >> >> >>> > <action issue="VALIDATOR-356" dev="seb" type="fix" > >> >> >>> > IDN.toASCII drops trailing dot in Java 6 & 7 >> >> >>> > </action> >> >> >>> > >> >> >>> > Modified: >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> >> >>> > URL: >> >> >>> >> >> >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java?rev=1654500&r1=1654499&r2=1654500&view=diff >> >> >>> > >> >> >>> >> >> >> ============================================================================== >> >> >>> > --- >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> >> >>> (original) >> >> >>> > +++ >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> >> >>> Sat Jan 24 12:01:20 2015 >> >> >>> > @@ -69,7 +69,7 @@ public class DomainValidator implements >> >> >>> > >> >> >>> > // RFC2396: domainlabel = alphanum | alphanum *( alphanum >> | >> >> "-" ) >> >> >>> alphanum >> >> >>> > // Max 63 characters >> >> >>> > - private static final String DOMAIN_LABEL_REGEX = >> >> >>> "\\p{Alnum}(?>[\\p{Alnum}-]{0,61}\\p{Alnum})?"; >> >> >>> > + private static final String DOMAIN_LABEL_REGEX = >> >> >>> "\\p{Alnum}(?>[\\p{Alnum}-_]{0,61}\\p{Alnum})?"; >> >> >>> > >> >> >>> > // RFC2396 toplabel = alpha | alpha *( alphanum | "-" ) >> alphanum >> >> >>> > // Max 63 characters >> >> >>> > >> >> >>> > Modified: >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/UrlValidator.java >> >> >>> > URL: >> >> >>> >> >> >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/UrlValidator.java?rev=1654500&r1=1654499&r2=1654500&view=diff >> >> >>> > >> >> >>> >> >> >> ============================================================================== >> >> >>> > --- >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/UrlValidator.java >> >> >>> (original) >> >> >>> > +++ >> >> >>> >> >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/UrlValidator.java >> >> >>> Sat Jan 24 12:01:20 2015 >> >> >>> > @@ -133,7 +133,7 @@ public class UrlValidator implements Ser >> >> >>> > // Drop numeric, and "+-." for now >> >> >>> > // TODO does not allow for optional userinfo. >> >> >>> > // Validation of character set is done by isValidAuthority >> >> >>> > - private static final String AUTHORITY_CHARS_REGEX = >> >> >>> "\\p{Alnum}\\-\\."; >> >> >>> > + private static final String AUTHORITY_CHARS_REGEX = >> >> >>> "\\p{Alnum}\\-\\._"; >> >> >>> > >> >> >>> > private static final String AUTHORITY_REGEX = >> >> >>> > "^([" + AUTHORITY_CHARS_REGEX + "]*)(:\\d*)?(.*)?"; >> >> >>> > >> >> >>> > Modified: >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/EmailValidatorTest.java >> >> >>> > URL: >> >> >>> >> >> >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/EmailValidatorTest.java?rev=1654500&r1=1654499&r2=1654500&view=diff >> >> >>> > >> >> >>> >> >> >> ============================================================================== >> >> >>> > --- >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/EmailValidatorTest.java >> >> >>> (original) >> >> >>> > +++ >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/EmailValidatorTest.java >> >> >>> Sat Jan 24 12:01:20 2015 >> >> >>> > @@ -468,6 +468,6 @@ public class EmailValidatorTest extends >> >> >>> > assertTrue(validator.isValid("a...@abc.com")); >> >> >>> > assertTrue(validator.isValid("abc-...@abc.com")); >> >> >>> > assertTrue(validator.isValid("abc_...@abc.com")); >> >> >>> > - assertFalse(validator.isValid("abc@abc_def.com")); >> >> >>> > + assertTrue(validator.isValid("abc@abc_def.com")); >> >> >>> > } >> >> >>> > } >> >> >>> > >> >> >>> > Modified: >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java >> >> >>> > URL: >> >> >>> >> >> >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java?rev=1654500&r1=1654499&r2=1654500&view=diff >> >> >>> > >> >> >>> >> >> >> ============================================================================== >> >> >>> > --- >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java >> >> >>> (original) >> >> >>> > +++ >> >> >>> >> >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/UrlValidatorTest.java >> >> >>> Sat Jan 24 12:01:20 2015 >> >> >>> > @@ -441,6 +441,7 @@ public class UrlValidatorTest extends Te >> >> >>> > new ResultPair("", true)}; >> >> >>> > >> >> >>> > ResultPair[] testUrlAuthority = {new ResultPair(" >> www.google.com >> >> ", >> >> >>> true), >> >> >>> > + >> new >> >> >>> ResultPair("my_domain.s3.amazonaws.com", true), >> >> >>> > new ResultPair("go.com", >> true), >> >> >>> > new ResultPair("go.au", true), >> >> >>> > new ResultPair("0.0.0.0", >> true), >> >> >>> > >> >> >>> > >> >> >>> >> >> >>> >> --------------------------------------------------------------------- >> >> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >> >>> For additional commands, e-mail: dev-h...@commons.apache.org >> >> >>> >> >> >>> >> >> >> >> >> >> >> >> >> -- >> >> >> http://people.apache.org/~britter/ >> >> >> http://www.systemoutprintln.de/ >> >> >> http://twitter.com/BenediktRitter >> >> >> http://github.com/britter >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> >> >> >> > >> > >> > -- >> > http://people.apache.org/~britter/ >> > http://www.systemoutprintln.de/ >> > http://twitter.com/BenediktRitter >> > http://github.com/britter >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> > > > -- > http://people.apache.org/~britter/ > http://www.systemoutprintln.de/ > http://twitter.com/BenediktRitter > http://github.com/britter > -- http://people.apache.org/~britter/ http://www.systemoutprintln.de/ http://twitter.com/BenediktRitter http://github.com/britter
