[Catalyst] Catalyst::Plugin::Session::State::Cookie and HttpOnly flag

[Scott Thomson]

On Thu, Apr 16, 2009 at 5:47 PM, Scott Thomson <smoothhound at gmail.com> wrote:
> On Tue, Apr 14, 2009 at 7:50 PM, Tomas Doran <bobtfish at bobtfish.net> wrote:
>>
>> On 14 Apr 2009, at 10:58, Scott Thomson wrote:
>>>
>>> Currently Catalyst::Plugin::Session::State::Cookie doesn't allow
>>> configuration of the HttpOnly flag, it looks trivial to add, so
>>> basically I'm wondering whether this idea has been discussed and
>>> discounted before and if there is any reason why I shouldn't just
>>> patch it?
>>>
>>
>> No reason I can think of right now.
>>
>> Patches with tests always welcome.
>>
>> Cheers
>> t0m
>
> OK - I had a look through the various components to figure out how to
> do this and it is not as simple as I first thought as Catalyst::Engine
> uses CGI::Simple::Cookie to create cookies which doesn't support the
> HttpOnly flag.
>
> So I have locally patched CGI::Cookie::Simple,
> Catalyst::Plugin::Session::State::Cookie and Catalyst::Engine and it
> all seems to work. So my plan is first to send the patch to the
> CGI::Simple maintainer and if it looks like it will go in, send the
> Catalyst patches - with tests! :) - here.
>
> Cheers,
>
> Scott

CGI::Simple::Cookie 1.109 (soon to hit the mirrors) now contains
support for HttpOnly. My question now is about the tests for the
Catalyst components, I've had a dig through them and I can't see where
I would add update them.

In Catalyst the only appropriate existing test file is
t/live_engine_request_cookies.t which tests the content (the key
values) of the cookies but not any of the other config, 'path',
'secure' etc.

In C::P::Session::State::Cookie the tests are only concerned with the
correct save/restore of the session data, again nothing about the
config.

Is the assumption that CGI::Simple::Cookie manages that side of things
so we don't need to test it in any consumers? or are there missing
tests? I'm not trying to get out of supplying tests btw :)

Cheers,

Scott