Fix: prevent fatal TypeError in EditFormRoute when post ID is non-numeric#8247
Open
thisismyurl wants to merge 5 commits into
Open
Fix: prevent fatal TypeError in EditFormRoute when post ID is non-numeric#8247thisismyurl wants to merge 5 commits into
thisismyurl wants to merge 5 commits into
Conversation
…fatal TypeError with non-numeric post IDs Fixes impress-org#8229 abs() on a non-numeric string (e.g. Contact Form 7's hex post IDs) throws a fatal TypeError on PHP 8.x. Add is_numeric() to the existing is_array() guard and cast to int before passing to abs() for explicit type safety.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds defensive input validation to the Form Builder’s edit route to prevent PHP 8 TypeErrors when $_GET['post'] is non-numeric (e.g., hex IDs), and introduces regression coverage + a changelog entry.
Changes:
- Guard
EditFormRouteagainst non-numeric$_GET['post']before callingabs(). - Add feature tests covering non-numeric and array
postquery values. - Add a changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| tests/Feature/FormBuilder/EditFormRouteTest.php | Adds regression tests to ensure EditFormRoute won’t TypeError on non-numeric/array post query values. |
| src/FormBuilder/Routes/EditFormRoute.php | Adds numeric guarding + casts to int before calling abs()/get_post(). |
| changelog/fix-edit-form-route-non-numeric-post-id.md | Documents the patch fix and the affected scenario (e.g., Contact Form 7 hex IDs). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class EditFormRoute | ||
| { | ||
| /** | ||
| * @since TBD Add is_numeric() guard to prevent fatal TypeError when post ID is non-numeric (e.g. Contact Form 7 hex IDs). |
Comment on lines
+27
to
+28
| if ( ! is_array($_GET['post']) && is_numeric($_GET['post'])) { | ||
| $post = get_post(abs((int) $_GET['post'])); |
| // WP sends an array of IDs so if that is the case here, we can skip this | ||
| if ( ! is_array($_GET['post'])) { | ||
| $post = get_post(abs($_GET['post'])); | ||
| if ( ! is_array($_GET['post']) && is_numeric($_GET['post'])) { |
Comment on lines
+31
to
+32
| $_GET['post'] = '5c80d03'; // Hexadecimal ID as used by Contact Form 7. | ||
| $_GET['action'] = 'edit'; |
| 'EditFormRoute threw a TypeError for non-numeric post ID: ' . $e->getMessage() | ||
| ); | ||
| } finally { | ||
| unset( $_GET['post'], $_GET['action'] ); |
| unset( $_GET['post'], $_GET['action'] ); | ||
| } | ||
|
|
||
| $this->assertTrue( true, 'No TypeError was thrown for non-numeric post ID.' ); |
| * | ||
| * @see https://github.com/impress-org/givewp/issues/8229 | ||
| * | ||
| * @since TBD |
| * of type int|float, string given" on PHP 8.x, crashing every admin page that | ||
| * CF7 or similar plugins opened with a hex post ID in the query string. | ||
| * | ||
| * @since TBD |
| * Confirm that the route also handles an array post ID gracefully (the bulk | ||
| * action case that the original is_array() guard was written for). | ||
| * | ||
| * @since TBD |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey team,
Fixes #8229.
The bug
EditFormRoute::__invoke()callsabs($_GET['post'])after only checking! is_array(). When another plugin (Contact Form 7 uses hexadecimal IDs like5c80d03) opens an admin edit page, theadmin_inithook fires globally, this route runs, and PHP 8.x throws:because
'5c80d03'is not numeric (is_numeric('5c80d03')→false) and PHP 8 no longer coerces non-numeric strings in arithmetic functions.The fix
Extend the existing guard with
&& is_numeric($_GET['post'])so the route body only runs when the value can safely be passed toabs(). Add an explicit(int)cast for type clarity:Tests
Added
tests/Feature/FormBuilder/EditFormRouteTest.phpwith two cases:testInvokeWithNonNumericPostIdDoesNotThrowTypeError— sets$_GET['post'] = '5c80d03', invokes the route, asserts noTypeErroris thrown. This is the delete-the-fix guard: removingis_numeric()causesabs('5c80d03')to throw on PHP 8.x, which the catch block catches and$this->fail()turns into a test failure.testInvokeWithArrayPostIdIsSkipped— confirms the existingis_array()guard still works.Changelog
Added
changelog/fix-edit-form-route-non-numeric-post-id.mdwithSignificance: patch,Type: fixed.(full disclosure: AI helped me identify the issue and verify my work)