Easy-digital-downloads: Session cookies are created on every page load

[mindctrl]

We've had two reports recently about the fact that a session cookie gets created on every page load, making it hard to cache. One came through a Pagely customer and another was from Pressjitsu.

One report:

The use of sessions in EDD prevents any page from being cached because it's issuing a unique cookie to every visitor. It would be best if EDD initialized this unique session ONLY when the user has taken any sort of action, like add something to the cart. That way, the majority of the users can enjoy fast cached responses throughout the whole website, while those with intent to buy something will get their unique cookies.

Other report:

The site is setting edd_wp_session cookies for every visitor, which makes thing a lot more painful, as nothing could be cached at the Nginx level & every request is dynamic.

If you use EDD_USE_PHP_SESSIONS set to false and fall back to WP_Session, you always get a cookie because it creates one in __construct(). Link.

[cklosowski]

We've got to make sure if we make any changes to this, that it doesn't break extensions that require the sessions, things like User History.

[pippinsplugins]

Let's try and resolve this in a point release that has almost nothing else in it. For now, milestoning for 2.5.9

[cklosowski]

looking into this I can almost get there. I can get sessions/cookies to only be created when adding to the cart, however for some reason the cookies don't carry over to page refreshes. All AJAX requests know about the current cart contents, but the front of site has no idea about it, which is strange.

[chriscct7]

This is going to break any extensions that rely on EDD sessions from the sound of it outside of the normal checkout flow

[pippinsplugins]

@chriscct7 Yes it will and we'll have to deal with that. Unfortunately, breaking caching on _every_ single page of _every single_ EDD site is far worse.

[cklosowski]

Every time I try and get this done, something major breaks in the cart system. I've taken a few stabs at it between getting other issues resolved. I'm going to punt (for now) and we can look to get this in a future release.

It's not that it's not important, it's just important to get right.

[bmoredrew]

Thanks for the effort everyone. Is there something we can do on an individual site level? My client has a pretty large site in terms of traffic that just falls over on Pagely when we try to implement EDD, so all the pieces are in place just kind of sitting there for a one day hopeful fix/use.

[cklosowski]

Drew,

At this point, we've already released some restrictions that stop it from
loading on the RSS feeds. On the EDD Site specifically we've stopped it
from starting sessions on the homepage with something like the following:

function eddwp_maybe_start_session( $start_session ) {
if ( '/' == $_SERVER['REQUEST_URI'] ) {
$start_session = false;
}

return $start_session;

}
add_filter( 'edd_start_session', 'eddwp_maybe_start_session', 10, 1 );

[pippinsplugins]

@bmoredrew One major improvement you could make is to prevent sessions from starting on specific pages, such as pages that will never have an add to cart button on them: https://github.com/easydigitaldownloads/easy-digital-downloads/issues/4429

[mindctrl]

This issue was mentioned to one of our customers by Pagely again. The sessions are preventing her site from being cached.

[pippinsplugins]

@mindctrl go ahead and show her how to exclude certain pages (such as home if possible) from starting caches.

[pippinsplugins]

WooCommerce 2.5 included a complete rewrite of their session manager. They originally used a very similar system to ours. Let's look at what they did to see if that will help.

https://woocommerce.wordpress.com/2015/10/07/new-session-handler-in-2-5/

[sunnyratilal]

Woo use a custom table to store this data and have a custom implementation built which handles sessions via this new table: https://github.com/kloon/woocommerce-large-sessions

[bcole808]

+1 to address this because EDD currently breaks nginx caching on every single page load for our site. We have a lot of pages that have nothing to do with store purchases, so this is important for our site.

The quick fix I'm implementing is to add the following filter which prevents a session from being started unless the request is related to purchasing EDD products.

On our site, I believe that the three rules below capture all possible requests that are related to product purchases (We are using very few EDD extensions, however). In case I've overlooked something.... please let me know!

function restrict_edd_session_creation($use_session) {

    // Allow session if the user has an item in their cart
    if ( isset( $_COOKIE['edd_items_in_cart'] ) ) {
        return $use_session;
    }

    // Allow session if this is an EDD ajax request
    if ( defined('DOING_AJAX') && DOING_AJAX && false !== strpos($_REQUEST['action'], 'edd_') ) {
        return $use_session;
    }

    // Allow session if this is a checkout/invoice/payment page
    if ( false !== strpos($_SERVER['REQUEST_URI'], '/checkout') ) {
        return $use_session;
    }

    // Otherwise, no EDD sessions allowed!
    return false;

}

add_filter('edd_start_session', 'restrict_edd_session_creation');

With regards to other extensions that require sessions on every page.... perhaps the base EDD plugin could have sessions disabled on most pages by default, and the extensions that require user sessions could enable the usage of sessions on more pages?

[pippinsplugins]

That looks like it may work as a solution for this. Nice.

[sunnyratilal]

PR opened and ready for testing

[pippinsplugins]

Removed PR and testing labels since the PR needs adjustment.

[cklosowski]

This is having the same problem I saw before, I can add to the cart, but then when I go to the checkout, it's empty. The AJAX request seems to get the session, but the browser doesn't seem to know about the session that was created during the AJAX request.

[sunnyratilal]

Tested and working _very_ well! Nice work @cklosowski 👍

[pippinsplugins]

I will test tomorrow.

[pippinsplugins]

Just pushed a small change to fix sessions not starting when custom add to cart links are used on pages other than the checkout, such as site.com/?edd_action=add_to_cart&download_id=23

[pippinsplugins]

Another item we'll need to take care of is the EDD_USE_CART_COOKIE constant and edd_use_cart_cookie filter. We introduced those in 2.5 as a method for site owners to disable the edd_items_in_cart cookie that broke caching on sites.

We needed to give site owners a way to remove that cookie since our sessions were _always_ getting started. Now that we have session starting working properly, we need to remove the option to disable the cart cookie since disabling it _completely_ breaks sessions.

I'm going to:

1. Make the constant and filter not do anything
2. Show deprecated notices with link to the dev blog post (which I'm writing later this week) on any site that is using the constant or filter to disable the cookie

[pippinsplugins]

Deprecation added for the constant and filter.

[pippinsplugins]

Dev blog post drafted: https://easydigitaldownloads.com/development/?p=170

[pippinsplugins]

This is triggering a fatal error when EDD_USE_PHP_SESSIONS is defined as false.

[20-Jan-2017 14:53:11 UTC] PHP Fatal error: Uncaught Error: Call to a member function get_queried_object() on null in /app/public/wp-includes/query.php:45
Stack trace:

0 /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-session.php(349): get_queried_object()

1 /app/public/wp-content/plugins/easy-digital-downloads/includes/class-edd-session.php(78): EDD_Session->should_start_session()

2 /app/public/wp-content/plugins/easy-digital-downloads/easy-digital-downloads.php(150): EDD_Session->__construct()

[sunnyratilal]

Looks like get_queried_object() is called way too early - before WP_Query is set up.

Currently, when EDD_USE_PHP_SESSIONS is false, we call EDD_Session::init on plugins_loaded but when true it's loaded on init.

[cklosowski]

This seems to resolve it being called to early, see if that works @sunnyratilal @pippinsplugins

[spencerfinnell]

@cklosowski That won't work with the default permalink structure.

[cklosowski]

@spencerfinnell well for few major versions, default has been altered from the query string params. I looked at url_to_postid but it's too early for that as well.

[pippinsplugins]

@cklosowski It's been a while since I played with but could we load EDD_Session on init even when EDD_USE_PHP_SESSIONS is false?

[cklosowski]

@pippinsplugins I'll give that a shot to see if that works.

[spencerfinnell]

That was going to be my next question as well. Seems like that would allow for the fewest special cases.

[cklosowski]

@pippinsplugins @spencerfinnell honestly, this seems to be working fine, I'll commit it up so we can have some more people test it out.

[pippinsplugins]

This appears to work perfectly for me!

Something did just occur to me: we will likely break any plugin that sets EDD session data before items are added to the cart.

@cklosowski @sunnyratilal could you both do some testing to confirm that session vars can still be set before items are added to the cart? We may need to call EDD()->session->set_cart_cookie( true ); any time something is added to the session instead of just when adding items to the cart.

[cklosowski]

Even doing that isn't seeming to work. @sunnyratilal can you give it a shot? I just used this to determine if it was working:

function test_stuff() {
    if ( isset( $_GET['test-stuff'] ) ) {
        EDD()->session->set( 'testing-cookie', 'testing-value' );
    }
        var_dump(EDD()->session->get( 'testing-cookie' );
}
add_action( 'init', 'test_stuff' );

Basically visit with ?test-stuff=true in the URL, and then see the cookie saved there in the session....then remove the query string, and visit the same page. My guess is, it's gone for you too?

[sunnyratilal]

@cklosowski the cookie is gone when removing the query string

[cklosowski]

Ok that sounds like what I was seeing too @sunnyratilal.

@pippinsplugins going to try and have to think through this, honestly, I'm ok if this doesn't make Beta 1....but it HAS to make Beta 2.

[pippinsplugins]

honestly, I'm ok if this doesn't make Beta 1....but it HAS to make Beta 2.

Nope, this will be in beta 1 or not in at all.

[pippinsplugins]

It's too important to the fundamental foundation of every EDD store to not have in beta 1.

[cklosowski]

Then I need someone to take over on it, Ive got too much time to invest
finishing up a few other things to have it ready in time, and frankly I'm
out of ideas on where to take it without rewriting the whole session
handling.

[cklosowski]

Ok, I determined why my test wasn't working, and it actually affects many things...anything that tries to run EDD()->session->set() will typically run on init or later due to our edd_action running at that point, but this means that headers_sent() will return true, including things like Discounts Pro, Cart Resuming, and SL actions.

We cannot run $_COOKIE['cookie-name'] on things after the headers were sent. Which means, that we cannot use the method to always set the items-in-cart cookie whenever set() is run.

We'll have to find another way around this.

[pippinsplugins]

:(

[sunnyratilal]

Closing this in favour of #5418 as this requires a fundamental rebuild to how sessions work.