ampproject / ampproject/amphtml

Race condition in amp-cid-backup experiment deleting parts of the amp-store entry in localStorage

Open
#35,753 3 comments 0 reactions 0 assignees View on GitHub
Stale Type: Bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.