diff --git a/Controller/ArgumentResolver/RequestPayloadValueResolver.php b/Controller/ArgumentResolver/RequestPayloadValueResolver.php index 2b1c420844..bac746c7ab 100644 --- a/Controller/ArgumentResolver/RequestPayloadValueResolver.php +++ b/Controller/ArgumentResolver/RequestPayloadValueResolver.php @@ -232,6 +232,10 @@ private function mapRequestPayload(Request $request, ArgumentMetadata $argument, private function mapUploadedFile(Request $request, ArgumentMetadata $argument, MapUploadedFile $attribute): UploadedFile|array|null { - return $request->files->get($attribute->name ?? $argument->getName(), []); + if (!($files = $request->files->get($attribute->name ?? $argument->getName())) && ($argument->isNullable() || $argument->hasDefaultValue())) { + return null; + } + + return $files ?? ('array' === $argument->getType() ? [] : null); } } diff --git a/DataCollector/RouterDataCollector.php b/DataCollector/RouterDataCollector.php index cb7b233f5d..81d3533a63 100644 --- a/DataCollector/RouterDataCollector.php +++ b/DataCollector/RouterDataCollector.php @@ -40,7 +40,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep $this->data['redirect'] = true; $this->data['url'] = $response->getTargetUrl(); - if ($this->controllers->contains($request)) { + if ($this->controllers->offsetExists($request)) { $this->data['route'] = $this->guessRoute($request, $this->controllers[$request]); } } diff --git a/EventListener/CacheAttributeListener.php b/EventListener/CacheAttributeListener.php index e913edf9e5..436e031bbb 100644 --- a/EventListener/CacheAttributeListener.php +++ b/EventListener/CacheAttributeListener.php @@ -165,7 +165,6 @@ public function onKernelResponse(ResponseEvent $event): void } if (true === $cache->noStore) { - $response->setPrivate(); $response->headers->addCacheControlDirective('no-store'); } diff --git a/EventListener/ErrorListener.php b/EventListener/ErrorListener.php index 18e8bff441..81f5dfb7fc 100644 --- a/EventListener/ErrorListener.php +++ b/EventListener/ErrorListener.php @@ -161,16 +161,16 @@ public static function getSubscribedEvents(): array /** * Logs an exception. - * + * * @param ?string $logChannel */ - protected function logException(\Throwable $exception, string $message, ?string $logLevel = null, /* ?string $logChannel = null */): void + protected function logException(\Throwable $exception, string $message, ?string $logLevel = null/* , ?string $logChannel = null */): void { - $logChannel = (3 < \func_num_args() ? \func_get_arg(3) : null) ?? $this->resolveLogChannel($exception); - + $logChannel = (3 < \func_num_args() ? func_get_arg(3) : null) ?? $this->resolveLogChannel($exception); + $logLevel ??= $this->resolveLogLevel($exception); - - if(!$logger = $this->getLogger($logChannel)) { + + if (!$logger = $this->getLogger($logChannel)) { return; } @@ -218,7 +218,7 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re $attributes = [ '_controller' => $this->controller, 'exception' => $exception, - 'logger' => DebugLoggerConfigurator::getDebugLogger($this->getLogger($exception)), + 'logger' => DebugLoggerConfigurator::getDebugLogger($this->getLogger($this->resolveLogChannel($exception))), ]; $request = $request->duplicate(null, null, $attributes); $request->setMethod('GET'); diff --git a/HttpCache/CacheWasLockedException.php b/HttpCache/CacheWasLockedException.php new file mode 100644 index 0000000000..f13946ad71 --- /dev/null +++ b/HttpCache/CacheWasLockedException.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpKernel\HttpCache; + +/** + * @internal + */ +class CacheWasLockedException extends \Exception +{ +} diff --git a/HttpCache/HttpCache.php b/HttpCache/HttpCache.php index 3ef1b8dcb8..2b1be6a95a 100644 --- a/HttpCache/HttpCache.php +++ b/HttpCache/HttpCache.php @@ -210,7 +210,13 @@ public function handle(Request $request, int $type = HttpKernelInterface::MAIN_R $this->record($request, 'reload'); $response = $this->fetch($request, $catch); } else { - $response = $this->lookup($request, $catch); + $response = null; + do { + try { + $response = $this->lookup($request, $catch); + } catch (CacheWasLockedException) { + } + } while (null === $response); } $this->restoreResponseBody($request, $response); @@ -560,15 +566,7 @@ protected function lock(Request $request, Response $entry): bool // wait for the lock to be released if ($this->waitForLock($request)) { - // replace the current entry with the fresh one - $new = $this->lookup($request); - $entry->headers = $new->headers; - $entry->setContent($new->getContent()); - $entry->setStatusCode($new->getStatusCode()); - $entry->setProtocolVersion($new->getProtocolVersion()); - foreach ($new->headers->getCookies() as $cookie) { - $entry->headers->setCookie($cookie); - } + throw new CacheWasLockedException(); // unwind back to handle(), try again } else { // backend is slow as hell, send a 503 response (to avoid the dog pile effect) $entry->setStatusCode(503); diff --git a/Kernel.php b/Kernel.php index bfef40fac5..42123cea19 100644 --- a/Kernel.php +++ b/Kernel.php @@ -73,14 +73,14 @@ abstract class Kernel implements KernelInterface, RebootableInterface, Terminabl */ private static array $freshCache = []; - public const VERSION = '7.3.0'; - public const VERSION_ID = 70300; + public const VERSION = '7.3.3-DEV'; + public const VERSION_ID = 70303; public const MAJOR_VERSION = 7; public const MINOR_VERSION = 3; - public const RELEASE_VERSION = 0; - public const EXTRA_VERSION = ''; + public const RELEASE_VERSION = 3; + public const EXTRA_VERSION = 'DEV'; - public const END_OF_MAINTENANCE = '05/2025'; + public const END_OF_MAINTENANCE = '01/2026'; public const END_OF_LIFE = '01/2026'; public function __construct( diff --git a/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php b/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php index 11ab6f36a1..c0005325a7 100644 --- a/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php +++ b/Tests/Controller/ArgumentResolver/UploadedFileValueResolverTest.php @@ -82,6 +82,35 @@ static function () {}, $request, HttpKernelInterface::MAIN_REQUEST ); + + $this->expectException(HttpException::class); + + $resolver->onKernelControllerArguments($event); + } + + /** + * @dataProvider provideContext + */ + public function testEmptyArrayArgument(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + 'array', + false, + false, + null, + false, + [$attribute::class => $attribute] + ); + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + $resolver->onKernelControllerArguments($event); $data = $event->getArguments()[0]; @@ -307,6 +336,96 @@ static function () {}, $resolver->onKernelControllerArguments($event); } + /** + * @dataProvider provideContext + */ + public function testShouldAllowEmptyWhenNullable(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + UploadedFile::class, + false, + false, + null, + true, + [$attribute::class => $attribute] + ); + /** @var HttpKernelInterface&MockObject $httpKernel */ + $httpKernel = $this->createMock(HttpKernelInterface::class); + $event = new ControllerArgumentsEvent( + $httpKernel, + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + $resolver->onKernelControllerArguments($event); + $data = $event->getArguments()[0]; + + $this->assertNull($data); + } + + /** + * @dataProvider provideContext + */ + public function testShouldAllowEmptyWhenNullableArray(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + 'array', + false, + false, + null, + true, + [$attribute::class => $attribute] + ); + /** @var HttpKernelInterface&MockObject $httpKernel */ + $httpKernel = $this->createMock(HttpKernelInterface::class); + $event = new ControllerArgumentsEvent( + $httpKernel, + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + $resolver->onKernelControllerArguments($event); + $data = $event->getArguments()[0]; + + $this->assertNull($data); + } + + /** + * @dataProvider provideContext + */ + public function testShouldAllowEmptyWhenHasDefaultValue(RequestPayloadValueResolver $resolver, Request $request) + { + $attribute = new MapUploadedFile(); + $argument = new ArgumentMetadata( + 'qux', + UploadedFile::class, + false, + true, + 'default-value', + false, + [$attribute::class => $attribute] + ); + /** @var HttpKernelInterface&MockObject $httpKernel */ + $httpKernel = $this->createMock(HttpKernelInterface::class); + $event = new ControllerArgumentsEvent( + $httpKernel, + static function () {}, + $resolver->resolve($request, $argument), + $request, + HttpKernelInterface::MAIN_REQUEST + ); + $resolver->onKernelControllerArguments($event); + $data = $event->getArguments()[0]; + + $this->assertSame('default-value', $data); + } + public static function provideContext(): iterable { $resolver = new RequestPayloadValueResolver( diff --git a/Tests/DataCollector/LoggerDataCollectorTest.php b/Tests/DataCollector/LoggerDataCollectorTest.php index 67dd853b40..0698dda774 100644 --- a/Tests/DataCollector/LoggerDataCollectorTest.php +++ b/Tests/DataCollector/LoggerDataCollectorTest.php @@ -33,7 +33,7 @@ public function testCollectWithUnexpectedFormat() $c = new LoggerDataCollector($logger, __DIR__.'/'); $c->lateCollect(); - $compilerLogs = $c->getCompilerLogs()->getValue('message'); + $compilerLogs = $c->getCompilerLogs()->getValue(true); $this->assertSame([ ['message' => 'Removed service "Psr\Container\ContainerInterface"; reason: private alias.'], diff --git a/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php b/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php index a4b1e91d0a..b281c873cb 100644 --- a/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php +++ b/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php @@ -510,7 +510,7 @@ public function testAutowireAttribute() $this->assertInstanceOf(\stdClass::class, $locator->get('serviceAsValue')); $this->assertInstanceOf(\stdClass::class, $locator->get('expressionAsValue')); $this->assertSame('bar', $locator->get('rawValue')); - $this->stringContains('Symfony_Component_HttpKernel_Tests_Fixtures_Suit_APP_SUIT', $locator->get('suit')); + $this->assertStringContainsString('Symfony_Component_HttpKernel_Tests_Fixtures_Suit_APP_SUIT', $locator->get('suit')); $this->assertSame('@bar', $locator->get('escapedRawValue')); $this->assertSame('foo', $locator->get('customAutowire')); $this->assertInstanceOf(FooInterface::class, $autowireCallable = $locator->get('autowireCallable')); diff --git a/Tests/EventListener/CacheAttributeListenerTest.php b/Tests/EventListener/CacheAttributeListenerTest.php index b185ea8994..d2c8ed0db6 100644 --- a/Tests/EventListener/CacheAttributeListenerTest.php +++ b/Tests/EventListener/CacheAttributeListenerTest.php @@ -102,18 +102,18 @@ public function testResponseIsPublicIfConfigurationIsPublicTrueNoStoreFalse() $this->assertFalse($this->response->headers->hasCacheControlDirective('no-store')); } - public function testResponseIsPrivateIfConfigurationIsPublicTrueNoStoreTrue() + public function testResponseKeepPublicIfConfigurationIsPublicTrueNoStoreTrue() { $request = $this->createRequest(new Cache(public: true, noStore: true)); $this->listener->onKernelResponse($this->createEventMock($request, $this->response)); - $this->assertFalse($this->response->headers->hasCacheControlDirective('public')); - $this->assertTrue($this->response->headers->hasCacheControlDirective('private')); + $this->assertTrue($this->response->headers->hasCacheControlDirective('public')); + $this->assertFalse($this->response->headers->hasCacheControlDirective('private')); $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store')); } - public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue() + public function testResponseKeepPrivateNoStoreIfConfigurationIsNoStoreTrue() { $request = $this->createRequest(new Cache(noStore: true)); @@ -124,14 +124,14 @@ public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue() $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store')); } - public function testResponseIsPrivateIfSharedMaxAgeSetAndNoStoreIsTrue() + public function testResponseIsPublicIfSharedMaxAgeSetAndNoStoreIsTrue() { $request = $this->createRequest(new Cache(smaxage: 1, noStore: true)); $this->listener->onKernelResponse($this->createEventMock($request, $this->response)); - $this->assertFalse($this->response->headers->hasCacheControlDirective('public')); - $this->assertTrue($this->response->headers->hasCacheControlDirective('private')); + $this->assertTrue($this->response->headers->hasCacheControlDirective('public')); + $this->assertFalse($this->response->headers->hasCacheControlDirective('private')); $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store')); } diff --git a/Tests/EventListener/RouterListenerTest.php b/Tests/EventListener/RouterListenerTest.php index ca7bb1b1f6..f980984943 100644 --- a/Tests/EventListener/RouterListenerTest.php +++ b/Tests/EventListener/RouterListenerTest.php @@ -354,7 +354,7 @@ public static function provideRouteMapping(): iterable '_route_mapping' => [ 'id' => [ 'article', - 'id' + 'id', ], 'date' => [ 'article', diff --git a/Tests/EventListener/SessionListenerTest.php b/Tests/EventListener/SessionListenerTest.php index deba6661f0..fda957bb49 100644 --- a/Tests/EventListener/SessionListenerTest.php +++ b/Tests/EventListener/SessionListenerTest.php @@ -104,13 +104,13 @@ public static function provideSessionOptions(): \Generator yield 'set_cookiesecure_auto_by_symfony_false_by_php' => [ 'phpSessionOptions' => ['secure' => false], - 'sessionOptions' => ['cookie_path' => '/test/', 'cookie_httponly' => 'auto', 'cookie_secure' => 'auto', 'cookie_samesite' => Cookie::SAMESITE_LAX], + 'sessionOptions' => ['cookie_path' => '/test/', 'cookie_httponly' => true, 'cookie_secure' => 'auto', 'cookie_samesite' => Cookie::SAMESITE_LAX], 'expectedSessionOptions' => ['cookie_path' => '/test/', 'cookie_domain' => '', 'cookie_secure' => false, 'cookie_httponly' => true, 'cookie_samesite' => Cookie::SAMESITE_LAX], ]; yield 'set_cookiesecure_auto_by_symfony_true_by_php' => [ 'phpSessionOptions' => ['secure' => true], - 'sessionOptions' => ['cookie_path' => '/test/', 'cookie_httponly' => 'auto', 'cookie_secure' => 'auto', 'cookie_samesite' => Cookie::SAMESITE_LAX], + 'sessionOptions' => ['cookie_path' => '/test/', 'cookie_httponly' => true, 'cookie_secure' => 'auto', 'cookie_samesite' => Cookie::SAMESITE_LAX], 'expectedSessionOptions' => ['cookie_path' => '/test/', 'cookie_domain' => '', 'cookie_secure' => true, 'cookie_httponly' => true, 'cookie_samesite' => Cookie::SAMESITE_LAX], ]; diff --git a/Tests/HttpCache/HttpCacheTest.php b/Tests/HttpCache/HttpCacheTest.php index e82e8fd81b..9f03f6d8e8 100644 --- a/Tests/HttpCache/HttpCacheTest.php +++ b/Tests/HttpCache/HttpCacheTest.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpKernel\Event\TerminateEvent; use Symfony\Component\HttpKernel\HttpCache\Esi; use Symfony\Component\HttpKernel\HttpCache\HttpCache; +use Symfony\Component\HttpKernel\HttpCache\Store; use Symfony\Component\HttpKernel\HttpCache\StoreInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Kernel; @@ -662,6 +663,7 @@ public function testDegradationWhenCacheLocked() */ sleep(10); + $this->store = $this->createStore(); // create another store instance that does not hold the current lock $this->request('GET', '/'); $this->assertHttpKernelIsNotCalled(); $this->assertEquals(200, $this->response->getStatusCode()); @@ -680,6 +682,64 @@ public function testDegradationWhenCacheLocked() $this->assertEquals('Old response', $this->response->getContent()); } + public function testHitBackendOnlyOnceWhenCacheWasLocked() + { + // Disable stale-while-revalidate, it circumvents waiting for the lock + $this->cacheConfig['stale_while_revalidate'] = 0; + + $this->setNextResponses([ + [ + 'status' => 200, + 'body' => 'initial response', + 'headers' => [ + 'Cache-Control' => 'public, no-cache', + 'Last-Modified' => 'some while ago', + ], + ], + [ + 'status' => 304, + 'body' => '', + 'headers' => [ + 'Cache-Control' => 'public, no-cache', + 'Last-Modified' => 'some while ago', + ], + ], + [ + 'status' => 500, + 'body' => 'The backend should not be called twice during revalidation', + 'headers' => [], + ], + ]); + + $this->request('GET', '/'); // warm the cache + + // Use a store that simulates a cache entry being locked upon first attempt + $this->store = new class(sys_get_temp_dir().'/http_cache') extends Store { + private bool $hasLock = false; + + public function lock(Request $request): bool + { + $hasLock = $this->hasLock; + $this->hasLock = true; + + return $hasLock; + } + + public function isLocked(Request $request): bool + { + return false; + } + }; + + $this->request('GET', '/'); // hit the cache with simulated lock/concurrency block + + $this->assertEquals(200, $this->response->getStatusCode()); + $this->assertEquals('initial response', $this->response->getContent()); + + $traces = $this->cache->getTraces(); + $this->assertSame(['stale', 'valid', 'store'], current($traces)); + } + public function testHitsCachedResponseWithSMaxAgeDirective() { $time = \DateTimeImmutable::createFromFormat('U', time() - 5); diff --git a/Tests/HttpCache/HttpCacheTestCase.php b/Tests/HttpCache/HttpCacheTestCase.php index 26a29f16b2..b05d913666 100644 --- a/Tests/HttpCache/HttpCacheTestCase.php +++ b/Tests/HttpCache/HttpCacheTestCase.php @@ -30,7 +30,7 @@ abstract class HttpCacheTestCase extends TestCase protected $responses; protected $catch; protected $esi; - protected Store $store; + protected ?Store $store = null; protected function setUp(): void { @@ -115,7 +115,9 @@ public function request($method, $uri = '/', $server = [], $cookies = [], $esi = $this->kernel->reset(); - $this->store = new Store(sys_get_temp_dir().'/http_cache'); + if (!$this->store) { + $this->store = $this->createStore(); + } if (!isset($this->cacheConfig['debug'])) { $this->cacheConfig['debug'] = true; @@ -183,4 +185,9 @@ public static function clearDirectory($directory) closedir($fp); } + + protected function createStore(): Store + { + return new Store(sys_get_temp_dir().'/http_cache'); + } }