Skip to content

[HttpFoundation] unset Request properties within initialize() will cause issue on any extended Request implementing __isset() and __get() #51792

@crynobone

Description

@crynobone

Symfony version(s) affected

7.0.0

Description

Laravel Framework is preparing support for Symfony 7 for the upcoming Laravel 11 (target release Q1 2024) and based on our existing tests the usage such as protected string $baseUrl and using unset($this->baseUrl) will cause issues for Illuminate\Http\Request as this class extends Symfony\Component\HttpFoundation\Request and also implements __isset() and __get().

How to reproduce

The issue can be seen via our GitHub Actions: https://github.com/laravel/framework/actions/runs/6361912914/job/17276631512

To make it easier to understand, here is a simplified version of the usage:

class SymfonyRequest 
{
  protected string $baseUrl;

  public function __construct() 
  {
    unset($this->baseUrl);
  }

  public function getPathInfo()
  {
    if (null === ($baseUrl = $this->getBaseUrlReal())) {
      return 'http://localhost';
    }
    
    assert($baseUrl === 'foo', 'BaseURL should be `foo` from `prepareBaseUrl()` but instead is `'.$baseUrl.'`');

    return 'http://laravel.com';
  }

  private function getBaseUrlReal(): string
  {

    return $this->baseUrl ??= $this->prepareBaseUrl();
  }

  protected function prepareBaseUrl(): string
  {
    return 'foo';
  }
}

class LaravelRequest extends SymfonyRequest
{
    public function path() {
      return $this->getPathInfo();
    }

    public function __isset($key)
    {
      return ! is_null($this->__get($key));
    }
    
    public function __get($key)
    {
      if ($key === 'baseUrl') {
        return 'bar';
      }

      return null;
    }
}

$a = new LaravelRequest();

$a->path();

CleanShot 2023-10-01 at 11 00 07

Possible Solution

One possible solution is to opt for the following:

- protected string $baseUrl;
+protected ?string $baseUrl;

and set to null instead of unset.

-unset($this->baseUrl);
+$this->baseUrl = null;

Additional Context

Without the above change, any extended Request class similar to Laravel would need to look into exposing Symfony Request via __isset() and __get() which might not be ideal.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions