Merge branch 'master' into 4.next

phpstan-baseline.neon

@@ -1,11 +1,6 @@
 parameters:
 	ignoreErrors:
 		-
-			message: "#^Method Cake\\\\Auth\\\\BaseAuthenticate\\:\\:unauthenticated\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Auth/BaseAuthenticate.php
-
-		-
 			message: "#^Strict comparison using \\=\\=\\= between string and false will always evaluate to false\\.$#"
 			count: 1
 			path: src/Auth/DigestAuthenticate.php
@@ -136,11 +131,6 @@ parameters:
 			path: src/Collection/Iterator/NoChildrenIterator.php
 
 		-
-			message: "#^Method Cake\\\\Command\\\\Command\\:\\:execute\\(\\) should return int\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Command/Command.php
-
-		-
 			message: "#^Parameter \\#1 \\$var_array of function extract is passed by reference, so it expects variables only\\.$#"
 			count: 1
 			path: src/Command/I18nExtractCommand.php
@@ -151,16 +141,6 @@ parameters:
 			path: src/Command/I18nExtractCommand.php
 
 		-
-			message: "#^Result of \\|\\| is always true\\.$#"
-			count: 1
-			path: src/Command/I18nExtractCommand.php
-
-		-
-			message: "#^Unreachable statement \\- code above always terminates\\.$#"
-			count: 1
-			path: src/Command/I18nExtractCommand.php
-
-		-
 			message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#"
 			count: 1
 			path: src/Console/CommandCollection.php
@@ -201,26 +181,6 @@ parameters:
 			path: src/Controller/Controller.php
 
 		-
-			message: "#^Method Cake\\\\Controller\\\\Controller\\:\\:beforeFilter\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Controller/Controller.php
-
-		-
-			message: "#^Method Cake\\\\Controller\\\\Controller\\:\\:beforeRender\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Controller/Controller.php
-
-		-
-			message: "#^Method Cake\\\\Controller\\\\Controller\\:\\:beforeRedirect\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Controller/Controller.php
-
-		-
-			message: "#^Method Cake\\\\Controller\\\\Controller\\:\\:afterFilter\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Controller/Controller.php
-
-		-
 			message: "#^Parameter \\#1 \\$request of method Cake\\\\Controller\\\\ControllerFactory\\:\\:getControllerClass\\(\\) expects Cake\\\\Http\\\\ServerRequest, Psr\\\\Http\\\\Message\\\\ServerRequestInterface given\\.$#"
 			count: 1
 			path: src/Controller/ControllerFactory.php
@@ -231,11 +191,6 @@ parameters:
 			path: src/Controller/ControllerFactory.php
 
 		-
-			message: "#^Method Cake\\\\Controller\\\\ErrorController\\:\\:beforeRender\\(\\) should return Cake\\\\Http\\\\Response\\|void\\|null but return statement is missing\\.$#"
-			count: 1
-			path: src/Controller/ErrorController.php
-
-		-
 			message: "#^Parameter \\#1 \\$autoload_function of function spl_autoload_register expects callable\\(string\\)\\: void, array\\(\\$this\\(Cake\\\\Core\\\\ClassLoader\\), 'loadClass'\\) given\\.$#"
 			count: 1
 			path: src/Core/ClassLoader.php

src/Auth/BaseAuthenticate.php

@@ -123,6 +123,16 @@ abstract class BaseAuthenticate implements EventListenerInterface
         if ($password !== null) {
             $hasher = $this->passwordHasher();
             $hashedPassword = $result->get($passwordField);
+
+            if ($hashedPassword === null || $hashedPassword === '') {
+                // Waste time hashing the password, to prevent
+                // timing side-channels to distinguish whether
+                // user has password or not.
+                $hasher->hash($password);
+
+                return false;
+            }
+
             if (!$hasher->check($password, $hashedPassword)) {
                 return false;
             }

src/Mailer/Message.php

@@ -1851,7 +1851,7 @@ class Message implements JsonSerializable, Serializable
         });
 
         return array_filter($array, function ($i) {
-            return !is_array($i) && !is_null($i) && strlen($i) || !empty($i);
+            return $i !== null && !is_array($i) && !is_bool($i) && strlen($i) || !empty($i);
         });
     }
 

src/View/Helper/PaginatorHelper.php

@@ -1072,7 +1072,7 @@ class PaginatorHelper extends Helper
         } elseif ($params['page'] > 1 && is_string($first)) {
             $first = $options['escape'] ? h($first) : $first;
             $out .= $this->templater()->format('first', [
-                'url' => $this->generateUrl(['page' => 1], $options['model']),
+                'url' => $this->generateUrl(['page' => 1], $options['model'], $options['url']),
                 'text' => $first,
             ]);
         }
@@ -1132,7 +1132,7 @@ class PaginatorHelper extends Helper
         } elseif ($params['page'] < $params['pageCount'] && is_string($last)) {
             $last = $options['escape'] ? h($last) : $last;
             $out .= $this->templater()->format('last', [
-                'url' => $this->generateUrl(['page' => $params['pageCount']], $options['model']),
+                'url' => $this->generateUrl(['page' => $params['pageCount']], $options['model'], $options['url']),
                 'text' => $last,
             ]);
         }

tests/Fixture/DatatypesFixture.php

@@ -27,7 +27,7 @@ class DatatypesFixture extends TestFixture
     public $fields = [
         'id' => ['type' => 'biginteger'],
         'cost' => ['type' => 'decimal', 'length' => 20, 'precision' => 1, 'null' => true],
-        'fraction' => ['type' => 'decimal', 'length' => 15, 'precision' => 14, 'null' => true],
+        'fraction' => ['type' => 'decimal', 'length' => 20, 'precision' => 19, 'null' => true],
         'floaty' => ['type' => 'float', 'null' => true],
         'small' => ['type' => 'smallinteger', 'null' => true],
         'tiny' => ['type' => 'tinyinteger', 'null' => true],

tests/TestCase/Auth/DigestAuthenticateTest.php

@@ -65,6 +65,7 @@ class DigestAuthenticateTest extends TestCase
             'nonce' => 123,
             'opaque' => '123abc',
             'secret' => Security::getSalt(),
+            'passwordHasher' => 'ShouldNeverTryToUsePasswordHasher',
         ]);
 
         $password = DigestAuthenticate::password('mariano', 'cake', 'localhost');
@@ -109,8 +110,6 @@ class DigestAuthenticateTest extends TestCase
      */
     public function testAuthenticateWrongUsername(): void
     {
-        $this->expectException(\Cake\Http\Exception\UnauthorizedException::class);
-        $this->expectExceptionCode(401);
         $request = new ServerRequest(['url' => 'posts/index']);
 
         $data = [
@@ -125,6 +124,10 @@ class DigestAuthenticateTest extends TestCase
         $data['response'] = $this->auth->generateResponseHash($data, '09faa9931501bf30f0d4253fa7763022', 'GET');
         $request = $request->withEnv('PHP_AUTH_DIGEST', $this->digestHeader($data));
 
+        $this->assertFalse($this->auth->authenticate($request, new Response()));
+
+        $this->expectException(UnauthorizedException::class);
+        $this->expectExceptionCode(401);
         $this->auth->unauthenticated($request, new Response());
     }
 
@@ -497,7 +500,7 @@ DIGEST;
             'opaque' => '123abc',
         ];
         $digest = <<<DIGEST
-Digest username="mariano",
+Digest username="{$data['username']}",
 realm="{$data['realm']}",
 nonce="{$data['nonce']}",
 uri="{$data['uri']}",

tests/TestCase/Auth/FormAuthenticateTest.php

@@ -23,6 +23,7 @@ use Cake\Http\ServerRequest;
 use Cake\I18n\Time;
 use Cake\ORM\Entity;
 use Cake\TestSuite\TestCase;
+use TestApp\Auth\CallCounterPasswordHasher;
 
 /**
  * Test case for FormAuthentication
@@ -487,4 +488,50 @@ class FormAuthenticateTest extends TestCase
         $this->assertNotEmpty($result);
         $this->assertTrue($this->auth->needsPasswordRehash());
     }
+
+    /**
+     * Tests that password hasher function is called exactly once in all cases.
+     *
+     * @param string $username
+     * @param string|null $password
+     * @return void
+     * @dataProvider userList
+     */
+    public function testAuthenticateSingleHash(string $username, ?string $password): void
+    {
+        $this->auth = new FormAuthenticate($this->collection, [
+            'userModel' => 'Users',
+            'passwordHasher' => CallCounterPasswordHasher::class,
+        ]);
+        $this->getTableLocator()->get('Users')->updateAll(
+            ['password' => $password],
+            ['username' => $username]
+        );
+
+        $request = new ServerRequest([
+            'url' => 'posts/index',
+            'post' => [
+                'username' => $username,
+                'password' => 'anything',
+            ],
+        ]);
+        $result = $this->auth->authenticate($request, new Response());
+        $this->assertFalse($result);
+
+        /** @var \TestApp\Auth\CallCounterPasswordHasher $passwordHasher */
+        $passwordHasher = $this->auth->passwordHasher();
+
+        $this->assertInstanceOf(CallCounterPasswordHasher::class, $passwordHasher);
+        $this->assertSame(1, $passwordHasher->callCount);
+    }
+
+    public function userList()
+    {
+        return [
+            ['notexist', ''],
+            ['mariano', null],
+            ['mariano', ''],
+            ['mariano', 'somehash'],
+        ];
+    }
 }

tests/TestCase/Database/Type/DateTimeTypeTest.php

@@ -331,11 +331,13 @@ class DateTimeTypeTest extends TestCase
     public function testMarshalWithLocaleParsing()
     {
         $this->type->useLocaleParser();
+
         $expected = new Time('13-10-2013 23:28:00');
         $result = $this->type->marshal('10/13/2013 11:28pm');
         $this->assertEquals($expected, $result);
-
         $this->assertNull($this->type->marshal('11/derp/2013 11:28pm'));
+
+        $this->type->useLocaleParser(false);
     }
 
     /**
@@ -346,9 +348,12 @@ class DateTimeTypeTest extends TestCase
     public function testMarshalWithLocaleParsingWithFormat()
     {
         $this->type->useLocaleParser()->setLocaleFormat('dd MMM, y hh:mma');
+
         $expected = new Time('13-10-2013 13:54:00');
         $result = $this->type->marshal('13 Oct, 2013 01:54pm');
         $this->assertEquals($expected, $result);
+
+        $this->type->useLocaleParser(false)->setLocaleFormat(null);
     }
 
     /**

tests/TestCase/Database/Type/DateTypeTest.php

@@ -208,11 +208,13 @@ class DateTypeTest extends TestCase
     public function testMarshalWithLocaleParsing()
     {
         $this->type->useLocaleParser();
+
         $expected = new Date('13-10-2013');
         $result = $this->type->marshal('10/13/2013');
         $this->assertEquals($expected->format('Y-m-d'), $result->format('Y-m-d'));
-
         $this->assertNull($this->type->marshal('11/derp/2013'));
+
+        $this->type->useLocaleParser(false);
     }
 
     /**
@@ -223,9 +225,12 @@ class DateTypeTest extends TestCase
     public function testMarshalWithLocaleParsingWithFormat()
     {
         $this->type->useLocaleParser()->setLocaleFormat('dd MMM, y');
+
         $expected = new Date('13-10-2013');
         $result = $this->type->marshal('13 Oct, 2013');
         $this->assertEquals($expected->format('Y-m-d'), $result->format('Y-m-d'));
+
+        $this->type->useLocaleParser(false)->setLocaleFormat(null);
     }
 
     /**

tests/TestCase/Database/Type/DecimalTypeTest.php

@@ -18,7 +18,6 @@ namespace Cake\Test\TestCase\Database\Type;
 
 use Cake\Database\Driver;
 use Cake\Database\Type\DecimalType;
-use Cake\Database\TypeFactory;
 use Cake\I18n\I18n;
 use Cake\TestSuite\TestCase;
 use PDO;
@@ -56,7 +55,7 @@ class DecimalTypeTest extends TestCase
     public function setUp(): void
     {
         parent::setUp();
-        $this->type = TypeFactory::build('decimal');
+        $this->type = new DecimalType();
         $this->driver = $this->getMockBuilder(Driver::class)->getMock();
         $this->localeString = I18n::getLocale();
         $this->numberClass = DecimalType::$numberClass;
@@ -214,23 +213,24 @@ class DecimalTypeTest extends TestCase
      */
     public function testMarshalWithLocaleParsing()
     {
-        I18n::setLocale('de_DE');
         $this->type->useLocaleParser();
+
+        I18n::setLocale('de_DE');
         $expected = 1234.53;
         $result = $this->type->marshal('1.234,53');
         $this->assertEquals($expected, $result);
 
         I18n::setLocale('en_US');
-        $this->type->useLocaleParser();
         $expected = 1234;
         $result = $this->type->marshal('1,234');
         $this->assertEquals($expected, $result);
 
         I18n::setLocale('pt_BR');
-        $this->type->useLocaleParser();
         $expected = 5987123.231;
         $result = $this->type->marshal('5.987.123,231');
         $this->assertEquals($expected, $result);
+
+        $this->type->useLocaleParser(false);
     }
 
     /**
@@ -240,12 +240,14 @@ class DecimalTypeTest extends TestCase
      */
     public function testMarshalWithLocaleParsingDanish()
     {
-        I18n::setLocale('da_DK');
-
         $this->type->useLocaleParser();
+
+        I18n::setLocale('da_DK');
         $expected = '47500';
         $result = $this->type->marshal('47.500');
         $this->assertSame($expected, $result);
+
+        $this->type->useLocaleParser(false);
     }
 
     /**

tests/TestCase/Database/Type/FloatTypeTest.php

@@ -17,7 +17,6 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Database\Type;
 
 use Cake\Database\Type\FloatType;
-use Cake\Database\TypeFactory;
 use Cake\I18n\I18n;
 use Cake\TestSuite\TestCase;
 use PDO;
@@ -55,7 +54,7 @@ class FloatTypeTest extends TestCase
     public function setUp(): void
     {
         parent::setUp();
-        $this->type = TypeFactory::build('float');
+        $this->type = new FloatType();
         $this->driver = $this->getMockBuilder('Cake\Database\Driver')->getMock();
         $this->localeString = I18n::getLocale();
         $this->numberClass = FloatType::$numberClass;
@@ -176,23 +175,24 @@ class FloatTypeTest extends TestCase
      */
     public function testMarshalWithLocaleParsing()
     {
-        I18n::setLocale('de_DE');
         $this->type->useLocaleParser();
+
+        I18n::setLocale('de_DE');
         $expected = 1234.53;
         $result = $this->type->marshal('1.234,53');
         $this->assertEquals($expected, $result);
 
         I18n::setLocale('en_US');
-        $this->type->useLocaleParser();
         $expected = 1234;
         $result = $this->type->marshal('1,234');
         $this->assertEquals($expected, $result);
 
         I18n::setLocale('pt_BR');
-        $this->type->useLocaleParser();
         $expected = 5987123.231;
         $result = $this->type->marshal('5.987.123,231');
         $this->assertEquals($expected, $result);
+
+        $this->type->useLocaleParser(false);
     }
 
     /**

tests/TestCase/Database/Type/TimeTypeTest.php

@@ -230,11 +230,13 @@ class TimeTypeTest extends TestCase
     public function testMarshalWithLocaleParsing()
     {
         $this->type->useLocaleParser();
+
         $expected = new Time('23:23:00');
         $result = $this->type->marshal('11:23pm');
         $this->assertEquals($expected->format('H:i'), $result->format('H:i'));
-
         $this->assertNull($this->type->marshal('derp:23'));
+
+        $this->type->useLocaleParser(false);
     }
 
     /**
@@ -247,11 +249,14 @@ class TimeTypeTest extends TestCase
         $updated = setlocale(LC_COLLATE, 'da_DK.utf8');
         $this->skipIf($updated === false, 'Could not set locale to da_DK.utf8, skipping test.');
 
-        I18n::setLocale('da_DK');
         $this->type->useLocaleParser();
+
+        I18n::setLocale('da_DK');
         $expected = new Time('03:20:00');
         $result = $this->type->marshal('03.20');
         $this->assertEquals($expected->format('H:i'), $result->format('H:i'));
+
+        $this->type->useLocaleParser(false);
     }
 
     /**

tests/TestCase/ORM/QueryTest.php

@@ -3187,7 +3187,7 @@ class QueryTest extends TestCase
         $entity->tiny = 1;
         $entity->small = 10;
 
-        $table->saveOrFail($entity);
+        $table->save($entity);
         $out = $table->find()
             ->where([
                 'cost' => $big,
@@ -3196,22 +3196,22 @@ class QueryTest extends TestCase
         $this->assertNotEmpty($out, 'Should get a record');
         $this->assertSame($big, $out->cost);
 
-        $small = '0.12345678901234';
+        $small = '0.1234567890123456789';
         $entity = $table->newEntity(['fraction' => $small]);
 
-        $table->saveOrFail($entity);
+        $table->save($entity);
         $out = $table->find()
             ->where([
                 'fraction' => $small,
             ])
             ->first();
         $this->assertNotEmpty($out, 'Should get a record');
-        $this->assertRegExp('/^0?\.12345678901234/', $out->fraction);
+        $this->assertRegExp('/^0?\.1234567890123456789$/', $out->fraction);
 
-        $small = 0.12345678901234;
+        $small = 0.1234567890123456789;
         $entity = $table->newEntity(['fraction' => $small]);
 
-        $table->saveOrFail($entity);
+        $table->save($entity);
         $out = $table->find()
             ->where([
                 'fraction' => $small,
@@ -3219,7 +3219,7 @@ class QueryTest extends TestCase
             ->first();
         $this->assertNotEmpty($out, 'Should get a record');
         // There will be loss of precision if too large/small value is set as float instead of string.
-        $this->assertRegExp('/^0?\.1234567890123\d+$/', $out->fraction);
+        $this->assertRegExp('/^0?\.123456789012350+$/', $out->fraction);
     }
 
     /**

tests/TestCase/View/Helper/PaginatorHelperTest.php

@@ -2659,6 +2659,16 @@ class PaginatorHelperTest extends TestCase
             '/li',
         ];
         $this->assertHtml($expected, $result);
+
+        $result = $this->Paginator->first('first', ['url' => ['action' => 'paged']]);
+        $expected = [
+            'li' => ['class' => 'first'],
+            'a' => ['href' => '/paged'],
+            'first',
+            '/a',
+            '/li',
+        ];
+        $this->assertHtml($expected, $result);
     }
 
     /**
@@ -2860,6 +2870,16 @@ class PaginatorHelperTest extends TestCase
 
         $result = $this->Paginator->last(3);
         $this->assertSame('', $result, 'When inside the last links range, no links should be made');
+
+        $result = $this->Paginator->last('lastest', ['url' => ['action' => 'paged']]);
+        $expected = [
+            'li' => ['class' => 'last'],
+            'a' => ['href' => '/paged?page=7'],
+            'lastest',
+            '/a',
+            '/li',
+        ];
+        $this->assertHtml($expected, $result);
     }
 
     /**

tests/test_app/TestApp/Auth/CallCounterPasswordHasher.php

@@ -0,0 +1,36 @@
+<?php
+declare(strict_types=1);
+
+namespace TestApp\Auth;
+
+use Cake\Auth\AbstractPasswordHasher;
+use InvalidArgumentException;
+
+class CallCounterPasswordHasher extends AbstractPasswordHasher
+{
+    public $callCount = 0;
+
+    /**
+     * @inheritDoc
+     */
+    public function hash(string $password)
+    {
+        $this->callCount++;
+
+        return 'hash123';
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function check(string $password, string $hashedPassword): bool
+    {
+        if ($hashedPassword === '') {
+            throw new InvalidArgumentException('Empty hash not expected');
+        }
+
+        $this->callCount++;
+
+        return false;
+    }
+}