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

Use base64 to avoid truncation with PostgreSQL #167

Merged
merged 1 commit into from
May 7, 2022
Merged

Use base64 to avoid truncation with PostgreSQL #167

merged 1 commit into from
May 7, 2022

Conversation

mauriciv
Copy link

@mauriciv mauriciv commented May 5, 2022

When using a PostgreSQL database, serializing the cart's content and storing them in a text column doesn't work. because PostegreSQL drops everything after a zero byte in text columns. And seeing as PHP's serialize adds zero bytes to private and protected members
image
this results as a truncated string being inserted in the content column and being unable to unserialize it later.
This behaviour can be reproduced in the test suite by using a PostegreSQL database instead of a sqlite one.

Using base64, we can sidestep this issue, the zero byte will only "live" in PHP, and never be inserted in PostgreSQL.

Unfotunately, using base64 will end up being a breaking change for those that are currently using a PostgreSQL database, although I would imagine that already implemented some form of workaround because they would have been impacted by this bug already.

@bumbummen99
Copy link
Owner

Good work, LGTM 👍.

I am in general not a big fan of the fact that the package is serializing the cart in order to store it, it is just not atomic and comes with disatvantages for example if you have massive carts or want to work without session. That is also why the refactoring is currently blocked as i am looking for a good way to store each cart item seperately.

@bumbummen99 bumbummen99 merged commit 83bad37 into bumbummen99:master May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants