hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Samit Jain <jain.sa...@gmail.com>
Subject Re: cookie2 code review and suggestions
Date Mon, 12 Sep 2005 22:03:07 GMT
Hi Oleg,

I just committed the cookie attribute handler patch. Feel free to review
> changes made to the RFC2965 spec and suggest modifications you deem
> necessary. In particular see if CookieSource is a good name for this
> class. I am not a native English speaker and just could not think of a
> better name.

I was myself thinking of a good name but could not come up with something 
better than CookieSource. May be CookieData :)

On my part I would like to suggest some further changes to the
> CookieAttributeHandler interface. I am not entirely sure
> CookieAttributeHandler#format(StringBuffer, Cookie) method brings much.
> In essence all cookie attribute handlers implement exactly the same
> logic and simply delegate the job to the ParameterFormatter class. I can
> hardly think of any reasons to override the #format method for any
> individual attributes (custom expiry date formatting might be one, but
> this attribute should no longer be used). It seems this extra level of
> abstraction adds more complexity without any tangible benefits. The fact
> that CookieAttributeHandler#format cannot be used polymorphically
> highlights the problem with this method. I suggest we do away with it.

I think you are right. CookieAttributeHandler#format doesn't really bring 
much presently. The only reason I had it in there is for other specs that 
might require more complex cookie formatting and to contain changes in RFC 
2965 cookie formatting in one place. I will work on its removal and clean up 
some other stuff. May be we can keep this method in CookieAttributehandler 
but have an empty implementation for it in RFC 2965. I will also be adding 
cookieMatch(Cookie, Cookie) method for different specs in the same 
changelist. I should be done with it by tomorrow.

thanks for your comments.


> On Tue, 2005-08-30 at 08:05 +0000, Samit Jain wrote:
> > Hi everyone,
> >
> > I would really appreciate if you could review the code for RFC 2965
> > implementation and give your valuable comments. Especially, there are 
> some
> > TODO items in RFC2965Spec class which still need to be resolved (listed
> > below):
> >
> > //TODO(jain): when do we consider the header malformed and stop 
> processing
> > it? parse()
> > Presently if there is any error in parsing cookie attributes, we ignore 
> the
> > cookie and process other cookies in header. The response header is
> > considered malformed entirely only when the header name/value is null. 
> Are
> > there other cases while creating the cookie or parsing cookie attributes
> > that we consider the header malformed and stop processing the header, 
> e.g.
> > header value too long, or contains too many illegal characters, etc. The 
> > doesn't really expound on this besides stating the fact that unknown 
> cookie
> > attributes should be ignored.
> >
> > //TODO (jain): formatting old and new style cookies in the same header
> > formatCookies()
> > We had a separate thread for this issue. We came to the conclusion that
> > cookies in the same header should be formatted similarly. Moreover, if
> > cookies to be formatted include new and old style cookies, we format as 
> per
> > the old specification. So this is a closed issue.
> >
> > //TODO (jain): how do we handle the case when domain is specified and 
> equals
> > host? Cookie2DomainAttributeHandler.parse()
> > Just want to make sure that if the domain is specified in the 
> set-cookie2
> > header, it cannot be the same as the request host.
> >
> > //TODO (jain): other versions for set-cookie2 ?
> > Currently we only allow version 1 for set-cookie2 cookies. Is this 
> correct?
> >
> > cheers,
> >
> > Samit
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message