amp_cookie_test cookie values can contain disallowed characters

[mickaeltr]

Expected Behavior

amp_cookie_test cookies should not be rejected by firewalls.

Current Behavior

amp_cookie_test cookie values can sometimes be rejected by firewalls, such as the Spring Security StrictHttpFirewall

Here is an example of a cookie that does not pass through the firewall, because of special characters (\u0080\u0099):
amp_cookie_testZ4RgnufBu172SVyuXfqfSV=Sat Dec 18 2021 22:50:11 GMT+0100 (Ora standard dellâ\u0080\u0099Europa centrale);

Possible Solution

Maybe use something like new Date().toUTCString() rather than String(new Date()) in base-cookies.js

Steps to Reproduce

Environment

• JS SDK Version:
• Installation Method:
• Browser and Version:

[kevinpagtakhan]

Thanks @mickaeltr for reporting this issue. We are currently looking into this, and we will get back to you soon.

[mickaeltr]

Thanks for fixing it!

[mickaeltr]

Hello @kevinpagtakhan, I am giving a second thought.
Since we've had quite a lot of those issues (until you fixed it), that means that those test cookies are sent along the HTTP requests, although they're supposed to be cleaned up: see the code and test (that test seems wrong since it's based on a random base64 string).
What do you think?

[mickaeltr]

In addition, it would be really great to cleanup ALL remaining amp_cookie_test cookies, because despite of upgrading to the newer SDK, old faulty cookies are still being sent, e.g.: amp_cookie_test9BV14eCewbPZ5_CWG8-2C2=Fri Dec 10 2021 10:35:59 GMT+0100 (heure normale d�Europe centrale) was seen in our logs this morning

[kevinpagtakhan]

@mickaeltr I am surprised these are not being cleaned up. For context, how much of this cookie are you still seeing before and after upgrading?

[mickaeltr]

Hello @kevinpagtakhan, thanks for replying.

Here is an overview of the amount of errors we've met this week:

I believe that the original issue (the disallowed characters in the cookie value) has been fixed, however it revealed another issue: it seems the amp_cookie_test* are not properly removed. So we're still having errors due to cookies that were set before the fix.

The first thing I would do is to make sure that this test and this test too, are reliable (I feel like they're wrong):

const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id(); // generates a *first* random name
cookie.areCookiesEnabled(); // creates a cookie with a *second* random name
assert.isNull(cookie.get(`${cookieName}=`), null); // indeed there is no cookie with the first random name, but what about the second one?

This is what I would expect from a newer version of the SDK:

• Must-have: new amp_cookie_tests are cleaned up
• Nice-to-have: old amp_cookie_tests are also cleaned up

Thanks for your help 🙏

(meanwhile, we're trying to see how we could filter this cookie out in our API gateway, or ignore them in the Java / Spring Boot code that rejects them with a 500 error)