Amp-wp: Output buffering during validation requests can suppress certain validation errors from appearing

[jdelia]

WordPress 5.2.3
AMP Plugin 1.3
Antispam Bee plugin 2.9.1

When the antispam plugin is active, some posts show a validation error in the admin bar. When you click on the admin bar re-validate link, nothing shows on the results page. Checking the page in the AMP Validator - shows it as PASSED.

Deactivate the Antispam plugin, there are no issues in the admin bar - green checkbox.

Video capture of the issue: https://cl.ly/57d62234a65b

Video capture disable plugin and check page: https://cl.ly/c119185fe9e8

[swissspidy]

Thanks for your report!

I can verify that there is something odd going on. Not only do I see the same issue as you when in Standard mode, it also breaks Transitional mode as visiting example.com/example-page/?amp just redirects to example.com/example-page/. It works when Antispam Bee is active.

Years ago I used to work on Antispam Bee itself, so I know that it does some special things behind the scenes to detect bots. This is likely causing issues here as well.

Not sure what's the best way to test it though and what we could do on our side.

[jdelia]

Thanks, Pascal. My only issue on the AMP end is that the admin bar indicates an issue, but when you try to re-validate it, it appears to be fine. When you revisit the page, the same issue in the admin bar again. For most users, that might be very confusing as to what is actually happening.

I'm happy to find an alternative plugin - but I wanted to share this in case others have a similar issue with the plugin and get stuck in a loop like I did. LOL.

[westonruter]

@jdelia I'm able to reproduce this issue. I was able to get access to the validation error via modifying the plugin like this to log out the error when accessing the site on the frontend:

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
index 50983383..6d0ed331 100644
--- a/includes/class-amp-theme-support.php
+++ b/includes/class-amp-theme-support.php
@@ -2219,6 +2219,8 @@ class AMP_Theme_Support {
            $validation_results[] = $validation_result;
        }

+       error_log( json_encode( $validation_results, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
+
        $dom_serialize_start = microtime( true );

        // Gather all component scripts that are used in the document and then render any not already printed.

The validation error being logged is:

    {
        "error": {
            "node_name": "script",
            "parent_name": "p",
            "code": "invalid_element",
            "type": "js_error",
            "node_attributes": {
                "type": "text/javascript"
            },
            "text": "document.getElementById(\"comment\").setAttribute( \"id\", \"a3bd3a9c66770e69c8f3a57238288bed\" );document.getElementById(\"c08a1a06c7\").setAttribute( \"id\", \"comment\" );"
        },
        "sanitized": true
    }

The problem is that in \Antispam_Bee::prepare_comment_field() it is doing:

ob_start(
    array(
        'Antispam_Bee',
        'replace_comment_field',
    )
);

In a frontend request, the \Antispam_Bee::replace_comment_field() output buffer callback is being called with the output buffer for the whole page. However, in an AMP validation request (when you click “Re-validate”), additional output buffers are being added which capture the output when actions are being invoked. I believe this is the reason for the discrepancy.

This problem with output buffering should be eliminated once we move the source-attribution logic to use Origination, since it doesn't start output buffering for each invocation.

In the meantime, I suggest using Akismet or some other plugin.

[jdelia]

Thanks, Weston. I'll swap out the plugin for now.

[swissspidy]

Since Antispam Bee is quite popular, I wonder how their current implementation could be made AMP-compatible.

[westonruter]

Akismet was made AMP-compatibile by just short-circuiting the JS code from being added in AMP responses: https://plugins.trac.wordpress.org/changeset/2086419/akismet

That could be done here as well. Or amp-script could be explored as an alternative, but I think it's a bit early for it.

[westonruter]

I've raised this on the Antispam Bee plugin: https://github.com/pluginkollektiv/antispam-bee/issues/283#issuecomment-538030250

[amedina]

Closing this as it has been reported to the 3P plugin: pluginkollektiv/antispam-bee#283 (comment)