CGI::Cookie and CookieStore performing unnecessary work causing interoperability bug with Merb

It would be simple enough to make this work with existing cookies. Would a patch implementing this be likely to be accepted?

Here's a patch that passes all of rails' tests, my application specific tests and is backwards compatible (with tests for that too).

@value is expected to be an array but if you pass "one\ntwo" to Array you get:

Array("one\ntwo") # => ["one\n", "two"]

Which gets written out in the cookie header to "one%0A&two" (notice the & used for joining the separate elements). This issue, when used with other systems without this behavior (in my case merb), causes digest mis-matches.

Array("one\ntwo") != ["one\ntwo"] # => true

Ah, I see. Could you include a test for that case? The patch looks good, otherwise.

I actually did, it's called test_cookies_should_not_be_split_on_values_with_newlines

Sorry, I missed that. I have mixed feelings about changing this behavior for all cookie values. Backward compatibility applies to the cookie store only.

Yeah, I agree but, taken in context CGI::Cookie is plainly broken and acting differently depending on how you initialize.

When name is a string the value is already an array so:

CGI::Cookie.new("avar", "one\ntwo").value # => ["one\ntwo"]

While:

CGI::Cookie.new(:name => "avar", :value => "one\ntwo").value # => ["one\n", "two"]

That's why I didn't make the same change to the String path in CGI::Cookie#initialize because the variable argument *value is always an array and Array doesn't mangle an array's values the same way it does strings.

So the behavior is already inconsistent and this patch simply makes the initializer act consistently.

I buy that. Thanks for clarifying, Rich.

This change seems to change the behavior of deleting a cookie. When a cookie is deleted it's value is set to an empty string. Previously that would have meant:

Array('') => []

but now it is turned into: ['']

I'm not really familiar with how cookies are supposed to behave so I'm not sure where the bug is, but for example with the restful_authentication plugin generated code what happens is:

1. action does 'cookies.delete :auth_token'

2. test fails on 'assert_equal @response.cookies["auth_token"], []' with [""] not being equal to []

Is this expected?

I've attached a new patch addressing this issue along with a test for it.