Private tempstore should store a value that persists with the user's session.

However, if the user is anonymous, then this is broken, because saving to tempstore doesn't start a session.

You can see the problem by running this code (eg in Devel's exec PHP form). Each execution causes a new row in key_value_expire:

\Drupal::service('user.private_tempstore')->get('test')->set('test', 'test value!');

The expected behaviour is that after the first row is inserted, subsequent executions just update it.

Comments

joachim created an issue. See original summary.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Active » Needs review
FileSize
658 bytes

Here's a patch.

Going by http://drupal.stackexchange.com/questions/163142/what-is-the-right-way-t..., the best way to forcibly start a session for an anon user is to store something in it.

gambry’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

This is a quite critical issue as developers assume by default user.private_tempstore works for anonymous by storing something on the $_SESSION (as mentioned on several page either on d.org or stack*.com), but by using it they figure out something is wrong.

Manually tested and patch on #4 works.

We may need test coverage to prove problem is real and patch fixes it?

joachim’s picture

This is just a quick hack; I think if we're adding test coverage it would be better done as part of providing a cleaner way of doing this: #2865991: provide an API to forcibly start a session.

wesleymusgrove’s picture

I'm running into this issue trying to store a value in an anonymous visitor's `user.private_tempstore` from within a custom `hook_form_submit` handler. My purpose is to respond to a form being submitted by firing off some tracking events to Google Analytics, but I only want these events to fire once after the success URL has been reached.

My initial thought was to store this value in $form_state like:

<?php
$form_state->setValue('hidden_field', 'some value');
?>

However the $_POST body doesn't persist because I'm using the core Contact Form and specifying a redirect URL (this is also true for Webforms that redirect). After the form submits and redirects to the success URL, the $_POST values aren't available to me.

<?php
\Drupal::request()->request->get('hidden_field'); // form post is null after redirect
?>

So that's why I was opting to store this value in user.private_tempstore, like this:

<?php
function example_form_submit($form, \Drupal\Core\Form\FormStateInterface $form_state) {
  $input = $form_state->getUserInput();
  if (isset($input) && !empty($input['name']) && !empty($input['mail'])) {
    $tempstore = \Drupal::service('user.private_tempstore')->get('example_tempstore');
    $tempstore->set('form_submitted', TRUE);
  }
}
?>

And then printing out some tracking events in $page_bottom from a template like this:

<?php
function example_page_bottom(array &$page_bottom) {
  $tempstore = \Drupal::service('user.private_tempstore')->get('example_tempstore');

  //This is NULL because a session hasn't been started for the anonymous user.
  if ($form_submitted = $tempstore->get('form_submitted')) {
    $page_bottom['tracking_events'] = array(
      '#theme' => 'tracking_events',
      '#form_submitted' => $form_submitted,
    );

    //Unset this flag after this template is rendered the first time to prevent duplicate tracking events.
    $tempstore->set('form_submitted', FALSE);
    $tempstore->delete('form_submitted');
  }
}

function example_theme($existing, $type, $theme, $path) {
  return array(
    'tracking_events' => array(
      'variables' => array(
        'form_submitted' => NULL,
      ),
      'template' => 'tracking-events-twig-template',
    ),
  );
}
?>

The problem is just what this issue is about... When I check for the value in private temp store in the `hook_page_bottom`, it's NULL because a session hasn't yet been started for the anonymous user who submitted the form.

DedMoroz’s picture

#4 patch can not be applied after Drupal update to 8.3.1.
Here is reroll patch.

rackberg’s picture

I tested the patch of comment #8 using drupal 8.3.2 and can confirm that i am now able to use the private temporary storage as an anonymous user.

rackberg’s picture

Uploaded the patch again, trying to start the testing process for the patch.

agoradesign’s picture

Status: Needs work » Needs review

@rackberg: to start the testing process, you have to set the status to "Needs review"

The last submitted patch, 8: drupal-anonymous_session-2743931-8-D8.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: drupal-anonymous_session-2743931-8-D8.patch, failed testing.

Yogesh Pawar’s picture

Status: Needs work » Needs review
FileSize
659 bytes

Re-roll of #10 because it's failed to apply on 8.4.x

agoradesign’s picture

Thanks, just wanted to do the same :)

Dinesh18’s picture

Reviewed the patch #14. Looks good to me. +1 to RTBC

stijndmd’s picture

This patch doesn't fix the issue for me. I'm trying to do something very similar to what wesleymusgrove is doing in #7.

mrkdboyd’s picture

Patch in #14 worked for me on Drupal 8.3.2. +1 to RTBC

joachim’s picture

Status: Needs review » Reviewed & tested by the community

Ok, that's plenty of positive reviews -- let's set this to RTBC so it can get attention from a maintainer.

agoradesign’s picture

Good idea, hope this helps that this patch lands in 8.4

larowlan’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/user/src/PrivateTempStore.php
@@ -125,6 +125,13 @@ public function set($key, $value) {
+      $_SESSION['forced'] = TRUE;

We've worked actively in the Drupal 8 cycle to remove uses of the superglobals.

The correct way to do this, in a fashion that can be unit tested etc, is to use \Symfony\Component\HttpFoundation\Session\SessionInterface::set, which can be called on the Session object returned from calling getSession on the current request, which can be obtained by calling the getCurrentRequest method on the request_stack service.

As this service already has the request stack injected, we should definitely be using that approach over the superglobals.

Also, needs tests still

harsha012’s picture

Test not added. update the code as per the comment. please review

harsha012’s picture

Status: Needs work » Needs review
alexej_d’s picture

Status: Needs review » Needs work

@harsha012 I believe lerowlan meant using the injected service like so:

$this->requestStack->getCurrentRequest()->getSession()->set('forced', TRUE);
agoradesign’s picture

@alexej_d is right. The patch from #22 can't work at all, impossible

harsha012’s picture

Status: Needs work » Needs review
FileSize
702 bytes

@alexej_d,

Thanks for clarification.

added patch for that.