-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Browser SDK cannot handle url-encoded cookie values #664
Labels
bug
Something isn't working
Comments
dajinchu
added a commit
to dajinchu/Amplitude-TypeScript
that referenced
this issue
Mar 22, 2024
1 task
Please see #686 (comment) for more details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Expected Behavior
AMP_xxxxxx
). Cookie is returned to browser via aSet-Cookie
response header.deviceId
already set by the backend, and uses it ✅Current Behavior
AMP_xxxxxx
). Cookie is returned to browser via aSet-Cookie
response header.Possible Solution
Call
decodeURIcomponent
on cookie value beforeatob
when parsing Amplitude cookieSteps to Reproduce
Broken flow:
Manually set an Amplitude cookie in your browser, with an URL encoded value (as my Rails backend would) , for example
AMP_1310df2f88=JTdCJTIyZGV2aWNlSWQlMjIlM0ElMjJkZDU1YjIxNC0yNmY1LTQ5OTAtYjFiZi0zNTkzYTIxOTJlNDIlMjIlN0Q%3D
(JSON value would equal
{"deviceId":"dd55b214-26f5-4990-b1bf-3593a2192e42"}
)Initialize browser SDK and let it parse this cookie
Inspect cookie set by browser SDK, decode its value :
{"deviceId":"72ac8556-1535-4fb2-bce4-f844bdb1d6cc","sessionId":xxxxx,"optOut":false,"lastEventTime":1234,"lastEventId":5}
deviceId
does not match. The JS SDK has ignored the backend cookie and set its owndeviceId
Fixed flow, proves what is broken:
Manually set an Amplitude cookie in your browser, with a value that has not been URL encoded, for example⚠️ notice the
AMP_1310df2f88=JTdCJTIyZGV2aWNlSWQlMjIlM0ElMjJkZDU1YjIxNC0yNmY1LTQ5OTAtYjFiZi0zNTkzYTIxOTJlNDIlMjIlN0Q=
<--=
is not encoded into%3D
here(JSON value would equal
{"deviceId":"dd55b214-26f5-4990-b1bf-3593a2192e42"}
)Initialize browser SDK and let it parse this cookie
Inspect cookie set by browser SDK, decode its value :
{"deviceId":"dd55b214-26f5-4990-b1bf-3593a2192e42","sessionId":xxxxx,"optOut":false,"lastEventTime":1234,"lastEventId":5}
deviceId
is now the proper value we set initially, found at step 1. The JS SDK has taken the initial cookie into account and re-used the existingdeviceId
Environment
The text was updated successfully, but these errors were encountered: