commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: svn commit: r1654500 - in /commons/proper/validator/trunk/src: changes/ main/java/org/apache/commons/validator/routines/ test/java/org/apache/commons/validator/routines/
Date Mon, 26 Jan 2015 11:47:57 GMT
2015-01-26 12:47 GMT+01:00 Benedikt Ritter <britter@apache.org>:

>
>
> 2015-01-26 12:25 GMT+01:00 sebb <sebbaz@gmail.com>:
>
>> On 26 January 2015 at 07:30, Benedikt Ritter <britter@apache.org> wrote:
>> > Hello sebb,
>> >
>> > 2015-01-25 12:55 GMT+01:00 sebb <sebbaz@gmail.com>:
>> >
>> >> On 25 January 2015 at 11:12, sebb <sebbaz@gmail.com> wrote:
>> >> > On 25 January 2015 at 10:58, Benedikt Ritter <britter@apache.org>
>> wrote:
>> >> >> Hello sebb,
>> >> >>
>> >> >> 2015-01-24 13:16 GMT+01:00 sebb <sebbaz@gmail.com>:
>> >> >>
>> >> >>> On 24 January 2015 at 12:01,  <britter@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("abc_@abc.com"));
>> >> >>> >          assertTrue(validator.isValid("abc-def@abc.com"));
>> >> >>> >          assertTrue(validator.isValid("abc_def@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-unsubscribe@commons.apache.org
>> >> >>> For additional commands, e-mail: dev-help@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-unsubscribe@commons.apache.org
>> >> For additional commands, e-mail: dev-help@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-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message