Because crawlers generally do not save cookies, creating a session for them would result in a new session being created for every request. Normally, drupal will not save a session on a visitor's first request. But it will save a session anyway if it finds anything in $_SESSION. And Ubercart stores uc_referer_uri in $_SESSION at the exit of every request.
This quickly becomes a problem if the site is actively crawled by robots and has anything but a very short session timeout. The sessions table can grow quite large and impact site performance (changing the storage engine of the sessions table from MyISAM to InnoDB helps somewhat, by avoiding table-level locking).
I've attached a simple patch that avoids writing session data whenever it appears that this is the first request of a session.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 1093626-cart-referer.patch | 1.88 KB | longwave |
| uc_store-session-fix.patch | 553 bytes | balloon |
Comments
Comment #1
longwaveSeems like a better fix for #273574-14: Reduce HTTP_REFERER checks than the one in that issue.
Comment #2
tr commentedWon't this patch mess up the whole anonymous cart thingy? I'd have to go back and read through all those convoluted issues, but I thought we were deliberately starting a session on first request (contrary to what Drupal normally does).
Comment #3
torgospizzaA quick and dirty method would be to turn on garbage collection in settings.php. See: http://drupal.org/node/35834
Alternatively, you could enable the Session Expire module.
I agree, however, that a built-in fix would be our best option, if at all possible.
Comment #4
balloon commentedAnonymous carts should still work because anonymous visitors using a browser will store the session cookie (and crawlers will generally ignore it). Drupal starts a session on the first request and sends a cookie to the client, but doesn't actually save the session to the database unless:
1. a session cookie was received from the client, or
2. some data has been written to the session
Similarly, this patch prevents ubercart from setting session data unless it sees a session cookie, which indicates that the client has visited at least once before.
Comment #5
longwaveTested and committed on both branches.
uc_store_exit() only deals with storing the referrer, nothing to do with the cart ID - this is still correctly stored when needed and anonymous carts work correctly (and the referrer is stored if needed on the second page load).
Comment #6
tr commentedThis seems to cause 1 fail on the Cart settings test for the D7 branch.
Comment #7
longwaveThis is because of Drupal 7's lazy session start; the session ID isn't initialised until something actually writes to it. However I realised we can use the current URL instead of the referer when adding a product to the cart, so this seems to fix it.
Comment #8
tr commentedActually, now that I look closer, I see that I get the same 1 fail on the D6 test cases under certain circumstances. Commenting out the code
eliminates the fail in both D6 and D7.
I didn't try the patch yet - should it work for both D6 and D7?
Comment #9
Island Usurper commenteduc_store_exit() removed by #273574: Reduce HTTP_REFERER checks.