ampproject / ampproject/amphtml

.removeItem creates arbitrary value from state

Open
#38,364 3 comments 0 reactions 0 assignees View on GitHub
Component: amp-bind P3: When Possible Type: Bug WG: components WG: runtime
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

### Description

Issue reported before [here](https://github.com/ampproject/amphtml/issues/37739).

Using `window.localStorage.removeItem` within the amp-script worker using any value (even if it doesn’t exist) actually creates an item in local storage with that key and a value that is defined somewhere in AMP environment.

It can be reproduced by having the `removeItem` code within the amp-script JS that is guaranteed to be executed. The expected behaviour would be for this value to just be deleted.

cc @samouri

### Reproduction Steps

Added an AMP playground link [here](https://playground.amp.dev/#share=PCFET0NUWVBFIGh0bWw+CjxodG1sIOKaoT4KICA8aGVhZD4KICAgIDxtZXRhIGNoYXJzZXQ9InV0Zi04IiAvPgogICAgPHRpdGxlPk15IEFNUCBQYWdlPC90aXRsZT4KICAgIDxsaW5rIHJlbD0iY2Fub25pY2FsIiBocmVmPSJzZWxmLmh0bWwiIC8+CiAgICA8bWV0YSBuYW1lPSJ2aWV3cG9ydCIgY29udGVudD0id2lkdGg9ZGV2aWNlLXdpZHRoIiAvPgogICAgPHN0eWxlIGFtcC1ib2lsZXJwbGF0ZT5ib2R5ey13ZWJraXQtYW5pbWF0aW9uOi1hbXAtc3RhcnQgOHMgc3RlcHMoMSxlbmQpIDBzIDEgbm9ybWFsIGJvdGg7LW1vei1hbmltYXRpb246LWFtcC1zdGFydCA4cyBzdGVwcygxLGVuZCkgMHMgMSBub3JtYWwgYm90aDstbXMtYW5pbWF0aW9uOi1hbXAtc3RhcnQgOHMgc3RlcHMoMSxlbmQpIDBzIDEgbm9ybWFsIGJvdGg7YW5pbWF0aW9uOi1hbXAtc3RhcnQgOHMgc3RlcHMoMSxlbmQpIDBzIDEgbm9ybWFsIGJvdGh9QC13ZWJraXQta2V5ZnJhbWVzIC1hbXAtc3RhcnR7ZnJvbXt2aXNpYmlsaXR5OmhpZGRlbn10b3t2aXNpYmlsaXR5OnZpc2libGV9fUAtbW96LWtleWZyYW1lcyAtYW1wLXN0YXJ0e2Zyb217dmlzaWJpbGl0eTpoaWRkZW59dG97dmlzaWJpbGl0eTp2aXNpYmxlfX1ALW1zLWtleWZyYW1lcyAtYW1wLXN0YXJ0e2Zyb217dmlzaWJpbGl0eTpoaWRkZW59dG97dmlzaWJpbGl0eTp2aXNpYmxlfX1ALW8ta2V5ZnJhbWVzIC1hbXAtc3RhcnR7ZnJvbXt2aXNpYmlsaXR5OmhpZGRlbn10b3t2aXNpYmlsaXR5OnZpc2libGV9fUBrZXlmcmFtZXMgLWFtcC1zdGFydHtmcm9te3Zpc2liaWxpdHk6aGlkZGVufXRve3Zpc2liaWxpdHk6dmlzaWJsZX19PC9zdHlsZT48bm9zY3JpcHQ+PHN0eWxlIGFtcC1ib2lsZXJwbGF0ZT5ib2R5ey13ZWJraXQtYW5pbWF0aW9uOm5vbmU7LW1vei1hbmltYXRpb246bm9uZTstbXMtYW5pbWF0aW9uOm5vbmU7YW5pbWF0aW9uOm5vbmV9PC9zdHlsZT48L25vc2NyaXB0PgogICAgPHNjcmlwdCBhc3luYyBzcmM9Imh0dHBzOi8vY2RuLmFtcHByb2plY3Qub3JnL3YwLmpzIj48L3NjcmlwdD4KICAgIDxzY3JpcHQgYXN5bmMgY3VzdG9tLWVsZW1lbnQ9ImFtcC1zY3JpcHQiIHNyYz0iaHR0cHM6Ly9jZG4uYW1wcHJvamVjdC5vcmcvdjAvYW1wLXNjcmlwdC0wLjEuanMiPjwvc2NyaXB0PgogICAgPHN0eWxlIGFtcC1jdXN0b20+CiAgICAgIGgxIHsKICAgICAgICBtYXJnaW46IDFyZW07CiAgICAgIH0KICAgIDwvc3R5bGU+CiAgICA8bWV0YSBuYW1lPSJhbXAtc2NyaXB0LXNyYyIgY29udGVudD0ic2hhMzg0LTNadkVHaFNwRlFSYVFQWHNWYmZkYUZRazk0cmMyRkUzWk5PeGtkOWY2VEFlMGt6Z2JYaUZKbjdZWGMxeXZvNVIgIj4KICA8L2hlYWQ+CiAgPGJvZHk+CiAgICA8aDE+SGVsbG8gQU1QSFRNTCBXb3JsZCE8L2gxPgogIDwvYm9keT4KICA8YW1wLXNjcmlwdCB3aWR0aD0iMjAwIiBoZWlnaHQ9IjEwMCIgc2NyaXB0PSJoZWxsby13b3JsZCI+CiAgICA8YnV0dG9uPkhlbGxvIGFtcC1zY3JpcHQhPC9idXR0b24+CiAgPC9hbXAtc2NyaXB0PgoKICA8IS0tIEFkZCBbdGFyZ2V0PSJhbXAtc2NyaXB0Il0gdG8gdGhlIDxzY3JpcHQ+IGVsZW1lbnQuIC0tPgogIDxzY3JpcHQgaWQ9ImhlbGxvLXdvcmxkIiB0eXBlPSJ0ZXh0L3BsYWluIiB0YXJnZXQ9ImFtcC1zY3JpcHQiPgogICAgd2luZG93LmxvY2FsU3RvcmFnZS5yZW1vdmVJdGVtKCdoZWxsb3dvcmxkJykKICA8L3NjcmlwdD4KPC9odG1sPgo=). On line 27 I added `window.localStorage.removeItem` and you can see that the `helloworld` key to be removed is actually created in localstorage.

### 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 with the amp-script worker handling of window.localStorage.removeItem and reproduce the behavior using the linked AMP playground example. Verify that removing an existing or missing key does not create a localStorage entry and that the helloworld key is absent afterward.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, web-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.