Problem: "URL validation failed to due to the absence of the expected JSON-containing AMP_VALIDATION comment after the body"

Lofesa commented 4 years ago

When validate url this message show: "URL validation failed to due to the absence of the expected JSON-containing AMP_VALIDATION comment after the body."

Expected Behaviour

Show the errors of validation

WordPress version: 5.3
- Plugin version: 1.4.1
- Gutenberg plugin version (if applicable):
- AMP plugin template mode: transitional
- PHP version: 7.3
- OS: Centos 7
- Browser: [e.g. chrome, safari] Firefox
- Device: [e.g. iPhone6]

In the log files I have messages related to the use of is_amp_function related to a plugin, but when this plugin is disabled, the messages go away and the issue with validat url continue.

In the acces log file I get this: /wp-admin/edit.php?post_type=amp_validated_url&amp_validate_error=response_comment_absent&amp_urls_tested=0

I have tried adding ?amp&amp_validate (as stated in #1733) to the url, get the same mesage when a try to view the validation errors.

iconv is installed as estated in #3207

With all plugins disabled and a default theme, the issue is here. I have a list of plugins that when enabled the amp validator icon in admin side have issue, but cannot view cause this:

Autoptimize jQuery Updater Sassy Social Share Lazy Loader

But urls get amp validated in google and in amp validator.

Captura

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

westonruter commented 4 years ago

@Lofesa Thanks for the report. When you access a URL on the frontend and provide ?amp&amp_validate, it is expected that the page would end in this:

...
    </body><!--AMP_VALIDATION:{
    "results": [],
    "queried_object": {
        "id": 701,
        "type": "post"
    }
}
--></html>

Do you see something different?

If you look at your PHP error log, do you see anything?

Lofesa commented 4 years ago

Hi @westonruter No, none of these, not json snipet nor php error. When I disable these plugins (Autoptimize, jQuery Updater, Sassy Social Share, Lazy Loader) I got a green AMP in the admin bar, but when I try to validate, the message about no JSON sow.

How I can help to debug this? do you need access to the WP admin? In such case I need a IP cause the acces is IP white listed. I have enabled define('WP_DEBUG', true); and define('WP_DEBUG_LOG', true); but no log file is created, the error sow at screen. File permission in directory and owner are rigth. As far as the error exists when a default theme is set and no others plugins.... maybe a server missconfiguration? Tell me how I can help.

westonruter commented 4 years ago

How I can help to debug this? do you need access to the WP admin? In such case I need a IP cause the acces is IP white listed.

@Lofesa This may actually indicate the problem. What happens if you go to Site Health in the WordPress admin under the Tools menu? Are there any test failures for loopback requests? If so, you may need to make sure that you whitelist your own site's IP.

Lofesa commented 4 years ago

@westonruter The public IP and the loopback are whitelisted. In Site Health only have warnings about plugins and themes not used. No failures in loopback

`

wp-core

version: 5.3 site_language: es_ES user_language: es_ES timezone: Europe/Madrid permalink: /%postname%/ https_status: true user_registration: 0 default_comment_status: closed multisite: false user_count: 2 dotorg_communication: true

wp-paths-sizes

wordpress_path: /var/www/html/xxxxxxxx wordpress_size: 1,62 GB (1742360459 bytes) uploads_path: /var/www/html/xxxxxxx/wp-content/uploads uploads_size: 183,99 MB (192931172 bytes) themes_path: /var/www/html/xxxxxxx/wp-content/themes themes_size: 46,77 MB (49037910 bytes) plugins_path: /var/www/html/xxxxxxxx/wp-content/plugins plugins_size: 51,04 MB (53514926 bytes) database_size: 5,23 MB (5488640 bytes) total_size: 1,90 GB (2043333107 bytes)

wp-dropins (1)

object-cache.php: true

wp-active-theme

name: GeneratePress Child (generatepress_child) version: 0.1 author: Tom Usborne author_website: https://tomusborne.com parent_theme: GeneratePress (generatepress) theme_features: automatic-feed-links, post-thumbnails, post-formats, woocommerce, title-tag, html5, customize-selective-refresh-widgets, align-wide, responsive-embeds, custom-logo, menus, editor-style, amp, widgets theme_path: /var/www/html/xxxxxxxx/wp-content/themes/generatepress_child

wp-parent-theme

name: GeneratePress (generatepress) version: 2.4.1 author: Tom Usborne author_website: https://tomusborne.com theme_path: /var/www/html/xxxxxxxx/wp-content/themes/generatepress

wp-themes-inactive (3)

Bridge Child: version: 1.0.0, author: Qode Interactive Bridge: version: 18.0.9, author: Qode Interactive Twenty Nineteen: version: 1.4, author: el equipo de WordPress

wp-plugins-active (19)

AMP: version: 1.4.1, author: AMP Project Contributors AMP for GeneratePress: version: 0.1, author: Tom Usborne CAOS: version: 2.9.4, author: Daan van den Bergh Contact Form 7: version: 5.1.6, author: Takayuki Miyoshi Embed Any Document: version: 2.4.1, author: Awsm Innovations Enable Media Replace: version: 3.3.7, author: ShortPixel GP Premium: version: 1.9.1, author: Tom Usborne jQuery Updater: version: 3.4.1.2, author: Ramoonus Lazy Loader: version: 5.1.2, author: Florian Brinkmann, MarcDK Nginx Helper: version: 2.1.0, author: rtCamp Related Posts for WordPress: version: 2.0.3, author: Never5 Schema: version: 1.7.8, author: Hesham SEO Internal Links Revisited: version: 1.0, author: vietdex WPBakery Page Builder: version: 6.0.5, author: Michael M - WPBakery.com WP GDPR Cookie Notice: version: 1.0.0-beta.2, author: Felix Arntz WP Mail SMTP: version: 1.7.1, author: WPForms WP Redis: version: 0.7.1, author: Pantheon, Josh Koenig, Matthew Boynes, Daniel Bachhuber, Alley Interactive WP Twitter Feeds: version: 1.5, author: Team Startbit Yoast SEO: version: 12.6.2, author: Team Yoast

wp-plugins-inactive (6)

AMP-to-AMP Linking: version: 0.1.1, author: Weston Ruter, Google Autoptimize: version: 2.6.0-beta-4, author: Frank Goossens (futtta) Better Search Replace: version: 1.3.3, author: Delicious Brains Optimize Database after Deleting Revisions: version: 4.8.8, author: CAGE Web Design | Rolf van Gelder, Eindhoven, The Netherlands Redis Page Cache: author: (undefined), version: 0.8.3 Sassy Social Share: version: 3.3.6, author: Team Heateor

wp-media

image_editor: WP_Image_Editor_Imagick imagick_module_version: 1690 imagemagick_version: ImageMagick 6.9.10-77 Q16 x86_64 2019-12-02 https://imagemagick.org imagick_limits: imagick::RESOURCETYPE_AREA: 16 GB imagick::RESOURCETYPE_DISK: 9.2233720368548E+18 imagick::RESOURCETYPE_FILE: 768 imagick::RESOURCETYPE_MAP: 16 GB imagick::RESOURCETYPE_MEMORY: 8 GB imagick::RESOURCETYPE_THREAD: 4 gd_version: 2.2.5 ghostscript_version: 9.25

wp-server

server_architecture: Linux 5.4.2-1.el7.elrepo.x86_64 x86_64 httpd_software: nginx/1.17.6 php_version: 7.3.12 64bit php_sapi: fpm-fcgi max_input_variables: 1000 time_limit: 3200 memory_limit: 128M max_input_time: 120 upload_max_size: 100M php_post_max_size: 100M curl_version: 7.29.0 NSS/3.44 suhosin: false imagick_availability: true htaccess_extra_rules: true

wp-database

extension: mysqli server_version: 10.1.43-MariaDB client_version: mysqlnd 5.0.12-dev - 20150407 - $Id: 7cc7cc96e675f6d72e5cf0f267f48e167c2abb23 $

wp-constants

WP_HOME: https://xxxxxxxx WP_SITEURL: https://xxxxxx WP_CONTENT_DIR: /var/www/html/xxxxxx/wp-content WP_PLUGIN_DIR: /var/www/html/xxxxxxx/wp-content/plugins WP_MAX_MEMORY_LIMIT: 128M WP_DEBUG: false WP_DEBUG_DISPLAY: true WP_DEBUG_LOG: false SCRIPT_DEBUG: false WP_CACHE: true CONCATENATE_SCRIPTS: undefined COMPRESS_SCRIPTS: undefined COMPRESS_CSS: undefined WP_LOCAL_DEV: undefined DB_CHARSET: utf8mb4 DB_COLLATE: utf8mb4_unicode_ci

wp-filesystem

wordpress: writable wp-content: writable uploads: writable plugins: writable themes: writable

Captura

`

westonruter commented 4 years ago

It will be difficult for me to give you an IP to whitelist as I am traveling.

I'm confused as to why the JSON comment is not appearing at the end of the body. For example, on my own site when I am logged-in, I can go to https://weston.ruter.net/?amp_validate and I can see after the </body> the aforementioned HTML comment like . (If you are not authenticated, then this comment will not appear.)

Are the pages being generated valid AMP? In other words, if you copy the HTML response and paste it into https://validator.amp.dev/ does it validate? (You can ignore any validation error about dev mode, as this appears when the admin bar is showing.) If so, then this indicates the AMP post-processing is working properly.

Are other HTML comments being removed from responses?

Lofesa commented 4 years ago

Hi @westonruter Thank you for give me the clue to solve the issue. ".... Are other HTML comments being removed from responses?" I don´t realize thi´s an html comment.... I´m thinking is a json script. Yes HTML comments are removed by the PageSpeed Module, I add &PageSpeed=off to the url and the comment is here:

<!--AMP_VALIDATION:{
    "results": [],
    "queried_object": {
        "id": 12137,
        "type": "post"
    }
}
-->

westonruter commented 4 years ago

OK! That is excellent news. So the need here is to prevent the comments from getting removed during validation requests. Is there a way you can prevent comments from being removed when an amp_validate query param is present rather than PageSpeed=off?

In the meantime, you can also try patching the plugin like so:

--- a/includes/validation/class-amp-validation-manager.php
+++ b/includes/validation/class-amp-validation-manager.php
@@ -1820,6 +1820,7 @@ class AMP_Validation_Manager {
        $added_query_vars = [
            self::VALIDATE_QUERY_VAR   => self::get_amp_validate_nonce(),
            self::CACHE_BUST_QUERY_VAR => wp_rand(),
+           'PageSpeed'                => 'off',
        ];
        $validation_url   = add_query_arg( $added_query_vars, $url );

But we'd want to find a permanent solution for compatibility with PageSpeed.

Lofesa commented 4 years ago

Hi I have tried to disable PageSpeed in the /wp-admin/ location (I use nginx) but no chance. I will try with the parameters, disabling pagespeed when contain amp_validate. I´m thinking.... AMP plugin works (when in native or transitional mode) catching the output of the "normal" page and the converting this in an amp page, rigth? A possible solution maybe cheking the headers. PageSpeed module set a x-page-speed (in nginx) or x-modpage-speed (in apache) and if where present add the PageSpeed=off

westonruter commented 4 years ago

I will try with the parameters, disabling pagespeed when contain amp_validate.

We may want to make the URL filterable so that you can add those query params via a plugin.

I have tried to disable PageSpeed in the /wp-admin/ location (I use nginx) but no chance.

This wouldn't help because the the ?amp_validate requests are being made on the frontend. Yes, you initiate the request to validate from /wp-admin/ but this request results in WordPress doing a loopback request on the frontend with ?amp_validate in the URL.

I´m thinking.... AMP plugin works (when in native or transitional mode) catching the output of the "normal" page and the converting this in an amp page, rigth?

That's right. The AMP plugin uses output buffering to capture the output of a template and then it makes the necessary conversions for AMP while also removing anything that is not valid AMP. When you make the request with ?amp_validate then it does additional work to identify the themes and plugins that were responsible for the invalid markup.

A possible solution maybe cheking the headers. PageSpeed module set a x-page-speed (in nginx) or x-modpage-speed (in apache) and if where present add the PageSpeed=off

The X-Page-Speed and X-Mod-Pagespeed headers are sent in the response, correct? We wouldn't want to conditionally add PageSpeed=off to the URL if those response headers are present, because then it would mean we'd have to make two requests. First without ?PageSpeed=off and then again with ?PageSpeed=off when we determine that PageSpeed is running.

As an alternative to all of this, it would probably be more robust if we consider not sending the validation results back in JSON data contained in an HTML comment. The results could instead have the results written directly into the DB when making the request, as opposed to sending the results back in the response. That would help the situation here, but it could also introduce other problems.

Maybe instead of putting the data in an HTML comment we should rather put it inside of a <script type="application/json"> tag. That would probably make it less prone to be corrupted.

Lofesa commented 4 years ago

Hi I have tried to add PageSpeed=off to the parameters via an nginx if: if ( $args ~* (.*amp_validate.*) { set $args $args&PageSpeed=off; } but no luck, don´t work. I can explore the option to disable remove_comments filter in PageSpeed module code when parameters in url contains amp_validate, but....

Maybe instead of putting the data in an HTML comment we should rather put it inside of a <script type="application/json"> tag. That would probably make it less prone to be corrupted.

this option sound better, cause not only PageSpeed can remove html comments, most of minifiers plugins can do and I think the AMP plugin can´t test the presence of all these plugins or software that delete the html comments and disable it for the request.

westonruter commented 4 years ago

@Lofesa I believe I have an even better solution in #3898.

Please test: https://github.com/ampproject/amp-wp/pull/3898#issuecomment-562873898

Lofesa commented 4 years ago

@westonruter Tested the v1.5.0-alpha-20191207T181223Z-c3e9208b in one site. It works. If I found some issue I will report here.

ampproject / amp-wp