lat9 / keep_cart

Keeps a Zen Cart "cart" in a browser cookie.
GNU General Public License v2.0
1 stars 1 forks source link

`cart` cookie problem when updating product quantities in shopping cart #7

Closed marco-pm closed 1 year ago

marco-pm commented 2 years ago

This is a weird one.

Sometimes, when updating product quantities on the shopping cart page I'm getting a 302 redirect to the same page (as expected, after updating quantities the page reloads) and then nothing. No response from the server (not even an HTTP error status code), no errors in the Apache logs. This only happened in production, couldn't replicate it in my dev environment. So it took me quite a while to find the cause. After doing some tests I realized that it only happened when there were at least 2 or 3 different products in the cart, and only if the user was not logged in. The latter made me think about the keep_cart plugin, so I tried disabling it and... boom, problem solved.

I narrowed it down to this part of class.savecart.php

https://github.com/lat9/keep_cart/blob/c1516cb868f0d5db5cfa5ccc807346601d344657/includes/classes/observers/class.savecart.php#L170-L182

specifically the setcookie('cart', $cookie_value, $cookie_options). If I comment that out, the issue doesn't arise.

When a product quantity is updated on the cart page, that observer's code is executed at least twice for every product in cart, because NOTIFIER_CART_UPDATE_QUANTITY_END and NOTIFIER_CART_CLEANUP_END are triggered for each product (as you know, even if the user clicks on the update quantity button next to a product, the whole cart form is submitted every time so all product quantities are updated). So basically the same cart (and cartkey) cookie is updated several times — and almost always with the same value if only one product has the quantity changed — in a very short time. I think the browser or the server doesn't like it for some reason and blocks the connection to the website. Although I admit I have no idea what restricition/policy/configuration/etc. is causing this. I'm using the same browser in both development and production and am not experiencing the issue locally. Tried with different browsers (Firefox, Edge), same result. I thought maybe it was some php cookie configuration, but couldn't find/think of any. Can you think of something about what could be the cause? Interestingly, the cartkey cookie doesn't cause the problem, so could it be something related to the content or size of the cart cookie?

In the end, because I don't like ZC's behaviour of updating the entire cart every time anyway, I changed the shopping cart code so that only the quantity of the product that the user changed is updated. This way, only one product is updated at a time, and the cookie is only rewritten twice. It's a workaround of course, not a solution to the problem, but for now I couldn't think of anything else.

lat9 commented 2 years ago

Thanks for the detailed report, @marco-pm. I've got this on my ever-growing to-do list.

marco-pm commented 1 year ago

I found the cause. It's not a bug of the code, although it shows a potential problem when products have many attributes.

I was experiencing the same problem when the cart had more than 27 (or so) products. Again, only in production. So it had something to do with the server configuration. After a lot of searching I found the cause: nginx blocked the request because "upstream sent too big header while reading response header from upstream". I was looking at the apache logs but forgot to check also the nginx logs (there's a nginx reverse proxy before apache on the server). So the cart cookie sent with the HTTP headers is too big. Increasing the nginx buffer parameters solves it of course, but why is it happening? 27 products are not that many. The cause is that in that ZC store every product has many read-only attributes, which are saved into the shopping cart together with the product and therefore into the cookie, increasing its size. In the first scenario there were only 3 or so products in the cart, but calling setcookie multiple times increases the HTTP headers and... same problem.

I solved the first scenario by updating only a single product and not the entire cart, as I wrote in my first post. For the second scenario, I think I'll make a change to the code to avoid saving read-only attributes in the cookie.

marco-pm commented 1 year ago

Even without saving the read-only attributes, the cookie is still quite big and triggers the nginx rejection. Therefore the only viable solution seems to increase the server limits, for example as explained here.