New. BotDetectorService. Frontend data log. - #864
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors bot-detector “frontend data log” collection/transfer into a consolidated browser state payload (apbct_browser_state) that is injected into frontend requests, then parsed server-side and exposed via apbct__bot_detector_get_fd_log(). It also updates AltSessions allow-listing and expands PHPUnit coverage for transport-specific state retrieval.
Changes:
- Replace periodic “save log to AltSessions” JS with an
ApbctBrowserStatecollector and request-injection (apbct_browser_state) across XHR/fetch/FormData/string bodies. - Update PHP to return
apbct_browser_state(with defaults/error handling) and add helpers to parse state from RequestParameters/POST. - Expand standalone tests to cover AltSessions/NoCookie/POST transports and default-filling behavior.
Reviewed changes
Copilot reviewed 15 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/StandaloneFunctions/TestBotDetectorLogGathering.php | Reworked tests to validate new browser-state transport behavior and defaults. |
| lib/Cleantalk/ApbctWP/Variables/AltSessions.php | Allow-list apbct_browser_state as JSON in AltSessions validation. |
| js/src/public-3-bot-detector-log.js | Introduces ApbctBrowserState + apbctGetBrowserStatePair() for request injection. |
| js/src/public-1-main.js | Injects apbct_browser_state into multiple request interception paths (XHR/fetch/etc.). |
| js/src/public-1-functions.js | Includes browser state into NoCookie hidden-field payload / storage data array. |
| js/prebuild/apbct-public-bundle.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_int-protection.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_int-protection_gathering.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_gathering.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_full-protection.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_full-protection_gathering.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_ext-protection.js | Prebuilt bundle updated with browser state injection logic. |
| js/prebuild/apbct-public-bundle_ext-protection_gathering.js | Prebuilt bundle updated with browser state injection logic. |
| js/apbct-public-bundle.min.js | Minified public bundle updated to match new browser state behavior. |
| inc/cleantalk-common.php | Server-side browser state parsing + updated fd-log return structure. |
| cleantalk.php | Changes bot detector wrapper script URL constant. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ontend_data-logger.ab
# Conflicts: # js/apbct-public-bundle.min.js # js/apbct-public-bundle_ext-protection.min.js # js/apbct-public-bundle_ext-protection_gathering.min.js # js/apbct-public-bundle_full-protection.min.js # js/apbct-public-bundle_full-protection_gathering.min.js # js/apbct-public-bundle_gathering.min.js # js/apbct-public-bundle_int-protection.min.js # js/apbct-public-bundle_int-protection_gathering.min.js
|
Please, fix autotests first. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #864 +/- ##
============================================
+ Coverage 30.00% 30.55% +0.54%
- Complexity 6539 6636 +97
============================================
Files 289 293 +4
Lines 26259 26490 +231
============================================
+ Hits 7880 8095 +215
- Misses 18379 18395 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
# Conflicts: # lib/Cleantalk/ApbctWP/Variables/AltSessions.php
There was a problem hiding this comment.
🟡 Changes recommended
There are verified functional issues in the new browser-state/log pipeline (PHP notice risk and inconsistent frontend log key/reader) plus several incorrect deprecation references that should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
lib/Cleantalk/ApbctWP/BotDetectorService.php:147
- Method name isNoScripFlow() appears to be a typo (missing 't') while the doc refers to a "NoScript" flow. Since this is a new API surface and is referenced from a deprecated wrapper, consider introducing the correctly spelled isNoScriptFlow() and keeping the old name as a deprecated alias for backward compatibility.
public static function isNoScripFlow()
{
if (apbct_is_plugin_active('oxygen/functions.php') && Get::getBool('ct_builder')) {
return true;
}
inc/cleantalk-common.php:1850
- The
@deprecatednote for apbct__is_bot_detector_enabled() references BotDetectorService::enabled(), but the actual method is BotDetectorService::isEnabled(). This mismatch can mislead users and static analysis tools.
/**
* Check if Bot-Detector is enabled/disabled
* @deprecated since 6.88, use BotDetectorService::enabled() instead
* @return bool
*/
- Files reviewed: 18/26 changed files
- Comments generated: 5
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed issues that can break data transport and frontend execution (AltSessions type mismatch for apbct_browser_state, uncaught JSON parsing in polling, potential ApbctBrowserState ReferenceError, and an apparently incorrect/dev bot-detector script URL source).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/26 changed files
- Comments generated: 4
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new BotDetectorService::getFrontendDataLog() can return non-JSON output on encoding failure and treats “collection disabled by constant” as an ERROR status, both of which are contract/behavior regressions that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lib/Cleantalk/ApbctWP/BotDetectorService.php:82
- The getPreparedExclusions() docblock says it returns '{}' when no valid exclusions exist, but json_encode([]) returns '[]' (and the accompanying test asserts '[]'). Please align the docblock with actual behavior and fix typos to avoid misleading IDE hints.
/**
* Prepare exclusions for skippping bot-detector event token field.
* @return string JSON encoded array of valid exclusions. If no valid exclusions, returns '{}'.
*/
- Files reviewed: 18/26 changed files
- Comments generated: 4
- Review effort level: Lite
https://app.doboard.com/1/task/51081