ampproject / ampproject/amphtml
Race condition in amp-cid-backup experiment deleting parts of the amp-store entry in localStorage
- Dominant language
- JavaScript
- Stars
- 14.9k
- Forks
- 4.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Description
There seems to be a race condition when restoring client ids using the amp-cid-backup experiment. When using more than a given amount of variable replacement of the CLIENT_ID, part of the amp-store entry in the localStorage is lost.
For this example, I have placed amp-consent to show that the consentString is lost.
In the simple html page below, amp-analytics will try to track an event using seven CLIENT_ID variable replacement when the document is visible. If the event `"event": "https://example.com?rand=RANDOM&bar1=CLIENT_ID(bar)&bar2=CLIENT_ID(bar2)&bar3=CLIENT_ID(bar3)&bar4=CLIENT_ID(bar4)&bar5=CLIENT_ID(bar5)&bar6=CLIENT_ID(bar6)&bar7=CLIENT_ID(bar7)"` has only 6 CLIENT_ID replacements, the consentString will always be there after refreshing, but it will be gone if we put any more.
### Reproduction Steps
- How to reproduce the issue:
1. Using the following page, load and accept consent.
2. Reload the page, check that consentString is gone from the amp-store localStorage entry.
3. Reload the page, see that consent is requested again.
This can be repeated endlessly, having consent requested every other reload.
- How to not have the issue
- Setting the amp-cid-backup experiement off
- Having 6 or less CLIENT_ID replacements in the event url of the analytics
- Other scenarios where the issue can be reproduced
- If more than 6 CLIENT_ID replacements are needed, they don't have to be in the same amp component. Having some replacements in amp-state and others in amp-analytics have also lead into reproducing the issue. For simplicity, the snippet to reproduce it here just uses amp-analytics.
```
CMP issue scenario
body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}
Hello world!
{
"requests": {
"event": "https://example.com?rand=RANDOM&bar1=CLIENT_ID(bar)&bar2=CLIENT_ID(bar2)&bar3=CLIENT_ID(bar3)&bar4=CLIENT_ID(bar4)&bar5=CLIENT_ID(bar5)&bar6=CLIENT_ID(bar6)&bar7=CLIENT_ID(bar7)"
},
"triggers": {
"visile": {
"on": "visible",
"request": "event"
}
},
"transport": {
"beacon": false,
"xhrpost": true,
"image": false
}
}
{
"checkConsentHref": "https://live.mrf.io/cmp/www.santeplusmag.com/amp/check-consent?source=marfeel",
"promptUISrc": "https://live.mrf.io/cmp/www.santeplusmag.com/amp/index.html?build=25235",
"postPromptUI": "postPromptUI",
"uiConfig": {
"overlay": true
},
"policy": {
"default": {
"waitFor": { "Marfeel": [] }
}
},
"clientConfig": {
"vendorListBaseUrl": "https://live.mrf.io/statics/1w/js/ps/vendor-list.consensu.org/v2/",
"baseUrl": "https://live.mrf.io/cmp/www.santeplusmag.com",
"consentLanguage": "fr",
"publisherLogo": "https://www.santeplusmag.com/mrf4u/mstore/9ac49c1/d58b6c06490e56a90124c30a99ae8d9b485f0b7e/logo_carousel.png"
}
}
```
### Relevant Logs
_No response_
### Browser(s) Affected
_No response_
### OS(s) Affected
_No response_
### Device(s) Affected
_No response_
### AMP Version Affected
_No response_
Contributor guide
Research direction
Start by reproducing the issue with the provided AMP page, focusing on amp-cid-backup, amp-analytics, amp-consent, and the amp-store entry in localStorage. Compare behavior with six versus seven CLIENT_ID replacements and with the experiment disabled. Done means repeated reloads no longer remove consentString or request consent again unexpectedly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100