You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by se...@apache.org on 2014/09/05 20:19:56 UTC
svn commit: r1622753 -
/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
Author: sebb
Date: Fri Sep 5 18:19:55 2014
New Revision: 1622753
URL: http://svn.apache.org/r1622753
Log:
string matching is cheaper than regex matching so do it first
Modified:
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
@@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
@Override
public boolean match(final Cookie cookie, final CookieOrigin origin) {
final String domain = cookie.getDomain();
- if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
+ if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
return false;
} else {
return handler.match(cookie, origin);
Re: svn commit: r1622753
Posted by sebb <se...@gmail.com>.
On 6 September 2014 15:35, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Sat, 2014-09-06 at 15:22 +0100, sebb wrote:
>> On 6 September 2014 15:16, Oleg Kalnichevski <ol...@apache.org> wrote:
>> > On Fri, 2014-09-05 at 19:22 +0100, sebb AT ASF wrote:
>> >> On 5 September 2014 19:19, <se...@apache.org> wrote:
>> >> > Author: sebb
>> >> > Date: Fri Sep 5 18:19:55 2014
>> >> > New Revision: 1622753
>> >> >
>> >> > URL: http://svn.apache.org/r1622753
>> >> > Log:
>> >> > string matching is cheaper than regex matching so do it first
>> >> >
>> >> > Modified:
>> >> > httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
>> >> >
>> >> > Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
>> >> > URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
>> >> > ==============================================================================
>> >> > --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
>> >> > +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
>> >> > @@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
>> >> > @Override
>> >> > public boolean match(final Cookie cookie, final CookieOrigin origin) {
>> >> > final String domain = cookie.getDomain();
>> >> > - if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
>> >> > + if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
>> >>
>> >> Having changed the order of checking, I now wonder why localhost is
>> >> being explicitly excluded here?
>> >>
>> >
>> > As far as public suffix matcher is considered 'localhost' is not a valid
>> > domain. We would either need to normalize it to 'localhost.localdomain'
>> > or handle as a special case.
>>
>> I see.
>>
>> But perhaps the special case belongs in the calling code, rather than here.
>>
>
> That is a valid point, but we would have to duplicate this check in each
> and every cookie policy that makes use of the public suffix list.
Not necessarily; there could be a common utility wrapper to add the
check for localhost.
The current special case means that the PSDFilter class may not be as
generally usable as it could be.
> Once public suffix functionality is fully integrated into HttpClient we
> can still move things around and decide whether or not handling of this
> special case belongs to individual cookie specs.
>
> Oleg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1622753
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sat, 2014-09-06 at 15:22 +0100, sebb wrote:
> On 6 September 2014 15:16, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Fri, 2014-09-05 at 19:22 +0100, sebb AT ASF wrote:
> >> On 5 September 2014 19:19, <se...@apache.org> wrote:
> >> > Author: sebb
> >> > Date: Fri Sep 5 18:19:55 2014
> >> > New Revision: 1622753
> >> >
> >> > URL: http://svn.apache.org/r1622753
> >> > Log:
> >> > string matching is cheaper than regex matching so do it first
> >> >
> >> > Modified:
> >> > httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
> >> >
> >> > Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
> >> > URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
> >> > ==============================================================================
> >> > --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
> >> > +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
> >> > @@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
> >> > @Override
> >> > public boolean match(final Cookie cookie, final CookieOrigin origin) {
> >> > final String domain = cookie.getDomain();
> >> > - if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
> >> > + if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
> >>
> >> Having changed the order of checking, I now wonder why localhost is
> >> being explicitly excluded here?
> >>
> >
> > As far as public suffix matcher is considered 'localhost' is not a valid
> > domain. We would either need to normalize it to 'localhost.localdomain'
> > or handle as a special case.
>
> I see.
>
> But perhaps the special case belongs in the calling code, rather than here.
>
That is a valid point, but we would have to duplicate this check in each
and every cookie policy that makes use of the public suffix list.
Once public suffix functionality is fully integrated into HttpClient we
can still move things around and decide whether or not handling of this
special case belongs to individual cookie specs.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1622753
Posted by sebb <se...@gmail.com>.
On 6 September 2014 15:16, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Fri, 2014-09-05 at 19:22 +0100, sebb AT ASF wrote:
>> On 5 September 2014 19:19, <se...@apache.org> wrote:
>> > Author: sebb
>> > Date: Fri Sep 5 18:19:55 2014
>> > New Revision: 1622753
>> >
>> > URL: http://svn.apache.org/r1622753
>> > Log:
>> > string matching is cheaper than regex matching so do it first
>> >
>> > Modified:
>> > httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
>> >
>> > Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
>> > URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
>> > ==============================================================================
>> > --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
>> > +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
>> > @@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
>> > @Override
>> > public boolean match(final Cookie cookie, final CookieOrigin origin) {
>> > final String domain = cookie.getDomain();
>> > - if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
>> > + if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
>>
>> Having changed the order of checking, I now wonder why localhost is
>> being explicitly excluded here?
>>
>
> As far as public suffix matcher is considered 'localhost' is not a valid
> domain. We would either need to normalize it to 'localhost.localdomain'
> or handle as a special case.
I see.
But perhaps the special case belongs in the calling code, rather than here.
> HttpClient should still accept cookies from 'localhost', shouldn't it?
I guess so.
> Oleg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1622753
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Fri, 2014-09-05 at 19:22 +0100, sebb AT ASF wrote:
> On 5 September 2014 19:19, <se...@apache.org> wrote:
> > Author: sebb
> > Date: Fri Sep 5 18:19:55 2014
> > New Revision: 1622753
> >
> > URL: http://svn.apache.org/r1622753
> > Log:
> > string matching is cheaper than regex matching so do it first
> >
> > Modified:
> > httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
> >
> > Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
> > URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
> > ==============================================================================
> > --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
> > +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
> > @@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
> > @Override
> > public boolean match(final Cookie cookie, final CookieOrigin origin) {
> > final String domain = cookie.getDomain();
> > - if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
> > + if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
>
> Having changed the order of checking, I now wonder why localhost is
> being explicitly excluded here?
>
As far as public suffix matcher is considered 'localhost' is not a valid
domain. We would either need to normalize it to 'localhost.localdomain'
or handle as a special case.
HttpClient should still accept cookies from 'localhost', shouldn't it?
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1622753 - /httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
Posted by sebb AT ASF <se...@apache.org>.
On 5 September 2014 19:19, <se...@apache.org> wrote:
> Author: sebb
> Date: Fri Sep 5 18:19:55 2014
> New Revision: 1622753
>
> URL: http://svn.apache.org/r1622753
> Log:
> string matching is cheaper than regex matching so do it first
>
> Modified:
> httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
>
> Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java?rev=1622753&r1=1622752&r2=1622753&view=diff
> ==============================================================================
> --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java (original)
> +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/cookie/PublicSuffixDomainFilter.java Fri Sep 5 18:19:55 2014
> @@ -71,7 +71,7 @@ public class PublicSuffixDomainFilter im
> @Override
> public boolean match(final Cookie cookie, final CookieOrigin origin) {
> final String domain = cookie.getDomain();
> - if (matcher.matches(domain) && !domain.equalsIgnoreCase("localhost")) {
> + if (!domain.equalsIgnoreCase("localhost") && matcher.matches(domain)) {
Having changed the order of checking, I now wonder why localhost is
being explicitly excluded here?
> return false;
> } else {
> return handler.match(cookie, origin);
>
>