allow disabling of http only cookies using strings

[ccntrq]

I had some issues disabling http_only cookies for the session cookie.

session: YAML
engines:
    session:
        YAML:
            cookie_name: session.id
            is_http_only: 0

With a config like this the HttpOnly value would always get set in the cookie header string.
The HTTP::XSCookies module seems to handle strings different from numbers and does consider a string containing only '0' to be truthy. This can be fixed by forcing boolean context on the value before passing it to the module.

I added a fix for this and a test to make sure http_only can be disabled with a string.

[bigpresh]

Hmm. I would have expected a Moose/Moo Bool attribute to automatically do the sensible thing and coerce e.g. '0' into a false, but if it doesn't, is the cleaner solution not to add a coercion on the attribute to make sure it's always handled sensibly, not just where we pass it on here?

[ccntrq]

Hey. Thanks for your response.

I share your feeling that my solution is a little bit hacky. In my opinion the attribute does the sensible thing though without any coercions. As long as the value stays in perl code everything is handled as expected. In perl we expect

if("0") { print "oh noes" }
else { print "expected" }

And the value is handled exactly this way in the corresponding sub pp_to_header.

Using HTTP::XSCookie though we pass the value into C code and cannot rely on this behaviour anymore. It turns out the module takes a different approach at deciding wether to add these flags or not. I think the clean fix would be to change HTTP::XSCookie to match perls semantics for boolean values.

I took a quick approach implementing the suggested change.
https://github.com/ccntrq/http-xscookies/commit/1342e712b21fdb35eaf9bf776e9c799f946f8854

I have no experience with XS code and there are other things broken in the module so I didn't find the time to make it into a pr.

What's your opinion on this? I'd be fine with adding a coercion to the attribute too. As long as we can disable the option in our dancer apps I'm fine.

[bigpresh]

Right, I still think there's a better way to fix this - I commented on your work on HTTP::XSCookies, as that seems like the best fix - but, on the other hand, pragmatically, this PR does solve the problem at hand, so I don't object to it being merged as it is - it could always be reverted later and a dependency on an updated HTTP::XSCookies added instead, if that fix gets applied and released.

[ccntrq]

I think your proposed solution should be the way to go here. My main priority here is to get this fixed in Dancer and I'd be glad if we could merge it if there aren't any other objections.

I will be on vacation for a week now so I probably won't be very responsive on this. When I come back I will make some time to take a deeper dive into HTTP::XSCookies to fix the underlying issue there.
Thanks for looking into my work and taking your time on this.

[bigpresh]

Yeah, let's merge this as-is as it does the job, and if/when HTTP::XSCookies gets updated we can revisit it.

[gonzus]

Hello. Original author of HTTP::XSCookies here. I just pushed a fix and will cut a new release.

[gonzus]

Uploaded HTTP-XSCookies-0.000015.tar.gz. Cheers.