-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Description
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();
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.