Skip to content
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

Added data unset to stop data being read from outdated cookie #47

Merged
merged 6 commits into from
Feb 7, 2019

Conversation

gelysis
Copy link
Contributor

@gelysis gelysis commented Feb 1, 2019

Fix for the case when the session increases in size and passes the cookie store limit (of currently 1024 characters).

src/Store/CookieStore.php Outdated Show resolved Hide resolved
@gelysis
Copy link
Contributor Author

gelysis commented Feb 7, 2019

Hi @robbieaverill , hi @ScopeyNZ ,

Do you have a rough idea, when this PR will be merged?

We will need it for a production release early next week. If you need more time to review we will be able to release it from our fork.

Cheers,

@ScopeyNZ ScopeyNZ merged commit ec86442 into silverstripe:master Feb 7, 2019
@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Feb 7, 2019

Do you have a rough idea, when this PR will be merged?

Now 😆 . Although I wish I hadn't because I should've asked you to retarget the 2.1 or 2.0 branches as a patch change. Nevermind I'll cherry-pick it back for you.

Did you want a release to point at too? What version of this module is your project currently looking at?

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Feb 7, 2019

Ok the patch exists in 2.0, 2.1 and master now.

For reference:

  • 2.0 will be where a future 2.0.1 will be tagged from (released)
  • 2.1 for 2.1.1
  • master for 2.2.0

@gelysis
Copy link
Contributor Author

gelysis commented Feb 7, 2019

Wow, that's great. Thanks for all, @ScopeyNZ .
Project is pointing at 2.1.x (2.1.1 release would be great) and I have just pulled down the correct code.

@gelysis gelysis deleted the exceed-cookielength-fix branch February 7, 2019 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants