On Tue Jan 18 01:18:32 2011, OLIVER wrote:
Show quoted text> On Tue Jan 18 01:14:54 2011, OLIVER wrote:
> > This seems to be a conscious decision on my part as in the code it
> > checks for 5xx and aborts
> > if that's the case. And as I wrote this code a few years ago now I
> > cannot remember what the
> > reason for that was. A lesson in needing better code commenting, I
> > suppose!
>
> Replying to myself, I wonder whether the reason is because the cookie
> data might not have
> been fully set by the application. For example, the cookie is prepared
> in the catalyst $c object
> but still needs some kind of data adding to it at the point the app
> throws a 5xx. Perhaps the
> user doesn't want that half created cookie to be baked and signed.
Firstly - have been running with it commented out for today, and it
hasn't caused any issues. I don't really do any hacking in core though,
so I haven't been triggering 500s outside of my own application code.
I have been implementing session-related stuff (sudo to other users),
and thus have been triggering various session-related errors
deliberately, and each time I've wanted the cookie to be signed.
Show quoted text>
> What's your opinion, as a user? Presumably your app doesn't get into
> this situation. Or maybe
> we can do something else like not re-bake a previously signed cookie
> (i.e. remove) if doing a
> 5xx so it's more like a no-op.
Even if the cookie was half-completed when the 500 occurred, I don't
think it's a reason to not sign it. If the application is coded
correctly, it should handle the half-baked cookie case gracefully, and
even if it doesn't, I don't think it's the purpose of this module to
decide not to sign it.
I feel that the module should be as transparent as it can, so that the
behaviour is the same both when it's enabled and when it's not. Without
this module, the cookie would be set anyway.
I suppose it could be a no-op, but then I also don't see the point. It
seems like more code for no great benefit.
One caveat to my feedback: I don't know much about Catalyst's inner
workings. So maybe there is a time when a 500 occurs that not signing
the cookie makes sense. I defer to your judgement about what to do if
that's the case, though my preference would be for the module to sign
cookies if the 500 was caused by the application rather than Catalyst
itself.
Cheers,
Nigel
