Bug #50576 for CGI: CGI::Cookie doesn't conform to RFC2109, 'Max-Age' ( Needs peer review )

Fri Oct 16 17:15:37 2009 http://jackyf.livejournal.com/ - Ticket created

Subject:

CGI::Cookie doesn't conform to RFC2109, 'Max-Age'

From http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=64308: Conforming to RFC2109, the 'Expires' field is deprecated in favour of 'Max-Age'. Please provide both fields to conform both "old" specification and "modern" one. E.g. this one should have also 'Max-Age' "field". $ perl -MCGI::Cookie -le '$a=CGI::Cookie->new(q{-name} => 'ID', q{-expires} => "+3M"); print $a->as_string()' ID=; path=/; expires=Thu, 14-Jan-2010 19:51:06 GMT Perl version: 5.10.1. OS: Debian.

Sat Oct 17 10:50:55 2009 yanick [...] babyl.dyndns.org - Correspondence added

CC:	yanick+cpan [...] babyl.dyndns.org
Subject:	Re: [rt.cpan.org #50576] CGI::Cookie doesn't conform to RFC2109, 'Max-Age'
Date:	Sat, 17 Oct 2009 10:54:03 -0400
To:	bug-CGI.pm [...] rt.cpan.org
From:	Yanick Champoux <yanick [...] babyl.dyndns.org>

http://jackyf.livejournal.com/ via RT wrote: Show quoted text

> From http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=64308: > > Conforming to RFC2109, the 'Expires' field is deprecated in favour of > 'Max-Age'. Please provide both fields to conform both "old" > specification and "modern" one. > > E.g. this one should have also 'Max-Age' "field". > > $ perl -MCGI::Cookie -le '$a=CGI::Cookie->new(q{-name} => 'ID', > q{-expires} => "+3M"); print $a->as_string()' > ID=; path=/; expires=Thu, 14-Jan-2010 19:51:06 GMT

I've peeked into CGI::Cookie, and support for max-age seems to already be stubbed in. I'll try to dig a little deeper today and see in the history if there's a reason why it's not active yet. Cheers, `/anick

Sat Oct 17 10:50:56 2009 The RT System itself - Status changed from 'new' to 'open'

Sun Nov 01 14:33:51 2009 yanick+cpan [...] babyl.dyndns.org - Correspondence added

A tentative patch is at http://github.com/yanick/CGI.pm/tree/rt-50576 Points of discussion/caveats: * By default, the cookie now prints both 'Expires' and 'Max-Age' fields. Their display can be controlled via the show_expires/show_max_age methods or the -show_expires/-show_max_age new() arguments. * The time to live can now be set using max_age() or expires(). They both modify the same underlaying value. * If the time to live is specified as a date string, the module Date::Parse is used to convert it into a timestamp. * CGI.pm has not been modified to follow the same convention (yet).

Wed Nov 18 22:13:41 2009 MARKSTOS [...] cpan.org - Taken

Fri Nov 20 12:53:54 2009 cpan [...] chmrr.net - Correspondence added

On Sun Nov 01 14:33:51 2009, YANICK wrote: Show quoted text

> A tentative patch is at http://github.com/yanick/CGI.pm/tree/rt-50576

Mark asked me to peer-review this patchset in order to help get a release of CGI.pm out the door. It looks pretty good to me, with one caveat, below. Show quoted text

> * By default, the cookie now prints both 'Expires' and 'Max-Age' fields. > Their display can be controlled via the show_expires/show_max_age > methods or the -show_expires/-show_max_age new() arguments.

I somewhat agree with Mark on this -- I'm not convinced that such fine-grained control is necessary. In my mind, sending both is unlikely to ever cause problems. Show quoted text

> * If the time to live is specified as a date string, the module > Date::Parse is used to convert it into a timestamp.

This _is_ going to possibly cause problems -- Date::Parse isn't in core, but CGI.pm is. The options which provide similar functions, which _are_ in core, are Time::Piece->strptime or Time::Local::timegm. Hauling in Time::Piece seems heavier-weight, but the implementation will be easier. Thoughts? - Alex

Fri Nov 20 13:13:13 2009 yanick [...] babyl.dyndns.org - Correspondence added

Subject:	Re: [rt.cpan.org #50576] CGI::Cookie doesn't conform to RFC2109, 'Max-Age'
Date:	Fri, 20 Nov 2009 13:12:47 -0500
To:	bug-CGI.pm [...] rt.cpan.org
From:	Yanick Champoux <yanick [...] babyl.dyndns.org>

On November 20, 2009 12:53:55 pm Alex Vandiver via RT wrote: Show quoted text

> > * By default, the cookie now prints both 'Expires' and 'Max-Age' fields. > > Their display can be controlled via the show_expires/show_max_age > > methods or the -show_expires/-show_max_age new() arguments.

> > I somewhat agree with Mark on this -- I'm not convinced that such > fine-grained control is necessary. In my mind, sending both is unlikely > to ever cause problems.

I also agree. I couldn't think of an example where having both would cause harm, buuut I thought that giving peeps the control if they so desire was the ultimate insurance against gotchas. But I agree, we can always take that out. Show quoted text

> > * If the time to live is specified as a date string, the module > > Date::Parse is used to convert it into a timestamp.

> > This _is_ going to possibly cause problems -- Date::Parse isn't in core, > but CGI.pm is.

Very good point. Show quoted text

> The options which provide similar functions, which _are_ > in core, are Time::Piece->strptime or Time::Local::timegm. Hauling in > Time::Piece seems heavier-weight, but the implementation will be easier. > Thoughts?

Time::Piece would be good. And this will only happen in the weird cases where someone would enter the expiry date as an absolute date instead of a delta. I'll be offline for the next 3 days, but I'll try to bring in those changes asap. Or any of you can fork my github branch and play with it, if you so desire. :-) Thanks! `/anick

Tue Nov 24 00:20:47 2009 cpan [...] chmrr.net - Cc ALEXMV added

Sun Nov 29 20:07:22 2009 yanick [...] babyl.dyndns.org - Correspondence added

Subject:	Re: [rt.cpan.org #50576] CGI::Cookie doesn't conform to RFC2109, 'Max-Age'
Date:	Sun, 29 Nov 2009 20:09:35 -0500
To:	bug-CGI.pm [...] rt.cpan.org
From:	Yanick Champoux <yanick [...] babyl.dyndns.org>

Alex Vandiver via RT wrote: Show quoted text

>> * By default, the cookie now prints both 'Expires' and 'Max-Age' fields. >> Their display can be controlled via the show_expires/show_max_age >> methods or the -show_expires/-show_max_age new() arguments.

> > I somewhat agree with Mark on this -- I'm not convinced that such > fine-grained control is necessary. In my mind, sending both is unlikely > to ever cause problems.

Both arguments removed. If an expire/max-age has been specified, we put both fields in the header (and if nothing has been specified, we output nothing). Show quoted text

>> * If the time to live is specified as a date string, the module >> Date::Parse is used to convert it into a timestamp.

> > This _is_ going to possibly cause problems -- Date::Parse isn't in core, > but CGI.pm is. The options which provide similar functions, which _are_ > in core, are Time::Piece->strptime or Time::Local::timegm. Hauling in > Time::Piece seems heavier-weight, but the implementation will be easier. > Thoughts?

I've switched Date::Parse for Time::Piece. The big difference is that Time::Piece requires its string to be in a very specific format, whereas D::P would do its damnedest to figure out what the user want. Before we didn't have this problem because we would just output verbatim an recognized string -- something we can't do anymore before we have to compute the time delta for max-age. That shouldn't be too much of an issue, hopefully. I expect that very few people use absolute dates for their expire data. `/anick

Wed Dec 02 23:13:52 2009 MARKSTOS [...] cpan.org - Correspondence added

I've looked at some more. Some notes: - The original bug report from the year 2000. CGI::Cookie got max-age support in September 2002, in the 2.83 release. - When researching this, I came across an article I'd written about it myself: http://mark.stosberg.com/blog/2008/12/cookie-handling-in- titanium-catalyst-and-mojo.html - The Max-Age support currently in CGI.pm can be considered incomplete in the sense that it has a "documentation bug". Using max-age is not documented, and there is nothing to inform the user that max-age is the header used in the spec, or advise about which header to use in terms of practical browser compatibility. Since CGI.pm is a low-level library for a number of projects, I have some inclination to provide users very fine grained support: If use you 'expires' you set the Expires header, and if you use "max_age", you set the Max-Age header. If you want them both, you should set them both. Clearly, setting just "Expires" works fine for pratical cases. The downstream bug report is only complaining that we don't technically follow the spec... it's not reporting that any known browser actually recognizes only Max-Age and not Expires. This approach would be perfectly backwards compatible, and saves a bit processing power to generate both headers when really only one is desired. Any of the projects built-on CGI.pm can choose to provide a way to set both headers at once if they want to. What I'm proposing has the fine-grained control of Yanick's original proposal, but I think is a simpler way to accomplish it. I think always setting both headers might introduce backwards compatibility issues in some cases simply by being a different behavior, even if it shouldn't normally cause a problem.

Sat Dec 25 22:03:05 2010 MARKSTOS [...] cpan.org - Correspondence added

Subject:

for final review, new 'Max-Age' implementation

Yanick, I have taken a second try at the Max-Age support. It is available here: https://github.com/markstos/CGI.pm/tree/max-age-cookie-take-2 I manually incorporated some of the ideas and code from the original branch where it applied. I started over because as the ticket reflects, the line of thinking changed. Originally, we worked on managing the two headers together. The current proposed approach is to manage them individually. This is completely backwards compatible, and gives the flexibilities to users or frameworks to use their own method manage the fields together if they prefer. If the changes and docs look OK to use, please go ahead and pull the work to my "master" branch and let's consider this done.

Sun Jan 23 20:47:27 2011 MARKSTOS [...] cpan.org - Told changed from Sat Dec 25 22:03:06 2010 to Sun Jan 23 20:47:27 2011

Sun Jan 23 20:48:15 2011 MARKSTOS [...] cpan.org - Correspondence added

RT-Send-CC:

yanick [...] babyl.dyndns.org

( If anyone else besides Yanick wants to provide a peer review, that's always welcome, too. )

Sun Jan 23 20:48:38 2011 MARKSTOS [...] cpan.org - Subject changed from 'CGI::Cookie doesn't conform to RFC2109, 'Max-Age'' to 'CGI::Cookie doesn't conform to RFC2109, 'Max-Age' ( Needs peer review )'

Thu May 22 08:05:10 2014 LEEJO [...] cpan.org - Correspondence added

This issue has been copied to: https://github.com/leejo/CGI.pm/issues/60 please take all future correspondence there. This ticket will remain open but please do not reply here. This ticket will be closed when the github issue is dealt with.

Fri May 23 14:28:04 2014 The RT System itself - Queue changed from CGI.pm to CGI

Wed Jun 18 03:37:28 2014 LEEJO [...] cpan.org - Correspondence added

Resolved, will go out in next release to CPAN. Thanks. commit 7d46fae0f0baed36488d1bd4e4e05c3505f14008 Author: Lee Johnson <lee@givengain.ch> Date: Wed Jun 18 09:30:48 2014 +0200 resolve #60 [rt.cpan.org #50576] - cookie -max-age argument to CGI::Cookie is now correctly set when this is passed in the constructor. previously the value from the -expires argument was being used, meaning it was not possible to pass *only* the -max-age argument and have this set. this also means that the values for the -max-age and -expires arguments can be different, should you want to do something like that update tests to reflect above change and increase coverage for the -max-age constructor argument / max_age method. also update perldoc adding missing documentation for the max_age method

Wed Jun 18 03:37:31 2014 LEEJO [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #50576 for CGI: CGI::Cookie doesn't conform to RFC2109, 'Max-Age' ( Needs peer review )

Preferred bug tracker