Bug #76179 for Catalyst-Plugin-Session-State-Cookie: t/live_app.t fails with Catalyst-Runtime >= 5.90008

Sat Mar 31 01:13:15 2012 ntyni [...] iki.fi - Ticket created

Subject:

t/live_app.t fails with Catalyst-Runtime >= 5.90008

Hi, as reported in <http://bugs.debian.org/665222>, t/live_app.t is failing with recent versions (>= 5.90008) of Catalyst-Runtime. Bisecting says 9c4288eaedee0bba8e4a123b4664bc07a4b5c24c is the first bad commit commit 9c4288eaedee0bba8e4a123b4664bc07a4b5c24c Author: Tomas Doran <bobtfish@bobtfish.net> Date: Mon Oct 31 18:32:25 2011 +0000 Move write and finalize_headers into response object The commit can be seen at http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits/Catalyst-Runtime.git;a=commit;h=9c4288eaedee0bba8e4a123b4664bc07a4b5c24c This isn't showing up on CPAN Testers because all the recent reports say t/live_app.t ....... skipped: This test requires Test::WWW::Mechanize::Catalyst in order to run Relevant test output with that module installed: t/live_app.t ....... 1/? # Failed test 'session data restored' # at t/live_app.t line 31. # searched: "Hi! hit number 1" # can't find: "hit number 2" # LCSS: "hit number " # LCSS context: "Hi! hit number 1" # Failed test 'session data restored' # at t/live_app.t line 34. # searched: "hit number 2" # can't find: "hit number 3" # LCSS: "hit number " # LCSS context: "hit number 2" t/live_app.t ....... 7/? # Failed test 'session data restored' # at t/live_app.t line 39. # searched: "hit number 3" # can't find: "hit number 4" # LCSS: "hit number " # LCSS context: "hit number 3" Use of uninitialized value $got in numeric lt (<) at (eval in cmp_ok) t/live_app.t line 43. # Failed test 'session data restored (blah)' # at t/live_app.t line 47. # searched: "Hi! hit number 4" # can't find: "hit number 5" # LCSS: "hit number " # LCSS context: "Hi! hit number 4" t/live_app.t ....... 12/? # Failed test 'session data restored' # at t/live_app.t line 52. # searched: "hit number 5" # can't find: "hit number 6" # LCSS: "hit number " # LCSS context: "hit number 5" # Looks like you failed 5 tests of 22. t/live_app.t ....... Dubious, test returned 5 (wstat 1280, 0x500) Failed 5/22 subtests t/no_new_method.t .. ok Test Summary Report ------------------- t/live_app.t (Wstat: 1280 Tests: 22 Failed: 5) Failed tests: 4, 6, 8, 11, 13 Non-zero exit status: 5 Files=6, Tests=37, 4 wallclock secs ( 0.03 usr 0.01 sys + 1.75 cusr 0.08 csys = 1.87 CPU) Result: FAIL Failed 1/6 test programs. 5/37 subtests failed. Thanks for your work, -- Niko Tyni (Debian Perl Group) ntyni@debian.org

Sat Jun 23 16:00:17 2012 intrigeri [...] boum.org - Correspondence added

On Sat Mar 31 01:13:15 2012, ntyni@iki.fi wrote: Show quoted text

> Test Summary Report > ------------------- > t/live_app.t (Wstat: 1280 Tests: 22 Failed: 5) > Failed tests: 4, 6, 8, 11, 13 > Non-zero exit status: 5 > Files=6, Tests=37, 4 wallclock secs ( 0.03 usr 0.01 sys + 1.75 cusr > 0.08 csys = 1.87 CPU) > Result: FAIL > Failed 1/6 test programs. 5/37 subtests failed.

I could reproduce this with Catalyst 5.90012 and 5.90013.

Sat Jun 23 16:00:20 2012 The RT System itself - Status changed from 'new' to 'open'

Mon Jun 25 03:00:11 2012 ARODLAND [...] cpan.org - Correspondence added

The part of this commit that's to blame is that in 5.90007, $c->write called $c->finalize_headers before sending any body. In 5.90008, $c- Show quoted text

>write calls $c->response->write, which calls $c->response- >finalize_headers. This breaks Session because Session hooks $c- >finalize_headers to extend the session expiration and send the session

cookie. A partial fix is to have Catalyst::write call finalize_headers (which will call the Response's finalize_headers in turn, so the finalize_headers call can be removed from Response). Unfortunately this leaves Catalyst::Response::write broken. As far as I can tell, the only way to fix *that* is to re-introduce the _context ref to Response so that Catalyst::Response::write can call $self->_context- Show quoted text

>finalize_headers (reverting 684ca75d81f91dc5302f1654d7029c93be4f5a37).

Tue Jun 26 12:03:18 2012 intrigeri [...] boum.org - Correspondence added

On Mon Jun 25 03:00:11 2012, ARODLAND wrote: Show quoted text

> As far as I can tell, the only > way to fix *that* is to re-introduce the _context ref to Response so > that Catalyst::Response::write can call $self->_context-

> >finalize_headers (reverting 684ca75d81f91dc5302f1654d7029c93be4f5a37).

Thanks for you detailed analysis. Any chance the solution you're suggesting happens?

Tue Jun 26 12:34:38 2012 jjn1056 [...] yahoo.com - Correspondence added

Subject:	Re: [rt.cpan.org #76179] t/live_app.t fails with Catalyst-Runtime >= 5.90008
Date:	Tue, 26 Jun 2012 12:34:24 -0400
To:	"bug-Catalyst-Plugin-Session-State-Cookie [...] rt.cpan.org" <bug-Catalyst-Plugin-Session-State-Cookie [...] rt.cpan.org>
From:	John Napiorkowski <jjn1056 [...] yahoo.com>

The recent catalyst release 5.90014 applies this patch. Please give it a try! Sent from my iPhone On Jun 26, 2012, at 12:03 PM, "intrigeri via RT" <bug-Catalyst-Plugin-Session-State-Cookie@rt.cpan.org> wrote: Show quoted text

> Queue: Catalyst-Plugin-Session-State-Cookie > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=76179 > > > On Mon Jun 25 03:00:11 2012, ARODLAND wrote:

>> As far as I can tell, the only >> way to fix *that* is to re-introduce the _context ref to Response so >> that Catalyst::Response::write can call $self->_context-

>>> finalize_headers (reverting 684ca75d81f91dc5302f1654d7029c93be4f5a37).

> > Thanks for you detailed analysis. Any chance the solution you're > suggesting happens? >

Wed Jun 27 19:18:28 2012 intrigeri [...] boum.org - Correspondence added

On Tue Jun 26 12:34:38 2012, jjn1056@yahoo.com wrote: Show quoted text

> The recent catalyst release 5.90014 applies this patch. Please give > it a try!

Unfortunately, Catalyst 5.90014 does not fix this bug, and it breaks the Catalyst-Plugin-StackTrace 0.11 test suite :/

Sat Jun 30 14:06:12 2012 bobtfish [...] bobtfish.net - Correspondence added

Subject:	Re: [rt.cpan.org #76179] t/live_app.t fails with Catalyst-Runtime >= 5.90008
Date:	Sat, 30 Jun 2012 19:06:01 +0100
To:	bug-Catalyst-Plugin-Session-State-Cookie [...] rt.cpan.org
From:	Tomas Doran <bobtfish [...] bobtfish.net>

On 28 Jun 2012, at 00:18, intrigeri via RT wrote: Show quoted text

> Queue: Catalyst-Plugin-Session-State-Cookie > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=76179 > > > On Tue Jun 26 12:34:38 2012, jjn1056@yahoo.com wrote:

>> The recent catalyst release 5.90014 applies this patch. Please give >> it a try!

> > Unfortunately, Catalyst 5.90014 does not fix this bug, > and it breaks the Catalyst-Plugin-StackTrace 0.11 test suite :/ >

<sigh> Sorry about that. Catalyst 5.90015 has been tested with both of these and appears to fix the issues. Cheers t0m

Fri Jul 13 00:20:58 2012 intrigeri [...] boum.org - Correspondence added

On Sat Jun 30 14:06:12 2012, BOBTFISH wrote: Show quoted text

> Sorry about that. > > Catalyst 5.90015 has been tested with both of these and appears to fix > the issues.

Thanks a lot! We'll try to get that release into Debian Wheezy (see https://lists.debian.org/debian-perl/2012/07/msg00098.html for details), even if it's frozen already.

Fri Jul 13 08:47:30 2012 bobtfish [...] bobtfish.net - Status changed from 'open' to 'resolved'