Build on php8.5(CI)

[77web]

A follow-up for the comment on the previous PR: #27 (review)

[basi]

Thanks for adding PHP 8.5 to CI. The new job already paid off: it caught the out-of-range float-to-int warnings this PR fixes.

I looked into whether the @ operator can be avoided. It can, at both call sites, and the result is better than silencing. I checked the replacements below against the current code on PHP 8.0.30, 8.4.24 and 8.5.11 (Docker), and against the php-src sources. Details are in the inline comments.

Should

• Security::cipher(): reduce the seed with fmod() instead of silencing the cast. The effective seed stays exactly the same (inline).
• Add known-answer tests for Security::cipher(). testCipher only checks round trips. A changed seed would still pass, while existing cipher-encrypted data (e.g. CookieComponent cookies, whose default type is cipher) could no longer be decrypted. The tests are worth adding even if you keep @.
• limit() in DboSource / Postgres / Sqlite / Sqlserver: clamp out-of-range floats before %u instead of silencing the cast. Here @ hides a pre-existing bug: huge values wrap around, e.g. to 0 (inline).

Notes (non-blocking)

• The 8.5 job doesn't catch deprecations raised after bootstrap. lib/Cake/bootstrap.php (L30) and Error.level in app/Config/core.php set error_reporting to E_ALL & ~E_DEPRECATED at runtime, which overrides error_reporting=-1 from setup-php.
  • The warnings fixed here are E_WARNING, so they were caught. The #27 fix (PDO::MYSQL_ATTR_* → Pdo\Mysql::ATTR_*), however, is still not verified by CI.
  • lib/Cake/Test/bootstrap.php already converts deprecations to exceptions, so re-enabling E_DEPRECATED there (e.g. error_reporting(E_ALL) at the end) would be enough. It will likely surface unrelated deprecations, though, so it fits a separate PR.
• PostgresTest::testLimit and SqliteTest::testLimit pass the same 3e29 offset, but they only run in the 8.0 pgsql / sqlite jobs. So the Postgres / Sqlite changes are not exercised on 8.5. Adding 8.5 × pgsql / sqlite to the matrix would cover them.
• Pre-existing, out of scope: in Sqlserver::limit(), an overflowed float offset fails the is_int() / ctype_digit() check, so it is silently dropped and the query falls back to TOP (the first rows). ctype_digit() with a float is also deprecated since PHP 8.1. Clamping $offset before that check would fix it. However, that switches the generated SQL to OFFSET … FETCH, and SQL Server isn't in CI, so a separate change seems safer.
• This PR now changes runtime code as well, so please list the Security / limit() changes in the PR description. If you adopt the clamp, a README Changelog entry with a BC note would help, since the generated SQL changes for float values ≥ 2^63.

[basi]

@ works here, but it has two downsides:

• It hides any other diagnostic raised in this expression. Also, on PHP 8 @ doesn't stop custom error handlers from being called. Handlers that still use the PHP 7 idiom error_reporting() === 0 would log this warning on every cipher() call, i.e. on every CookieComponent read and write with the default cipher type.
• The code keeps relying on a value the manual calls undefined: when a float is beyond the int range, "the result is undefined" (manual). PHP 8.5 happens to keep the old wrapped value and only adds the warning.

The out-of-range cast can be avoided entirely. srand() is an alias of mt_srand(), which uses the seed "interpreted as an unsigned 32 bit integer" (manual). fmod() is exact, so reducing the seed modulo 2^32 keeps the same low 32 bits as the old cast, and the result always fits in a 64-bit int:

Suggested change

	// cipherSeed usually exceeds PHP_INT_MAX. PHP 8.5+ warns on the lossy cast,
	// but the resulting seed must stay the same to decrypt existing data.
	srand(@(int)(float)Configure::read('Security.cipherSeed'));
	// mt_srand() only uses the low 32 bits of the seed, and fmod() is exact. This keeps
	// the seed of the former (int)(float) cast without casting an out-of-range float
	// to int (the seed usually exceeds PHP_INT_MAX, and PHP 8.5+ warns on such casts).
	// A non-finite seed maps to 0, as that cast did.
	$seed = (float)Configure::read('Security.cipherSeed');
	srand(is_finite($seed) ? (int)fmod($seed, 4294967296) : 0);

I compared this with the current expression on PHP 8.0.30, 8.4.24 and 8.5.11. For 3,022 seeds (random 1–40 digit numbers, negatives, fractions, the 2^32 / 2^63 / 2^64 boundaries, 1e999, '', null, 'abc'), both produced identical rand() sequences. The new expression raised no diagnostic on 8.5.11. On 32-bit PHP, a remainder outside the 32-bit int range would still warn (with the correct value). I don't think that needs extra code.

Known-answer tests would pin the output on every CI version, e.g. in SecurityTest:

/**
 * Known-answer vectors for Security::cipher(), generated with the former (int)(float)
 * seed cast (identical on PHP 8.0, 8.4 and 8.5): a seed within the int range, one
 * beyond PHP_INT_MAX and the default seed.
 *
 * @return array
 */
	public static function cipherKnownAnswerProvider() {
		return array(
			array('1234567890', 'fff77dcb883732592dd235ad622645e18405cb'),
			array('12345678901234567890', 'b209c722829c974771a534cebda99e52d70f26'),
			array('76859309657453542496749683645', 'a0236698a8b6c059f17f023f2666bfbe328fee'),
		);
	}

/**
 * Test that Security::cipher() output does not change, so existing data can still be decrypted.
 *
 * @dataProvider cipherKnownAnswerProvider
 * @param string $seed Security.cipherSeed value.
 * @param string $expected Expected ciphertext as hex.
 * @return void
 */
	public function testCipherKnownAnswer($seed, $expected) {
		Configure::write('Security.cipherSeed', $seed);
		$this->assertSame($expected, bin2hex(Security::cipher('The quick brown fox', 'my_key')));
	}

These vectors pass with the current code on 8.0.30, 8.4.24 and 8.5.11 (under @ on 8.5), and with the suggestion above.

[basi]

Here @ hides a pre-existing bug, not just noise. %u converts floats with modular arithmetic, so huge values wrap around. On PHP 8.4:

sprintf('%u', …) input	output
1.8446744073709552E+20 (offset in testOutOfVeryBigPageNumberGetsClamped)	0
18446744073709551615 (MySQL's "all rows" limit, 2^64 as a float)	0
3.0E+29 (testLimit)	5212423987472105472
9.2233720368547758E+18 (2^63)	9223372036854775808 (one past the signed BIGINT maximum)

PaginatorComponent casts a huge page to PHP_INT_MAX, so with a limit of 20 the offset is (PHP_INT_MAX - 1) * 20 = 1.8446744073709552E+20. That becomes LIMIT 0, 20, so the query fetches the first page instead of nothing. The paginator still throws NotFoundException afterwards, but a direct find('all', array('page' => PHP_INT_MAX, 'limit' => 20)) returns the first page. Numeric strings don't wrap: the same %u saturates '3e29' to 9223372036854775807.

Clamping floats the same way removes both the warning and the wraparound. A protected helper in DboSource could look like this. PHP_INT_MAX is compared as the float 2^63, the first value that no longer fits:

/**
 * Clamps a float LIMIT/OFFSET value that does not fit in an int.
 *
 * sprintf('%u') wraps such floats around (2^64 becomes 0), and PHP 8.5+ warns about
 * that cast. Numeric strings are already saturated by the same cast.
 *
 * @param mixed $value Limit or offset value.
 * @return mixed PHP_INT_MAX for floats that do not fit in an int, $value otherwise.
 */
	protected function _clampLimitValue($value) {
		if (is_float($value) && $value >= PHP_INT_MAX) {
			return PHP_INT_MAX;
		}
		return $value;
	}

It can replace each @ one to one in DboSource, Postgres, Sqlite and Sqlserver. Here, for example:

			if ($offset) {
				$rt .= sprintf(' %u,', $this->_clampLimitValue($offset));
			}

			$rt .= sprintf(' %u', $this->_clampLimitValue($limit));

On PHP 8.4, only floats ≥ 2^63 (and INF) change. Ints, numeric and non-numeric strings, null, bools and in-range floats give the same output as before. On 8.5.11, sprintf('%u') raised no warning for any of these inputs once they went through the helper. Model / PaginatorComponent never produce negative overflow or NAN, so the helper leaves those alone. The testLimit cases here and in PostgresTest / SqliteTest could then assert the exact result instead of only checking for scientific notation, e.g. $this->assertSame(' LIMIT ' . PHP_INT_MAX . ', 10', $db->limit(10, 300000000000000000000000000000));.