Comments in-line.
Marc Saegesser > -----Original Message----- > From: dIon Gillard [mailto:[EMAIL PROTECTED]] > Sent: Thursday, March 07, 2002 6:08 PM > To: Jakarta Commons Developers List > Subject: Re: [httpclient] Constructing Cookies with null > domains (again) > > > Marc Saegesser wrote: > > >/* I think this should throw IAE, but I can live with it, > > * if people really think this sort of thing is useful. > > */ > >cookies[0] = new Cookie(null, "name0", "value0"); > > > It's not that we think it's useful - it's being done now, so > changing it > breaks people's code. I conceed the point. I think this is bad practice, but changing it now causes too many other problems. > >/* While this seems valid, I don't like that we can construct > > * a cookie with no path set. Cookies should always have a > > * path (just like a domain) otherwise they are of little use. > > * We'll never be able to send them. > > */ > >cookies[1] = new Cookie("fubuar.com", "name1", "value1"); > > > See later comments. > > >/* This one makes sense */ > >cookies[2] = new Cookie("fubar.com", "name2", "value2", > > "/path", null, false) > > > >/* This *must* fail. I'd prefer throwing IAE, but NPE > > * would be OK. The purpose of createCookieHeader is > > * to build a cookie header that contains all the cookies > > * from the given array that match the given domain and > > * path as described in the specification. If you aren't > > * given a domain or a path you can't create the header. > > */ > >Cookie.createCookieHeader(null, null, cookies); > > > Agreed. The domain and path methods to create cookie header > shouldn't be > null. > > >/* If we allow the creation of Cookies without domains > > * or paths, then we *must* prevent them from being sent > > * to a server. Given the three cookies created above, > > * only cookie[2] would be added to the header. The domain > > * for cookie[0] doesn't match (it's null) and the path > > * for cookie[1] doesn't match (it's null). > > */ > >Cookie.createCookieHeader("fubar.com", "/path/path", cookies); > > > Null should match *any* path or domain. This is the way match > works at > the moment. NO! THIS IS A BUG!. > >I'll give up on throwing exceptions in the Cookie > constructors. People can > >create their own cookies with the domain or path null. I'll > fix the Cookie > >class so that it won't barf when it sees these cookies, but > they'll never > > > Where does the class fail at the moment? I thought that we'd > covered all > the cases in the tests and the only ones failing are the ones for > createCookieHeader. Is there a test case that we've missed? > > >show up in any cookie header until someone comes along and calls the > >setDomain() and setPath() methods on them to give them a > valid values. > > > Why wouldn't they show up in the header? match picks them up > ok at the > moment.... They should not show up in the header because a cookie with no domain (i.e. null) SHOULD NOT DOMAIN-MATCH ANY DOMAIN. The current implementation does the exact opposite. RFC 2109 defines only one way to create cookies, via set-cookie headers in an HTTP response. Section 4.3.1 describes how the set-cookie header is interpreted. Note that there is no way obtain a cookie that does not have an associated domain and path. Section 4.3.4 describes how to build a cookie header. Only cookies that both domain-match and path match the request host are added to the header. The domain-match algorithm is defined in section 2. I see no way that null (or empty) domain can match anything according to this algorithm. The path matching algorithm is simply that the cookie's path must be a prefix of the request path. Again, I don't see how a null (or empty) path can be treated as a prefix of anything. What you've done is create something completely outside the specification. That is, the ability to create cookies that have no domain or path information, and the magic functionality that these cookies get sent *to everyone*. This magic functionality allowed a fairly serious bug to make it through undetected. The Cookie.parse() method takes the request path as a parameter and this path (up to but not including the right-most /) should be the cookie's default path (see RFC2109/4.3.1). However, the existing implementation ignores the passed in request path and thus creates a cookie with a null path if there's no explicit path in the set-cookie header. This means that a cookie that came in response to a request for /a/b/c/fubar.html will be sent in all future requests to this host. For example it will be be sent back in a request /a/b/rabuf.html. This violates the specification and would have been caught much sooner if it wasn't for this magic extra-spec functionality. I've got a fix for this problem locally, but I'm waiting to get several new test cases written and verified before I check in any cookie changes. In my opinion, the concept of a cookie that will be sent on every request to every host is not only outside the specification, but contrary to the spirit of the specification and not something we should support. > >Marc Saegesser > > > -- > dIon Gillard, Multitask Consulting > http://adslgateway.multitask.com.au/developers > > > > > -- > To unsubscribe, e-mail: > <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
