Fix. ContactEncoder. Unfreeze composer version. - #881
Merged
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (27.14%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #881 +/- ##
=========================================
Coverage 30.00% 30.00%
Complexity 6539 6539
=========================================
Files 289 289
Lines 26259 26259
=========================================
Hits 7880 7880
Misses 18379 18379 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed runtime/logic issues in ContactsEncoder around Antispam namespace usage and match-position handling that can lead to fatal errors or incorrect content removal.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the bundled Contact(s) Encoder library and its documentation, adjusts Composer metadata/dependencies, and standardizes trimming behavior across multiple components in the plugin.
Changes:
- Updated the ContactsEncoder common implementation (now non-abstract) and refactored encoding logic/helpers (including option-tag handling and aria-label handling adjustments).
- Expanded/rewrote Contacts Encoder README and added a bundled GPLv3 LICENSE file plus richer package metadata in the library’s composer.json.
- Replaced many
trim($x)calls with explicit charlists and updated the rootcomposer.jsonto unfreezecleantalk/contacts-encoder(dev-master) and addreplaceentries.
File summaries
| File | Description |
|---|---|
| lib/Cleantalk/Common/TextPlate.php | Uses explicit trim charlist in text template rendering. |
| lib/Cleantalk/Common/Helper.php | Uses explicit trim charlist in IP parsing/CSV helpers. |
| lib/Cleantalk/Common/ContactsEncoder/README.MD | Major documentation expansion (setup, decode endpoint, troubleshooting). |
| lib/Cleantalk/Common/ContactsEncoder/LICENSE | Adds bundled GPLv3 license text. |
| lib/Cleantalk/Common/ContactsEncoder/Helper/ContactsEncoderHelper.php | Updates exclusion rules and refactors helper APIs (mailto/option detection). |
| lib/Cleantalk/Common/ContactsEncoder/ContactsEncoder.php | Refactors encoding/decoding flow; adds bot-check logic and aria-label handling changes. |
| lib/Cleantalk/Common/ContactsEncoder/composer.json | Adds metadata + requires cleantalk/antispam and updates allow-plugins. |
| lib/Cleantalk/ApbctWP/UpdatePlugin/DbColumnCreator.php | Uses explicit trim charlist in schema parsing. |
| lib/Cleantalk/ApbctWP/ServerRequirementsChecker/ServerRequirementsChecker.php | Uses explicit trim charlist for memory-limit normalization. |
| lib/Cleantalk/ApbctWP/RemoteCalls.php | Uses explicit trim charlist for incoming API key. |
| lib/Cleantalk/ApbctWP/GetFieldsAny.php | Uses explicit trim charlist during field extraction/normalization. |
| lib/Cleantalk/ApbctWP/Firewall/SFW.php | Uses explicit trim charlist for log IP normalization. |
| lib/Cleantalk/ApbctWP/FindSpam/UsersChecker.php | Uses explicit trim charlist for user email normalization. |
| lib/Cleantalk/ApbctWP/FindSpam/CommentsChecker.php | Uses explicit trim charlist for comment IP/email normalization. |
| lib/Cleantalk/ApbctWP/Cron.php | Uses explicit trim charlist before unserialize. |
| lib/Cleantalk/ApbctWP/ContactsEncoder/Shortcodes/ExcludedEncodeContentSC.php | Uses explicit trim charlist when normalizing protected content. |
| lib/Cleantalk/ApbctWP/ContactsEncoder/Integrations/CEIntegrationGridBuilder.php | Uses explicit trim charlist for CSS chunk processing. |
| lib/Cleantalk/ApbctWP/ContactsEncoder/ContactsEncoder.php | Removes duplicated decoded-contacts storage (now in base) and adjusts trim usage. |
| lib/Cleantalk/Antispam/IntegrationsByClass/WPSearchForm.php | Uses explicit trim charlist for form method parsing. |
| lib/Cleantalk/Antispam/IntegrationsByClass/WPForms.php | Uses explicit trim charlist for field label normalization. |
| lib/Cleantalk/Antispam/Integrations/WordpressFileUpload.php | Uses explicit trim charlist for userdata parsing. |
| lib/Cleantalk/Antispam/Integrations/Forminator.php | Uses explicit trim charlist for email candidate normalization. |
| lib/Cleantalk/Antispam/Integrations/ElfsightForm.php | Uses explicit trim charlist for field name/value normalization. |
| lib/Cleantalk/Antispam/Integrations/ElementorPro.php | Uses explicit trim charlist for nickname building. |
| lib/Cleantalk/Antispam/Integrations/CleantalkWpDieOnComment.php | Uses explicit trim charlist for comment author fields. |
| lib/Cleantalk/Antispam/Integrations/ClassifiedListingRegister.php | Uses explicit trim charlist for username normalization. |
| lib/Cleantalk/Antispam/Integrations/BookingCalendar.php | Uses explicit trim charlist for extracted field values. |
| lib/Cleantalk/Antispam/Integrations/AmemberRegister.php | Uses explicit trim charlist for registration fields. |
| inc/cleantalk-wpcli.php | Uses explicit trim charlist when installing API key via WP-CLI. |
| inc/cleantalk-settings.php | Uses explicit trim charlist in settings validation/sanitization. |
| inc/cleantalk-public.php | Uses explicit trim charlist for JS check value. |
| inc/cleantalk-public-integrations.php | Uses explicit trim charlist for WooCommerce wishlist nickname. |
| inc/cleantalk-common.php | Uses explicit trim charlist for pre-post email handling and nickname building. |
| composer.json | Unfreezes contacts-encoder to dev-master and adds replace mappings. |
| cleantalk.php | Uses explicit trim charlist for inserted auth key. |
Review details
Suppressed comments (1)
lib/Cleantalk/Common/ContactsEncoder/Helper/ContactsEncoderHelper.php:99
isInsideOptionTag()also relies onstrpos($content, $email)(first occurrence). With repeated emails/phones in the same HTML snippet, this can misclassify later matches (outside<option>) as being inside<option>and skip encoding incorrectly. Accepting a known match offset avoids this class of false positives.
public function isInsideOptionTag($email, $content)
{
$pos = strpos($content, $email);
if ($pos === false) {
return false;
- Files reviewed: 35/36 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The common ContactsEncoder now imports non-existent Cleantalk\Common\Antispam\* classes in this repo (runtime fatal in checkRequest()), and the root Composer dependency is pinned to a non-reproducible moving dev-master branch.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
lib/Cleantalk/Common/ContactsEncoder/Helper/ContactsEncoderHelper.php:104
isInsideOptionTag()searches for the last<optionusingstrrpos(), which is case-sensitive. HTML tag names are case-insensitive, so<OPTION>(or mixed case) won't be detected and emails inside option text may be encoded unexpectedly.
This issue also appears on line 129 of the same file.
lib/Cleantalk/Common/ContactsEncoder/ContactsEncoder.php:6
ContactsEncoderimportsCleantalk\Common\Antispam\Cleantalk/CleantalkRequest, but this repository only defines these classes underCleantalk\Antispam\...(e.g.lib/Cleantalk/Antispam/Cleantalk.php). As-is,checkRequest()will fatal with "Class not found" when called.
use Cleantalk\Common\Antispam\Cleantalk;
use Cleantalk\Common\Antispam\CleantalkRequest;
lib/Cleantalk/Common/ContactsEncoder/Helper/ContactsEncoderHelper.php:134
isInsideScriptTag()is intended to be HTML-aware, but it searches for<script/</script>with case-sensitive functions (strrpos()/strpos()). Since HTML tag names are case-insensitive, contacts inside<SCRIPT>...</SCRIPT>may be encoded unexpectedly.
public function isInsideScriptTag($email, $content, $position = null)
{
$pos = $this->resolveMatchPosition($email, $content, $position);
if ($pos === false) {
return false;
}
composer.json:22
- Depending on
cleantalk/contacts-encoderasdev-mastermakes installs non-reproducible and can break CI/builds when upstream changes. If the goal is to "unfreeze" the version, consider pinning to a tagged release (or a specific commit hash) instead of a moving branch.
"cleantalk/apbct-installer": "*",
"cleantalk/contacts-encoder": "dev-master",
"cleantalk/rate-limiter": "*"
- Files reviewed: 35/36 changed files
- Comments generated: 0 new
- Review effort level: Lite
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.
task https://app.doboard.com/1/task/54895