On 26 January 2015 at 11:47, Benedikt Ritter <brit...@apache.org> wrote: > 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. >
That really depends on what "Domain" means in this context. I'm fairly sure it does not actually mean Domain as in DNS, because: - the rules are too restrictive for DNS - DNS entry validation seems out of scope for VALIDATOR, which is more focussed on validating URIs. We need to look again at the RFCs for the validation classes that depend on DomainValidator and check that the usage is correct. And update the Javadocs where necessary to clarify the processing. >> >> > 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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
