commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
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:25:30 GMT
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.

> 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


Mime
View raw message