8000 Merge branch '4.3' into 4.4 · symfony/symfony@f24320d · GitHub
[go: up one dir, main page]

Skip to content
8000

Commit f24320d

Browse files
Merge branch '4.3' into 4.4
* 4.3: [Validator] fix access to uninitialized property when getting value [HttpClient] Fix regex bearer [HttpKernel] Fix stale-if-error behavior, add tests Improved error message when no supported user provider is found Properly handle phpunit arguments for configuration file
2 parents 55df55e + 0f13d5a commit f24320d

File tree

11 files changed

+261
-16
lines changed

11 files changed

+261
-16
lines changed

src/Symfony/Bridge/PhpUnit/bin/simple-phpunit.php

Lines changed: 39 additions & 7 deletions
+
break;
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,47 @@
2424

2525
static $phpunitConfig = null;
2626
if (null === $phpunitConfig) {
27-
$opt = min(array_search('-c', $opts = array_reverse($argv), true) ?: INF, array_search('--configuration', $opts, true) ?: INF);
2827
$phpunitConfigFilename = null;
29-
if (INF !== $opt && isset($opts[$opt - 1])) {
30-
$phpunitConfigFilename = $opts[$opt - 1];
31-
} elseif (file_exists('phpunit.xml')) {
32-
$phpunitConfigFilename = 'phpunit.xml';
33-
} elseif (file_exists('phpunit.xml.dist')) {
34-
$phpunitConfigFilename = 'phpunit.xml.dist';
28+
$getPhpUnitConfig = function ($probableConfig) use (&$getPhpUnitConfig) {
29+
if (!$probableConfig) {
30+
return null;
31+
}
32+
if (is_dir($probableConfig)) {
33+
return $getPhpUnitConfig($probableConfig.DIRECTORY_SEPARATOR.'phpunit.xml');
34+
}
35+
36+
if (file_exists($probableConfig)) {
37+
return $probableConfig;
38+
}
39+
if (file_exists($probableConfig.'.dist')) {
40+
return $probableConfig.'.dist';
41+
}
42+
43+
return null;
44+
};
45+
46+
foreach ($argv as $cliArgumentIndex => $cliArgument) {
47+
if ('--' === $cliArgument) {
48+
break;
49+
}
50+
// long option
51+
if ('--configuration' === $cliArgument && array_key_exists($cliArgumentIndex + 1, $argv)) {
52+
$phpunitConfigFilename = $getPhpUnitConfig($argv[$cliArgumentIndex + 1]);
53
54+
}
55+
// short option
56+
if (0 === strpos($cliArgument, '-c')) {
57+
if ('-c' === $cliArgument && array_key_exists($cliArgumentIndex + 1, $argv)) {
58+
$phpunitConfigFilename = $getPhpUnitConfig($argv[$cliArgumentIndex + 1]);
59+
} else {
60+
$phpunitConfigFilename = $getPhpUnitConfig(substr($cliArgument, 2));
61+
}
62+
break;
63+
}
3564
}
65+
66+
$phpunitConfigFilename = $phpunitConfigFilename ?: $getPhpUnitConfig('phpunit.xml');
67+
3668
if ($phpunitConfigFilename) {
3769
$phpunitConfig = new DomDocument();
3870
$phpunitConfig->load($phpunitConfigFilename);

src/Symfony/Component/HttpClient/HttpClientTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ private static function prepareRequest(?string $method, ?string $url, array $opt
110110
throw new InvalidArgumentException(sprintf('Option "auth_basic" must be string or an array, %s given.', \gettype($options['auth_basic'])));
111111
}
112112

113-
if (isset($options['auth_bearer']) && (!\is_string($options['auth_bearer']) || !preg_match('{^[-._~+/0-9a-zA-Z]++=*+$}', $options['auth_bearer']))) {
113+
if (isset($options['auth_bearer']) && (!\is_string($options['auth_bearer']) || !preg_match('{^[-._=~+/0-9a-zA-Z]++$}', $options['auth_bearer']))) {
114114
throw new InvalidArgumentException(sprintf('Option "auth_bearer" must be a string containing only characters from the base 64 alphabet, %s given.', \is_string($options['auth_bearer']) ? 'invalid string' : \gettype($options['auth_bearer'])));
115115
}
116116

src/Symfony/Component/HttpFoundation/Response.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ public function isImmutable(): bool
628628
}
629629

630630
/**
631-
* Returns true if the response must be revalidated by caches.
631+
* Returns true if the response must be revalidated by shared caches once it has become stale.
632632
*
633633
* This method indicates that the response must not be served stale by a
634634
* cache in any circumstance without first revalidating with the origin.

src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,13 +476,37 @@ protected function forward(Request $request, $catch = false, Response $entry = n
476476
// always a "master" request (as the real master request can be in cache)
477477
$response = SubRequestHandler::handle($this->kernel, $request, HttpKernelInterface::MASTER_REQUEST, $catch);
478478

479-
// we don't implement the stale-if-error on Requests, which is nonetheless part of the RFC
480-
if (null !== $entry && \in_array($response->getStatusCode(), [500, 502, 503, 504])) {
479+
/*
480+
* Support stale-if-error given on Responses or as a config option.
481+
* RFC 7234 summarizes in Section 4.2.4 (but also mentions with the individual
482+
* Cache-Control directives) that
483+
*
484+
* A cache MUST NOT generate a stale response if it is prohibited by an
485+
* explicit in-protocol directive (e.g., by a "no-store" or "no-cache"
486+
* cache directive, a "must-revalidate" cache-response-directive, or an
487+
* applicable "s-maxage" or "proxy-revalidate" cache-response-directive;
488+
* see Section 5.2.2).
489+
*
490+
* https://tools.ietf.org/html/rfc7234#section-4.2.4
491+
*
492+
* We deviate from this in one detail, namely that we *do* serve entries in the
493+
* stale-if-error case even if they have a `s-maxage` Cache-Control directive.
494+
*/
495+
if (null !== $entry
496+
&& \in_array($response->getStatusCode(), [500, 502, 503, 504])
497+
&& !$entry->headers->hasCacheControlDirective('no-cache')
498+
&& !$entry->mustRevalidate()
499+
) {
481500
if (null === $age = $entry->headers->getCacheControlDirective('stale-if-error')) {
482501
$age = $this->options['stale_if_error'];
483502
}
484503

485-
if (abs($entry->getTtl()) < $age) {
504+
/*
505+
* stale-if-error gives the (extra) time that the Response may be used *after* it has become stale.
506+
* So we compare the time the $entry has been sitting in the cache already with the
507+
* time it was fresh plus the allowed grace period.
508+
*/
509+
if ($entry->getAge() <= $entry->getMaxAge() + $age) {
486510
$this->record($request, 'stale-if-error');
487511

488512
return $entry;

src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,168 @@ public function testUsesOriginalRequestForSurrogate()
15221522
$cache->handle($request, HttpKernelInterface::SUB_REQUEST);
15231523
}
15241524

1525+
public function testStaleIfErrorMustNotResetLifetime()
1526+
{
1527+
// Make sure we don't accidentally treat the response as fresh (revalidated) again
1528+
// when stale-if-error handling kicks in.
1529+
1530+
$responses = [
1531+
[
1532+
'status' => 200,
1533+
'body' => 'OK',
1534+
// This is cacheable and can be used in stale-if-error cases:
1535+
'headers' => ['Cache-Control' => 'public, max-age=10', 'ETag' => 'some-etag'],
1536+
],
1537+
[
1538+
'status' => 500,
1539+
'body' => 'FAIL',
1540+
'headers' => [],
1541+
],
1542+
[
1543+
'status' => 500,
1544+
'body' => 'FAIL',
1545+
'headers' => [],
1546+
],
1547+
];
1548+
1549+
$this->setNextResponses($responses);
1550+
$this->cacheConfig['stale_if_error'] = 10;
1551+
1552+
$this->request('GET', '/'); // warm cache
1553+
1554+
sleep(15); // now the entry is stale, but still within the grace period (10s max-age + 10s stale-if-error)
1555+
1556+
$this->request('GET', '/'); // hit backend error
1557+
$this->assertEquals(200, $this->response->getStatusCode()); // stale-if-error saved the day
1558+
$this->assertEquals(15, $this->response->getAge());
1559+
1560+
sleep(10); // now we're outside the grace period
1561+
1562+
$this->request('GET', '/'); // hit backend error
1563+
$this->assertEquals(500, $this->response->getStatusCode()); // fail
1564+
}
1565+
1566+
/**
1567+
* @dataProvider getResponseDataThatMayBeServedStaleIfError
1568+
*/
1569+
public function testResponsesThatMayBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1570+
{
1571+
$responses = [
1572+
[
1573+
'status' => 200,
1574+
'body' => 'OK',
1575+
'headers' => $responseHeaders,
1576+
],
1577+
[
1578+
'status' => 500,
1579+
'body' => 'FAIL',
1580+
'headers' => [],
1581+
],
1582+
];
1583+
1584+
$this->setNextResponses($responses);
1585+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1586+
1587+
$this->request('GET', '/'); // warm cache
1588+
1589+
if ($sleepBetweenRequests) {
1590+
sleep($sleepBetweenRequests);
1591+
}
1592+
1593+
$this->request('GET', '/'); // hit backend error
1594+
1595+
$this->assertEquals(200, $this->response->getStatusCode());
1596+
$this->assertEquals('OK', $this->response->getContent());
1597+
$this->assertTraceContains('stale-if-error');
1598+
}
1599+
1600+
public function getResponseDataThatMayBeServedStaleIfError()
1601+
{
1602+
// All data sets assume that a 10s stale-if-error grace period has been configured
1603+
yield 'public, max-age expired' => [['Cache-Control' => 'public, max-age=60'], 65];
1604+
yield 'public, validateable with ETag, no TTL' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 5];
1605+
yield 'public, validateable with Last-Modified, no TTL' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 5];
1606+
yield 'public, s-maxage will be served stale-if-error, even if the RFC mandates otherwise' => [['Cache-Control' => 'public, s-maxage=20'], 25];
1607+
}
1608+
1609+
/**
1610+
* @dataProvider getResponseDataThatMustNotBeServedStaleIfError
1611+
*/
1612+
public function testResponsesThatMustNotBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1613+
{
1614+
$responses = [
1615+
[
1616+
'status' => 200,
1617+
'body' => 'OK',
1618+
'headers' => $responseHeaders,
1619+
],
1620+
[
1621+
'status' => 500,
1622+
'body' => 'FAIL',
1623+
'headers' => [],
1624+
],
1625+
];
1626+
1627+
$this->setNextResponses($responses);
1628+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1629+
$this->cacheConfig['strict_smaxage'] = true; // full RFC compliance for this feature
1630+
1631+
$this->request('GET', '/'); // warm cache
1632+
1633+
if ($sleepBetweenRequests) {
1634+
sleep($sleepBetweenRequests);
1635+
}
1636+
1637+
$this->request('GET', '/'); // hit backend error
1638+
1639+
$this->assertEquals(500, $this->response->getStatusCode());
1640+
}
1641+
1642+
public function getResponseDataThatMustNotBeServedStaleIfError()
1643+
{
1644+
// All data sets assume that a 10s stale-if-error grace period has been configured
1645+
yield 'public, no TTL but beyond grace period' => [['Cache-Control' => 'public'], 15];
1646+
yield 'public, validateable with ETag, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 15];
1647+
yield 'public, validateable with Last-Modified, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 15];
1648+
yield 'public, stale beyond grace period' => [['Cache-Control' => 'public, max-age=10'], 30];
1649+
1650+
// Cache-control values that prohibit serving stale responses or responses without positive validation -
1651+
// see https://tools.ietf.org/html/rfc7234#section-4.2.4 and
1652+
// https://tools.ietf.org/html/rfc7234#section-5.2.2
1653+
yield 'no-cache requires positive validation' => [['Cache-Control' => 'public, no-cache', 'ETag' => 'some-etag']];
1654+
yield 'no-cache requires positive validation, even if fresh' => [['Cache-Control' => 'public, no-cache, max-age=10']];
1655+
yield 'must-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, must-revalidate'], 15];
1656+
yield 'proxy-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, proxy-revalidate'], 15];
1657+
}
1658+
1659+
public function testStaleIfErrorWhenStrictSmaxageDisabled()
1660+
{
1661+
$responses = [
1662+
[
1663+
'status' => 200,
1664+
'body' => 'OK',
1665+
'headers' => ['Cache-Control' => 'public, s-maxage=20'],
1666+
],
1667+
[
1668+
'status' => 500,
1669+
'body' => 'FAIL',
1670+
'headers' => [],
1671+
],
1672+
];
1673+
1674+
$this->setNextResponses($responses);
1675+
$this->cacheConfig['stale_if_error'] = 10;
1676+
$this->cacheConfig['strict_smaxage'] = false;
1677+
1678+
$this->request('GET', '/'); // warm cache
1679+
sleep(25);
1680+
$this->request('GET', '/'); // hit backend error
1681+
1682+
$this->assertEquals(200, $this->response->getStatusCode());
1683+
$this->assertEquals('OK', $this->response->getContent());
1684+
$this->assertTraceContains('stale-if-error');
1685+
}
1686+
15251687
public function testTraceHeaderNameCanBeChanged()
15261688
{
15271689
$this->cacheConfig['trace_header'] = 'X-My-Header';

src/Symfony/Component/Security/Core/User/ChainUserProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function refreshUser(UserInterface $user)
9191
$e->setUsername($user->getUsername());
9292
throw $e;
9393
} else {
94-
throw new UnsupportedUserException(sprintf('The account "%s" is not supported.', \get_class($user)));
94+
throw new UnsupportedUserException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', \get_class($user)));
9595
}
9696
}
9797

src/Symfony/Component/Security/Http/Firewall/ContextListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ protected function refreshUser(TokenInterface $token)
292292
return null;
293293
}
294294

295-
throw new \RuntimeException(sprintf('There is no user provider for user "%s".', $userClass));
295+
throw new \RuntimeException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', $userClass));
296296
}
297297

298298
private function safelyUnserialize(string $serializedToken)

src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ final protected function getUserProvider(string $class): UserProviderInterface
234234
}
235235
}
236236

237-
throw new UnsupportedUserException(sprintf('There is no user provider that supports class "%s".', $class));
237+
throw new UnsupportedUserException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', $class));
238238
}
239239

240240
/**

src/Symfony/Component/Validator/Mapping/PropertyMetadata.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,13 @@ public function __construct(string $class, string $name)
4848
*/
4949
public function getPropertyValue($object)
5050
{
51-
return $this->getReflectionMember($object)->getValue($object);
51+
$reflProperty = $this->getReflectionMember($object);
52+
53+
if (\PHP_VERSION_ID >= 70400 && !$reflProperty->isInitialized($object)) {
54+
return null;
55+
}
56+
57+
return $reflProperty->getValue($object);
5258
}
5359

5460
/**
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace Symfony\Component\Validator\Tests\Fixtures;
4+
5+
class Entity_74
6+
{
7+
public int $uninitialized;
8+
}

src/Symfony/Component/Validator/Tests/Mapping/PropertyMetadataTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Validator\Mapping\PropertyMetadata;
1616
use Symfony\Component\Validator\Tests\Fixtures\Entity;
17+
use Symfony\Component\Validator\Tests\Fixtures\Entity_74;
1718

1819
class PropertyMetadataTest extends TestCase
1920
{
2021
const CLASSNAME = 'Symfony\Component\Validator\Tests\Fixtures\Entity';
22+
const CLASSNAME_74 = 'Symfony\Component\Validator\Tests\Fixtures\Entity_74';
2123
const PARENTCLASS = 'Symfony\Component\Validator\Tests\Fixtures\EntityParent';
2224

2325
public function testInvalidPropertyName()
@@ -53,4 +55,15 @@ public function testGetPropertyValueFromRemovedProperty()
5355
$this->expectException('Symfony\Component\Validator\Exception\ValidatorException');
5456
$metadata->getPropertyValue($entity);
5557
}
58+
59+
/**
60+
* @requires PHP 7.4
61+
*/
62+
public function testGetPropertyValueFromUninitializedProperty()
63+
{
64+
$entity = new Entity_74();
65+
$metadata = new PropertyMetadata(self::CLASSNAME_74, 'uninitialized');
66+
67+
$this->assertNull($metadata->getPropertyValue($entity));
68+
}
5669
}

0 commit comments

Comments
 (0)
0