Repository navigation
WordPress 7.1 compatibility and PHP SDK 1.0.10 - #23
Merged
Merged
Conversation
Verified against WordPress 7.1 on PHP 8.2 and 8.4, with the index.php override both enabled and disabled, plus WP-CLI and wp-cron. - Guard $_SERVER['SERVER_ADDR'], which is not populated by every SAPI. This warned on every front-end request; with the index override active it printed before WordPress loaded, putting output ahead of the redirect headers the waiting room depends on. - Guard a missing public key when building the diagnostics header. - Declare Requires at least and Requires PHP. The 7.4 floor matches what WordPress 7.1 itself requires, and SDK 1.0.10 needs 7.1 as a minimum. - Update the SDK to 1.0.10, clearing the PHP 8.4 implicit-nullable deprecation and the unguarded $_SERVER reads behind it. - Bump to 1.1.15; 1.1.14 is already published on wordpress.org. - Backfill the 1.1.14 changelog entry, which until now existed only in the published SVN tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
composer.json still declares a PHP version floor (>=5.3) that conflicts with the plugin’s stated PHP 7.4 requirement and the SDK’s PHP >=7.1 requirement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the CrowdHandler WordPress plugin for WordPress 7.1 / PHP 8.2+ compatibility, removes PHP warnings that could break redirect/header behavior, and upgrades the bundled CrowdHandler PHP SDK.
Changes:
- Added defensive guards around
$_SERVER['SERVER_ADDR']and missing diagnostics public key to prevent PHP warnings. - Declared WordPress/PHP minimum requirements and bumped plugin version to 1.1.15 (including readme/changelog updates).
- Updated
crowdhandler/sdkdependency to 1.0.10 (and refreshed the lockfile).
File summaries
| File | Description |
|---|---|
| README.txt | Declares WP/PHP requirements, bumps stable tag, and backfills changelog entries. |
| includes/class-crowdhandler-gatekeeper.php | Avoids warnings when SERVER_ADDR is absent by reading it defensively. |
| includes/class-crowdhandler-diagnostics.php | Avoids warnings when the public key is missing and adjusts diagnostics header format/time format. |
| crowdhandler.php | Bumps plugin version and adds “Requires at least” / “Requires PHP” headers. |
| composer.lock | Locks SDK to 1.0.10 and updates dev dependency metadata. |
| composer.json | Updates SDK constraint to ^1.0.10. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
composer.json still declared php >=5.3 while the plugin header and readme declare Requires PHP 7.4 and the locked SDK 1.0.10 needs >=7.1. Composer could therefore resolve and install the plugin on runtimes it does not support, and the declared requirements disagreed with one another. Aligns composer.json with the plugin header and syncs the lock's platform block to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The diagnostics header construction still allows response header injection via unsanitized stored option data and should be hardened before release.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
includes/class-crowdhandler-diagnostics.php:31
$publicKeyis injected into an HTTP response header (x-crowdhandler-info) without stripping CR/LF. Because this value comes from a stored option (no sanitize callback is registered forcrowdhandler_settings), a maliciously crafted key containing newlines could break the header boundary (header injection). Also,$formatedTimeis misspelled, which makes the header-building code harder to read/maintain.
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
crowdhandler_settings was registered without a sanitize_callback, so the
stored public key was whatever was posted. That value is interpolated into
the x-crowdhandler-info response header.
PHP refuses to send any header containing CR/LF, so this was not an
injection route: the forged header never materialised. What did happen is
that the whole x-crowdhandler-info header was silently dropped and a PHP
warning emitted -- and with the index.php override active that warning
prints before WordPress loads, ahead of the redirect headers.
Strips CR/LF where the header is built, and registers a sanitize_callback
so the value is cleaned on the way in as well. Checkbox fields keep their
existing shape ('on' or absent), which isEnabled() and the index override
both depend on.
Also fixes the $formatedTime misspelling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
register_setting() should explicitly declare the crowdhandler_settings option type as array to match its actual stored/consumed shape and avoid inconsistent settings metadata behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, consistent with the PR description, and address concrete warning/compatibility issues without introducing risky behavioral changes.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
luke-owen-crowdhandler
approved these changes
Sep 9, 2026
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.
Verified against WordPress 7.1 on PHP 8.2 and 8.4, with the index.php override both enabled and disabled, plus WP-CLI and wp-cron.