commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vincent Massol" <vmas...@octo.com>
Subject RE: [httpclient] New change to Cookie.java breaks Cactus
Date Sun, 17 Feb 2002 12:41:12 GMT
I have had a closer look at my code ... :-) and the problem is not where
I though it was. Actually, I do specify the domain for each cookie so
this is not the issue.

The issue is with the cookie "path". I do not specify any path for my
cookies. This is in conformance with the Cookie class that has a
constructor that do not take a path parameter (i.e. it is not mandatory
and can be null).

Actually the Cookie.matches() method allows for null paths.

However, it seems the newly introduced compare() method breaks this as
it does not check for a null path !

Also, in createCookieHeader() method (around line 474), there is :

        Set addedCookies = new TreeSet(cookies[0]);
        for(int i=0;i<cookies.length;i++) {
            if(cookies[i].matches(domain,port,path,secure,now)) {
                addedCookies.add(cookies[i]);
                added = true;
            }
        }

thus, it seems cookies[0] gets added twice ? Or at least, it gets
checked when it is not needed, as the add() triggers a comparison to see
if the element (Cookie) already exist in the Tree, and the
Cookie.compare() method is called.

Can someone correct this please (sorry I have no time right now. It
actually already took me some time to find this backward-compatibility
break ! :-)

I highly suggest that you start by writing a test case that creates a
Cookie with no path specified and then try to call createCookieHeader().
Thus it it will fail. Then, correct the code. This way, it won't happen
again ... :-)

Thanks
-Vincent

> -----Original Message-----
> From: dIon Gillard [mailto:dion@multitask.com.au]
> Sent: 17 February 2002 00:16
> To: Jakarta Commons Developers List
> Subject: Re: [httpclient] New change to Cookie.java breaks Cactus
> 
> Vincent Massol wrote:
> 
> >It seems a change brought on the 14/02/2002 to Cookie.java is
breaking
> >the Cactus tests, as reported by GUMP :
> >
> >     [java]     [junit] Testcase: testSendMultipleCookies took 0.025
sec
> >     [java]     [junit] 	Caused an ERROR
> >     [java]     [junit] null
> >     [java]     [junit] java.lang.NullPointerException
> >     [java]     [junit] 	at
> >org.apache.commons.httpclient.Cookie.compare(Cookie.java:513)
> >     [java]     [junit] 	at
> >java.util.TreeMap.compare(TreeMap.java:1047)
> >     [java]     [junit] 	at
> >java.util.TreeMap.put(TreeMap.java:449)
> >     [java]     [junit] 	at
> >java.util.TreeSet.add(TreeSet.java:198)
> >     [java]     [junit] 	at
>
>org.apache.commons.httpclient.Cookie.createCookieHeader(Cookie.java:477
)
> >     [java]     [junit] 	at
>
>org.apache.commons.httpclient.Cookie.createCookieHeader(Cookie.java:456
)
> >     [java]     [junit] 	at
>
>org.apache.commons.httpclient.Cookie.createCookieHeader(Cookie.java:444
)
> >     [java]     [junit] 	at
>
>org.apache.commons.httpclient.Cookie.createCookieHeader(Cookie.java:421
)
> >     [java]     [junit] 	at
>
>org.apache.cactus.client.HttpClientHelper.addCookies(HttpClientHelper.j
a
> >va;org/apache/cactus/util/log/LogAspect.java(1k):377)
> >
> >My analysis is that the previous Cookie class was more lenient WRT
the
> >cookie domain (i.e. it could be "null"). However it seems the new
> >Cookie.compare() method throws a NPE if it is null.
> >
> >Questions :
> >1/ Is this done voluntarily (i.e. force the user to always specify a
> >domain) ?
> >
> Not particularly, but it does make some sense. I can't find anywhere
in
> the RFC  that says Domain is optional.
> 
> >
> >2/ Is HttpClient going to preserve a backward compatibility or should
I
> >change the Cactus code ?
> >
> Not sure....I'd consider allowing a null domain in this case a bug. If
I
> can find a good reason for keeping null domains, I'm happy to. Either
> way, the create code should throw a nullpointerexception well before
the
> compare if it's not allowed to be null.
> 
> --
> dIon Gillard, Multitask Consulting
> http://www.multitask.com.au/developers
> 
> 
> 
> 
> --
> To unsubscribe, e-mail:   <mailto:commons-dev-
> unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:commons-dev-
> help@jakarta.apache.org>
> 




--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message