fix bug with urldecoding cookie value

[malinink]

Discussion could be found in issue #15

[weierophinney]

I'm not sure this makes sense.

The specification indicates that the cookie values may consist of a cookie-octet, as defined below:

cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

This clearly allows + without encoding (it's character 2B in the US ASCII set). If we encode by default, we then have to assume that clients receiving the cookie are capable of, and will, detecting and decoding the value, which is not a safe assumption.

Since we do not currently do any encoding, I would argue that if the client you're sending the cookie to requires encoding, you should do it manually when you set the cookie value. This approach keeps the current class flexible enough to handle both urlencoded and non-urlencoded values, and allowing it to encompass the entire spec, and not a subset of it.

[malinink]

@weierophinney
The main problem is that I don't want to send cookie, I just want to get them from remote.
And that operation failed cause cookie format is not php urlencode() function result.

For example, value could be:

K%B42%B6%AA.%06%12J%25%259J%D6%99V%86%26%26%86F%16%26f%C6%E6%D6%C5V%96VJ%A9e%A9y%25%C1%40%ACd%9Dde%00%1434%B2RJ%CE%CF%2B%01%0Ay%96%A4%E6%16%2BY%27Z%19%82%CC%00I8%16%95d%26%E7%A4%C6%1B%9B%1A%9A%80L3%B6%AE%AD%05%00

This is valid value, but after decoding it starting contain wrong symbols

And I catch Exception here:
https://github.com/zendframework/zend-http/blob/master/src/Header/SetCookie.php#L305

I know that my PR solve particular problem, but creates another one.
I just don't know how to stay both of that functionality together.

May be there are any ideas? I could create Pr if I catch what should I do.

[malinink]

@weierophinney
Okay, I catch everything, now I can tell the main reason why current code is not correct.

Here https://github.com/zendframework/zend-http/blob/master/src/Header/SetCookie.php#L119
We set value of cookie from headers, but not real value, we urldecode it. In setter we check where cockie value is valid.

But, we must check value from headers before we urldecode them! Cause in specification there is no one world about encode/decode functions. This is only PHP specific way to store them.

[zerocrates]

It seems to me that @malinink is right here about there being a problem: the cookie value gets urldecoded when reading in from a string on line 119, and then re-encoded on the way out on line 236, so the validation is happening against the decoded value, which is incorrect.

The problem with this PR is that by now, that automatic decoding and encoding is expected from this class: it's been the behavior for years. So, you can't just take out the urldecode on line 119 and be okay.

The least-invasive solution I can come up with is to urlencode the value passed to HeaderValue::assertValid on line 305. This won't change the input or output from this class at all, it will just make it so that the validation check is working against the correct data, the same data that will be output by getFieldValue on line 236.

The downside is that you do a little useless round-trip between urldecode and urlencode, but the upside is that it should merely fix the problem without altering the external behavior.