You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Vincent Massol <vm...@octo.com> on 2002/02/17 00:54:16 UTC
[httpclient] New change to Cookie.java breaks Cactus
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.ja
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) ?
2/ Is HttpClient going to preserve a backward compatibility or should I
change the Cactus code ?
Thanks
-Vincent
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [httpclient] New change to Cookie.java breaks Cactus
Posted by dIon Gillard <di...@multitask.com.au>.
Thanks,
I think I've already closed the bug :)
Vincent Massol wrote:
>It needs to work fine now :-)
>
>Sorry for the delay in my answer but there were other bugs I had to
>resolve before knowing your fix was working !
>
>Thanks
>-Vincent
>
>>-----Original Message-----
>>From: dIon Gillard [mailto:dion@multitask.com.au]
>>Sent: 19 February 2002 23:33
>>To: Jakarta Commons Developers List
>>Subject: Re: [httpclient] New change to Cookie.java breaks Cactus
>>
>>Hi Vincent,
>> can you test this one for me, and let me know if all is now ok??
>>
>>
>>Vincent Massol wrote:
>>
>>>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
>>>
>>--
>>dIon
>>
--
dIon Gillard, Multitask Consulting
http://adslgateway.multitask.com.au/developers
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [httpclient] New change to Cookie.java breaks Cactus
Posted by Vincent Massol <vm...@octo.com>.
It needs to work fine now :-)
Sorry for the delay in my answer but there were other bugs I had to
resolve before knowing your fix was working !
Thanks
-Vincent
> -----Original Message-----
> From: dIon Gillard [mailto:dion@multitask.com.au]
> Sent: 19 February 2002 23:33
> To: Jakarta Commons Developers List
> Subject: Re: [httpclient] New change to Cookie.java breaks Cactus
>
> Hi Vincent,
> can you test this one for me, and let me know if all is now ok??
>
>
> Vincent Massol wrote:
>
> >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
> >
> --
> dIon
>
>
> --
> 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: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [httpclient] New change to Cookie.java breaks Cactus
Posted by dIon Gillard <di...@multitask.com.au>.
Hi Vincent,
can you test this one for me, and let me know if all is now ok??
Vincent Massol wrote:
>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
>
--
dIon
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [httpclient] New change to Cookie.java breaks Cactus
Posted by Vincent Massol <vm...@octo.com>.
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: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [httpclient] New change to Cookie.java breaks Cactus
Posted by dIon Gillard <di...@multitask.com.au>.
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.ja
>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: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>