fix: add int type hint to gc() in all session handlers - #10418
Conversation
michalsn
left a comment
There was a problem hiding this comment.
Quite an unrealistic scenario, but still a valid fix.
Please add a changelog entry.
- Add Unreleased section to CHANGELOG.md with PR codeigniter4#10418 entry - Consolidate testGCRejectsNonInt to test 6 non-int types in one method - Remove unrelated testGCWithZero and testGCWithNegativeValue tests - Add v4.7.5 RST changelog entry for session gc type hint
4079bbb to
fc5d53c
Compare
michalsn
left a comment
There was a problem hiding this comment.
Please rebase into the 4.8 branch, as this will be a more appropriate release for these changes. Also, please change this method signature for all handlers.
| $this->fail('TypeError expected for value: ' . gettype($malicious)); | ||
| } catch (TypeError) { | ||
| $this->addToAssertionCount(1); | ||
| } |
There was a problem hiding this comment.
Please use DataProvider instead of a loop.
|
|
||
| - **Content Security Policy:** Fixed a bug where empty ``Content-Security-Policy``, ``Content-Security-Policy-Report-Only``, and ``Reporting-Endpoints`` response headers were generated when no corresponding values existed. | ||
| - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. | ||
| - **Session:** Added ``int`` type hint to ``DatabaseHandler::gc()`` and ``PostgreHandler::gc()`` to prevent SQL injection from non-integer ``$max_lifetime`` values. |
There was a problem hiding this comment.
Please add something like this under the Method Signature Changes section:
- **Session:** The ``$max_lifetime`` parameter of the following ``gc()`` methods
now has the native ``int`` type, matching ``SessionHandlerInterface``.
You can also list all affected handlers - this will match other entries.
paulbalandan
left a comment
There was a problem hiding this comment.
This is a refactor, and as mentioned should be targeting 4.8 branch instead.
| $this->assertSame(1, $handler->gc(3600)); | ||
| } | ||
|
|
||
| public function testGCRejectsNonInt(): void |
There was a problem hiding this comment.
This tests a PHP feature, not the method itself. Thus, the test is unnecessary.
Add int type hint to $max_lifetime in gc() for all 6 session handlers to match SessionHandlerInterface, preventing SQL injection from non-integer input. Includes changelog entry in v4.8.0.rst under Method Signature Changes.
2124cc3 to
e4bb6fd
Compare
Description
Added int type hint to $max_lifetime in all 6 session handlers' gc() to match SessionHandlerInterface. This prevents SQL injection from non-integer input by rejecting it with a
TypeError.Affected handlers: ArrayHandler, DatabaseHandler, FileHandler, MemcachedHandler, PostgreHandler, RedisHandler.
Checklist: