Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 50576
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: MARKSTOS [...] cpan.org
Requestors: jackyf.devel [...] gmail.com
Cc: cpan [...] chmrr.net
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)



Subject: CGI::Cookie doesn't conform to RFC2109, 'Max-Age'
Download (untitled) / with headers
text/plain 472b
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.
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>
Download (untitled) / with headers
text/plain 718b
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
Download (untitled) / with headers
text/plain 600b
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).
On Sun Nov 01 14:33:51 2009, YANICK wrote: Show quoted text
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
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>
Download (untitled) / with headers
text/plain 1.4k
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
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>
Download (untitled) / with headers
text/plain 1.5k
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
Download (untitled) / with headers
text/plain 1.7k
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.
Subject: for final review, new 'Max-Age' implementation
Download (untitled) / with headers
text/plain 733b
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.
RT-Send-CC: yanick [...] babyl.dyndns.org
( If anyone else besides Yanick wants to provide a peer review, that's always welcome, too. )
Download (untitled) / with headers
text/plain 240b
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.
Download (untitled) / with headers
text/plain 827b
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


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.