Skip to content

fix: parse CID from _ga cookie if it only contains CID string #1874

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.

Sign up for GitHub

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

Jump to bottom
Merged
dkoo merged 1 commit into from
Aug 9, 2022
Merged

fix: parse CID from _ga cookie if it only contains CID string #1874

dkoo merged 1 commit into from
Aug 9, 2022

Conversation

@dkoo
Copy link
Contributor

@dkoo dkoo commented Aug 9, 2022

All Submissions:

Changes proposed in this Pull Request:

#1857 fixed cookie parsing if it only contains the version number, domain depth and CID string parts, but I noticed in some sessions the cookie string contains only the CID, which still causes the parsing to fail. This PR handles that situation.

I'm not sure if the cookie value can contain other combinations of strings, so @adekbadek may have some insight into this.

How to test the changes in this Pull Request:

  1. Register on a site with Reader Activation & Site Kit w/ GA configured
  2. manually set _ga cookie to value of amp-12345
  3. On master, observe a PHP Notice: Undefined offset: 1 notice logged
  4. Switch to this branch, observe no notice logged

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo added [Type] Bug Incorrect behavior or functionality [Status] Needs Review The issue or pull request needs to be reviewed Reader Activation labels Aug 9, 2022
@dkoo dkoo self-assigned this Aug 9, 2022
@dkoo dkoo requested a review from a team as a code owner August 9, 2022 03:17
miguelpeixe
miguelpeixe approved these changes Aug 9, 2022
View reviewed changes
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working as described!

I noticed the cookie only had the CID while accessing AMP pages anonymously.

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Aug 9, 2022
@dkoo dkoo merged commit dc1fb52 into master Aug 9, 2022
@dkoo dkoo deleted the fix/cookie-parsing-cid branch August 9, 2022 21:33
dkoo added a commit that referenced this pull request Aug 10, 2022
@dkoo
fix: parse CID from _ga cookie if it only contains CID string (#1874)
4f9fd12
matticbot pushed a commit that referenced this pull request Aug 12, 2022
@semantic-release-bot
chore(release): 1.89.0-alpha.2 [skip ci]
d06fa0c 
# [1.89.0-alpha.2](v1.89.0-alpha.1...v1.89.0-alpha.2) (2022-08-12)

### Bug Fixes

* **google-auth:** catch and display errors ([#1871](#1871)) ([67cbcfd](67cbcfd))
* **oauth:** csrf token lifespan ([#1869](#1869)) ([52e0f8b](52e0f8b))
* parse CID from _ga cookie if it only contains CID string ([#1874](#1874)) ([dc1fb52](dc1fb52))
* redirecting to My Account after logging in while pre-authed ([#1863](#1863)) ([ddf111e](ddf111e))
* verify reader on google authentication ([#1873](#1873)) ([c9c4eef](c9c4eef))

### Features

* authenticated reader cookie ([#1882](#1882)) ([352316b](352316b))
* better welcome email copy for initial verification ([#1880](#1880)) ([604ebf7](604ebf7))
* cookie reader's preferred auth strategy ([#1875](#1875)) ([fc47f41](fc47f41))
* disable woocomerce welcome emails in favor of verification email ([#1876](#1876)) ([1e470e3](1e470e3))
* lock access to My Account UI until account is verified ([#1877](#1877)) ([a850f48](a850f48))
@matticbot
Copy link
Contributor

matticbot commented Aug 12, 2022

🎉 This PR is included in version 1.89.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Aug 16, 2022
@semantic-release-bot
chore(release): 1.89.0 [skip ci]
26da849 
# [1.89.0](v1.88.0...v1.89.0) (2022-08-16)

### Bug Fixes

* **active-campaign:** legacy contacts detection ([#1858](#1858)) ([67640a5](67640a5))
* **campaigns-wizard:** segmentation wording ([ddf61ad](ddf61ad))
* ensure scroll on smaller height ([#1813](#1813)) ([e234e8b](e234e8b))
* fix fatal error when debug mode active ([#1826](#1826)) ([d9388ee](d9388ee))
* **ga:** cookie parsing ([#1857](#1857)) ([a936abd](a936abd))
* google auth button type ([#1829](#1829)) ([3704d9f](3704d9f))
* **google-auth:** catch and display errors ([#1871](#1871)) ([67cbcfd](67cbcfd))
* **google-auth:** ensure popup on user click event ([#1831](#1831)) ([0af9abf](0af9abf))
* **magic-links:** fix email encoding on sent link ([#1833](#1833)) ([8d4756c](8d4756c))
* **my account:** handle legacy data ([#1823](#1823)) ([6816799](6816799))
* **newsletters:** use international date format ([#1855](#1855)) ([4cda57d](4cda57d))
* **oauth:** csrf token lifespan ([#1869](#1869)) ([52e0f8b](52e0f8b))
* parse CID from _ga cookie if it only contains CID string ([#1874](#1874)) ([dc1fb52](dc1fb52))
* **popups:** use new Campaigns method for creating donation events on new orders ([#1794](#1794)) ([49dc14c](49dc14c))
* **reader-activation:** add metadata to reader registered on donation ([722724c](722724c))
* **reader-activation:** handle modal conflict when auth is triggered from a prompt ([c2a0141](c2a0141)), closes [#1835](#1835)
* **reader-activation:** handle no lists config available ([23b0249](23b0249))
* **reader-activation:** reinitialize auth links after DOM load ([#1812](#1812)) ([0a4b499](0a4b499))
* **reader-activation:** remove async prop from library ([#1846](#1846)) ([4131ca6](4131ca6))
* **reader-activation:** username generation handling ([#1789](#1789)) ([17edf2a](17edf2a))
* redirecting to My Account after logging in while pre-authed ([#1863](#1863)) ([ddf111e](ddf111e))
* **registration-block:** don't escape html for sign in labels ([#1834](#1834)) ([871300d](871300d))
* **registration-block:** margin for success message ([#1808](#1808)) ([1bfe546](1bfe546))
* **registration-block:** render on preview ([#1844](#1844)) ([87b9be9](87b9be9))
* tweak arguments for magic link client hash ([#1862](#1862)) ([8dcd45e](8dcd45e))
* verify reader on google authentication ([#1873](#1873)) ([c9c4eef](c9c4eef))

### Features

* **active-campaign:** metadata improvements ([#1851](#1851)) ([48883af](48883af))
* **active-campaigns:** override is-new-contact for legacy contacts ([34dd9a2](34dd9a2))
* **analytics:** send GA events on the server side ([#1828](#1828)) ([3e384e1](3e384e1))
* authenticated reader cookie ([#1882](#1882)) ([352316b](352316b))
* better welcome email copy for initial verification ([#1880](#1880)) ([604ebf7](604ebf7))
* cookie reader's preferred auth strategy ([#1875](#1875)) ([fc47f41](fc47f41))
* disable woocomerce welcome emails in favor of verification email ([#1876](#1876)) ([1e470e3](1e470e3))
* **donations:** remove defaultFrequency from the configuration ([#1814](#1814)) ([b6aa894](b6aa894))
* handle contact update w/out lists selection ([#1816](#1816)) ([67574d1](67574d1))
* handle new frequency options in Campaigns dashbaord ([#1779](#1779)) ([c770a7d](c770a7d))
* if registering an email that already has an account, show different message ([#1849](#1849)) ([bf48bc4](bf48bc4))
* lock access to My Account UI until account is verified ([#1877](#1877)) ([a850f48](a850f48))
* **my-account:** stripe billing portal link ([#1761](#1761)) ([3e69af1](3e69af1)), closes [#1742](#1742) [#1739](#1739) [#1740](#1740) [#1741](#1741) [#1782](#1782)
* **reader-activation:** account link and auth form ([#1754](#1754)) ([b163664](b163664))
* **reader-activation:** activecampaign master list ([#1818](#1818)) ([ecbbc47](ecbbc47))
* **reader-activation:** disable 3rd party login buttons initially ([#1806](#1806)) ([c806bfe](c806bfe))
* **reader-activation:** optimistic account link ([#1847](#1847)) ([85c550a](85c550a))
* **reader-activation:** prevent updating user email in my-account ([7d49db4](7d49db4))
* **reader-activation:** registration auth cookie control ([#1787](#1787)) ([aeb0b5b](aeb0b5b))
* **reader-activation:** settings wizard ([#1773](#1773)) ([aaff0de](aaff0de))
* **reader-auth:** make password login the first option, instead of login link ([1fe5ffa](1fe5ffa)), closes [#1809](#1809)
* register anonymous single donors ([#1795](#1795)) ([9e4f2f6](9e4f2f6))
* **registration-block:** add success icon ([#1804](#1804)) ([86c38f8](86c38f8))
* **registration-block:** editable success state ([#1785](#1785)) ([7dcea82](7dcea82)), closes [#1768](#1768)
* **registration-block:** login with Google ([#1781](#1781)) ([ed79c5c](ed79c5c)), closes [#1774](#1774)
* **registration-block:** newsletter subscription ([#1778](#1778)) ([717b5b8](717b5b8))
* reorganise donations wizard and use buttongroup for donation type ([#1824](#1824)) ([f7b58ae](f7b58ae))
* replace WooCommerce’s login form with our own ([#1854](#1854)) ([f5b24c4](f5b24c4))
* **rss:** adds offset feature ([#1790](#1790)) ([321eff5](321eff5))
* send user metadata to AC ([#1793](#1793)) ([03a15ba](03a15ba))
* set client id cookie; reader activation tweaks ([#1780](#1780)) ([96a07ae](96a07ae))
* **stripe:** webhook auto-creation and validation ([365aed9](365aed9))
* tweak registration block styling ([d83448e](d83448e))

### Reverts

* Revert "chore(release): 1.87.0 [skip ci]" ([ca8d55c](ca8d55c))
@matticbot
Copy link
Contributor

matticbot commented Aug 16, 2022

🎉 This PR is included in version 1.89.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miguelpeixe miguelpeixe miguelpeixe approved these changes

Assignees

@dkoo dkoo

Labels

Reader Activation released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge [Type] Bug Incorrect behavior or functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dkoo @matticbot @miguelpeixe