Issue with implementation of the Secure flag in cookies

[aftadros]

Hello,

I am using the PersistentCookieJar library in my project and I am noticing some incorrect behavior when there are subsequent requests that modify already existing cookies in some very particular cases, which are:

1. When a cookie is created with the "Secure" flag set and without an expiration date, and then a subsequent request does not include the Secure flag on the cookie, and changes the value of the cookie to a non-empty value, and sets the cookie to be expired, then the saved value of that cookie does not change.

Example:
First request:

header("Set-Cookie: BAD_cookie_with_secure_flag_next_request_unsecure_nonempty_expire=initial_value; path=/; Secure", false);

Second request:

$expiredDate = new \DateTime('now');
$expiredDate->modify('-1 day');
header("Set-Cookie: BAD_cookie_with_secure_flag_next_request_unsecure_nonempty_expire=nonempty_changed_value; expires={$expiredDate->format(DATE_COOKIE)}; path=/;", false);

In this example, after the second request, the value of BAD_cookie_with_secure_flag_next_request_unsecure_nonempty_expire remains as initial_value even though it should have been deleted and not sent at all on the next request, since the cookie is expired.

2. When a cookie is created with the "Secure" flag set and without an expiration date, and then a subsequent request does not include the Secure flag on the cookie, and changes the value of the cookie to empty, and sets the cookie to be expired, then the saved value of that cookie does not change.

Example:
First request:

header("Set-Cookie: BAD_cookie_with_secure_flag_next_request_unsecure_empty_expire=initial_value; path=/; Secure", false);

Second request:

$expiredDate = new \DateTime('now');
$expiredDate->modify('-1 day');
header("Set-Cookie: BAD_cookie_with_secure_flag_next_request_unsecure_empty_expire=; expires={$expiredDate->format(DATE_COOKIE)}; path=/;", false);

In this example, after the second request, the value of BAD_cookie_with_secure_flag_next_request_unsecure_empty_expire remains as initial_value even though it should have been deleted and not sent at all on the next request, since the cookie is expired.

3. When a cookie is created without the "Secure" flag set and without an expiration date, and then a subsequent request does include the Secure flag on the cookie, and changes the value of the cookie to empty, and sets the cookie to be expired, then the saved value of that cookie does not change.

Example:
First request:

header("Set-Cookie: BAD_cookie_without_secure_flag_next_request_secure_empty_expire=initial_value; path=/;", false);

Second request:

$expiredDate = new \DateTime('now');
$expiredDate->modify('-1 day');
header("Set-Cookie: BAD_cookie_without_secure_flag_next_request_secure_empty_expire=; expires={$expiredDate->format(DATE_COOKIE)}; Secure; path=/;", false);

In this example, after the second request, the value of BAD_cookie_without_secure_flag_next_request_secure_empty_expire remains as initial_value even though it should have been deleted and not sent at all on the next request, since the cookie is expired.

In all three of these cases, the tests were accessed via HTTPS. Chrome on Android deleted the cookies in all three cases. Desktop Chrome and Safari on Mac OS also deleted the cookies in all cases.

It looks to me like in these cases there is something going wrong in com.franmontiel.persistentcookiejar.PersistentCookieJar#saveFromResponse because the persistor seems to contain non-persistent cookies (without an expiration date) when it should not contain them.

I think this might be happening because there might duplicate cookies entries getting created because of the Secure flag getting added into these requests. I see that this is the case in com.franmontiel.persistentcookiejar.persistence.SharedPrefsCookiePersistor#createCookieKey. Based on the logic in that function, it seems that there is one cookie created for HTTP and another one for HTTPS. The expected behavior is for only one cookie to be created, not dependent upon the Secure flag or protocol.

For reference, the spec for the Secure attribute is here: https://tools.ietf.org/html/rfc6265#section-4.1.2.5