Problem/Motivation

Some forms desire to render the name of the currently logged in user (e.g., the comment form). If the form is stored in the cache, then the AccountProxy gets serialized. This currently does not seem to have any bad effects, but it does not make much sense to serialize the proxy either. Affected code typically looks like this:

$form['#theme'] = 'username';
$form['#account'] = $this->currentUser;

Proposed resolution

?

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#18 drupal-AccountProxy-should-not-be-serialized-2430447-18-test.patch651 bytesavpaderno
#7 2430447-07.patch589 bytesjhedstrom
#1 2430447-do-not-serialize-account-proxy.diff562 bytesznerol

Comments

znerol’s picture

znerol commented
StatusFileSize
new 2430447-do-not-serialize-account-proxy.diff562 bytes
znerol’s picture

znerol commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: 2430447-do-not-serialize-account-proxy.diff, failed testing.

znerol’s picture

znerol commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Major

Discussed with @znerol; this sounds probably at least major.

dawehner’s picture

dawehner
Primary language German
commented

So those places are all kinda problems?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new 2430447-07.patch589 bytes

Re-roll of #1 just to see where this is still an issue.

Status: Needs review » Needs work

The last submitted patch, 7: 2430447-07.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Not bad, down to only 1 instance where this happens!

The last submitted patch, 7: 2430447-07.patch, failed testing.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.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.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.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.6.x-dev » 8.9.x-dev
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-AccountProxy-should-not-be-serialized-2430447-18-test.patch651 bytes

Status: Needs review » Needs work

The last submitted patch, 18: drupal-AccountProxy-should-not-be-serialized-2430447-18-test.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

I am not sure this issue is still relevant. Starting from Drupal 8.8.x, the AccountProxy class uses the DependencySerializationTrait trait, which was not used on previous versions.

Drupal 8.8.x

namespace Drupal\Core\Session;

use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;

/**
 * A proxied implementation of AccountInterface.
 *
 * The reason why we need an account proxy is that we don't want to have global
 * state directly stored in the container.
 *
 * This proxy object avoids multiple invocations of the authentication manager
 * which can happen if the current user is accessed in constructors. It also
 * allows legacy code to change the current user where the user cannot be
 * directly injected into dependent code.
 */
class AccountProxy implements AccountProxyInterface {
  use DependencySerializationTrait;

  /**
   * The instantiated account.
   *
   * @var \Drupal\Core\Session\AccountInterface
   */
  protected $account;

  // Omissis
}

Drupal 8.1.x

namespace Drupal\Core\Session;


/**
 * A proxied implementation of AccountInterface.
 *
 * The reason why we need an account proxy is that we don't want to have global
 * state directly stored in the container.
 *
 * This proxy object avoids multiple invocations of the authentication manager
 * which can happen if the current user is accessed in constructors. It also
 * allows legacy code to change the current user where the user cannot be
 * directly injected into dependent code.
 */
class AccountProxy implements AccountProxyInterface {

  /**
   * The instantiated account.
   *
   * @var \Drupal\Core\Session\AccountInterface
   */
  protected $account;

  // Omissis
}

Using DependencySerializationTrait should resolve any issue, since it avoids the service container and any service are serialized. (See DependencySerializationTrait::__sleep().)

  $vars = get_object_vars($this);
  foreach ($vars as $key => $value) {
    if (is_object($value) && isset($value->_serviceId)) {

      // If a class member was instantiated by the dependency injection
      // container, only store its ID so it can be used to get a fresh object
      // on unserialization.
      $this->_serviceIds[$key] = $value->_serviceId;
      unset($vars[$key]);
    }
    elseif ($value instanceof ContainerInterface) {
      $this->_serviceIds[$key] = 'service_container';
      unset($vars[$key]);
    }
    elseif ($value instanceof EntityStorageInterface) {

      // If a class member is an entity storage, only store the entity type ID
      // the storage is for so it can be used to get a fresh object on
      // unserialization. By doing this we prevent possible memory leaks when
      // the storage is serialized when it contains a static cache of entity
      // objects and additionally we ensure that we'll not have multiple
      // storage objects for the same entity type and therefore prevent
      // returning different references for the same entity.
      $this->_entityStorages[$key] = $value
        ->getEntityTypeId();
      unset($vars[$key]);
    }

It seems this issue should be closed as outdated.

The comments on the AccountProxy class should be updated/removed, but that should be object of a new issue.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Active

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

pameeela’s picture

pameeela commented
Status: Active » Closed (outdated)
Issue tags: +Bug Smash Initiative

Per #20 this is outdated. I will create a follow up task to update the comment but I don't know what specifically should be updated.