1.3.x ngCookies service overwrites/duplicates cookies set outside of ngCookies

[chrisakers]

Overview of the Issue

In the event that a pre-existing cookie is modified outside of $cookies just prior to a cookie being modified or created with $cookies, the $cookies service will, in error, recognize this as a cookie change made with the $cookies service and will propagate the change to the browser's cookies on the next $digest. If the pre-existing cookie was set on the fully qualified host name, this defect will overwrite the pre-existing cookie with a session cookie which may cause the cookie to expire earlier (or later) than desired. If the pre-existing cookie was set on a reduced domain, this defect will add an additional cookie on the fully qualified host name which duplicates the pre-existing cookie.

Motivation for or Use Case

This happens quite frequently in my use case. I have an external analytics library (non-Angular) that saves data into a cookie based on user interaction. When the user clicks an element that also triggers some angular application code that sets cookies this defect can occur. The user clicks, then analytics code modifies its cookie, and then angular application code sets a cookie using ngCookies.

In this scenario, our analytics cookies are set on a reduced domain (.example.com). When this defect occurs, ngCookies duplicates the analytics cookie to the FQHN (foo.bar.example.com). This duplicate cookie interferes with the proper analytics functionality.

Angular Version(s): 1.3.x

The updated API for ngCookies in 1.4.x appears to be immune to this issue. This issue occurs in the $watch for the 1.3.x $cookies service. Since 1.4.x $cookies don't use a $watch this defect only applies to version < 1.4.

Browsers and Operating System

Assume all browsers and OS's since this defect is in angular source code.

Reproduce the Error

1. Go to http://run.plnkr.co/plunks/FInyNczw6CDjsgXLvzUS/ in a browser that allows viewing of set cookies.
2. Click Add Vanilla Cookie button. Notice cookie added to .plnkr.co domain with expiration at the end of the year.
3. Click Add Angular Cookie button. Notice cookie added to FQHN with session expiration.
4. Modify the value of the jscookie and angcookie text inputs.
5. Click the Add Both Cookies button. Notice both cookies are updated to the new values, but also a duplicate "jscookie" is added to the FQHN with a session expiration.

Video of defect reproduction: https://youtu.be/8odal8eIig0

Related Issues

Possibly related issues: #10995 and #8324

Suggest a Fix

Background

The angular $cookies services uses a variable lastCookies (code) to track what the application has changed to the $cookies since the last $digest. This lastCookies variable is compared against the $cookies service object to determine whether the application has deleted , modified, or added new cookies. (code) If there has been a change to the $cookie service then the browser’s cookie is updated with the changes. This check occurs every time through the $digest loop since the function that contains this code has been added to the $watch list. This function is named "push" and it's responsible for "pushing" changes from service to browser.

Additionally, the $cookies service updates its internal state periodically with changes from the browser’s cookies. (code) This is done via $browser.addPollFn (approximately every 100ms). This can occur much less frequently than the push. This part of the code "pulls" changes from the browser’s cookies into the service.

In the $cookies service "push" function there is a section of code that "verifies what was actually stored" (code). This section of code is entered if a change was detected in the $cookies object. This verification code gets a fresh copy of the browser cookies and compares it to the $cookies service values. If the browser cookie has been removed or modified then the $cookies service value is updated to match. This is problematic when 2 digest loops, and therefore 2 "push" executions, occur before the $cookie service polling function which "pulls" the browser cookies into the internal "lastCookies" comparison variable.

Defective Behavior

In the scenario where a cookie is updated outside of the $cookie service, the first $digest loop "push" execution updates the $cookies service with the new value (code). And then on the second $digest loop "push" execution, the comparison against "lastCookies" (code) fails and the $cookies service updates the browser cookies with the new cookie value. This causes the new cookies to be set according to the default behavior of $browser.cookies (code) which sets on the fully qualified host name. This causes cookies to be duplicated from ".example.com" to "foo.bar.example.com".

Additionally, without any interaction with outside cookies, ngCookies itself will send multiple repetitious cookie updates and deletes to $browser.cookies. Since, the "lastCookies" verification check always fails until the next $browser poll where lastCookies gets updated with the current state of the browsers cookies.

Pull Request Fix

#11515 - This pull request updates the lastCookies verification variable with the updates that are being made to the browser cookies. Thus preventing future verification from failing.

[pkozlowski-opensource]

Now, this is what you call a good bug report! Awesome summary @chrisakers 👏 !

[johnpapa]

@IgorMinar @petebacondarwin This is the cookie issue I mentioned to you previously.

[IgorMinar]

thanks for the awesome writeup!