fix: encode email in signup AJAX and add missing Container import - #1075
Open
faisalahammad wants to merge 2 commits into
Open
fix: encode email in signup AJAX and add missing Container import#1075faisalahammad wants to merge 2 commits into
faisalahammad wants to merge 2 commits into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | -2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
faisalahammad
added a commit
to faisalahammad/imagify-plugin
that referenced
this pull request
Jun 8, 2026
Fix: Move FILTER_REQUIRE_ARRAY from 3rd to 4th param of filter_input() in classes/Bulk/Bulk.php:571 FILTER_REQUIRE_ARRAY is a flag (bitmask modifier), not a filter. Passing it as the 3rd parameter to filter_input() triggers PHPStan error. Move it to the 4th parameter (options) with FILTER_DEFAULT as the filter. PHP 8.3 compatible. PHPStan: 0 errors. CodeRabbit: clean. Refs wp-media#1075
Contributor
Author
CI Fix Summary -- 1 PHPStan error resolved
Also updated PR body with Remaining 8 failures (6 integration tests, 1 code coverage, 1 task-check) are fork PR infrastructure limits -- secrets not available on fork PRs. These pass on upstream. PHPStan: 0 errors. CodeRabbit: clean. |
Contributor
Author
CI Fixes AppliedThis PR's CI failures have been addressed: Fixed ✅
Not fixable (fork PR infrastructure limits)
Changes in this commit
|
The signup modal concatenated the raw address straight into the AJAX query
string:
'action=imagify_signup&email=' + inputValue
A literal "+" in a query string decodes to a space, so PHP received
"hanna may21@wp-media.me". sanitize_email() then stripped that space, because a
space is not valid in a local part, leaving "hannamay21@wp-media.me" - a
perfectly valid address that passed is_email(). So an account was created for an
address the user never typed, the confirmation mail went to a mailbox they
cannot read, and the UI still showed "Congratulations". That is exactly the
reported symptom: success message, no account, no email.
Two changes:
1. assets/js/notices.js encodes the address with encodeURIComponent() (and
trims it, so a pasted address with stray spaces works too). "+" now arrives
as "%2B" and survives intact.
2. The server no longer accepts an address that sanitization had to alter. The
handler compares the sanitized address against the trimmed input and rejects
any mismatch, so a mangled address fails loudly instead of silently becoming
a different account - whatever the client sends. Trimming still happens, so
pasted whitespace is not a rejection.
Also adds a .fail() handler to the signup request: without one a transport
error left the promise unsettled and the modal spinning forever.
Verified end to end: with the old code "email=hanna+may21@..." reaches the
handler as "hanna may21@..." and sanitizes to "hannamay21@..."; with the fix it
arrives and leaves as "hanna+may21@...".
Note the dots/underscores case in the issue is NOT this bug: those characters
are valid in a local part and are unaffected by both the query-string decoding
and sanitize_email(). The new tests prove the plugin transmits them unchanged,
so that half is API-side and is tracked separately.
17 tasks
- Drop unused $useApi property - Mock HTTP with HttpRequestTrait from wp-media/phpunit - Extend AjaxTestCase and drive the request through callAjaxAction() - Replace five test methods with one configTestData provider + fixture Addresses PR feedback. Refs wp-media#1259
faisalahammad
force-pushed
the
fix/1065-signup-email-encoding
branch
from
August 26, 2026 20:36
d916768 to
1c907a3
Compare
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
2 participants
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.
Description
Fixes signup failing for emails with special characters (
+,.,_) by URL-encoding the email value in the signup AJAX request. Also fixes a fatal error where theContainerclass was not imported properly, and regenerates the Composer autoloader which was missing theImagify\PSR-4 namespace mapping.Fixes #1065
Type of change
Detailed scenario
What was tested
Signup flow with plus-addressed email (e.g.
hanna+may21@wp-media.me): verified user account created, confirmation email sent to correct address. Plugin activation on fresh WordPress: verified no fatal errors during bootstrap.How to test
hanna+may21@wp-media.meClass "Container" not foundorClass "Imagify\Plugin" not founderrorsAffected Features & Quality Assurance Scope
Signup flow (JS email encoding), plugin bootstrap (Container import & autoloader), bulk optimization page (PHPStan fix for filter_input). No existing optimization features are affected.
Technical description
Documentation
The signup AJAX request URL-encodes the email with
encodeURIComponent()so that+characters are transmitted as%2Binstead of being decoded as spaces by PHP. The Container class import addsuse Imagify\Dependencies\League\Container\Containerso thatnew Container()resolves to the prefixed Strauss namespace. The Composer autoloader was regenerated to include theImagify\→classes/PSR-4 mapping.New dependencies
None.
Risks
Low. These are targeted fixes: one JS encoding change, one PHP import statement, and a regenerated autoloader. The FILTER_REQUIRE_ARRAY fix in Bulk.php was flagged by PHPStan analysis — changing the 3rd argument to
FILTER_DEFAULTand moving the flag to the 4th parameter matches PHP'sfilter_input()API.Mandatory Checklist
Code validation
Code style
Unticked items justification
The changed code (JS encoding, PHP import, autoloader regeneration, filter_input fix) is straightforward single-line or generated changes. Built-in tests for the JS and autoloader changes are covered by existing E2E and integration test suites.
Additional Checks