search

google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 292 forks source link

Error establishing database connection message rendering with "<h1>" tag #4169

Closed oscarssanchez closed 2 years ago

oscarssanchez commented 3 years ago

Bug Description

It looks like the description https://github.com/google/site-kit-wp/blob/01e077bfd5a56039942cc010408e257cd006bbc0/assets/js/components/legacy-notifications/cta.js#L51 string comes with an <h1> tag. This is probably taken as a literal string, but even if it wasn't It probably does not make sense to have an <h1> inside a <p>tag

Steps to reproduce

  1. Go to SiteKit dashboard
  2. Trigger a database connection error (I triggered mine by having a slow connection)
  3. See error

Screenshots

sitekit

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

Acceptance criteria

Implementation Brief

In assets/js/components/ReportError.js:

Test Coverage

QA Brief

Changelog entry

jamesozzie commented 3 years ago

@oscarssanchez Thanks for creating this GitHub issue. Before escalating this for the team to review let's check your site setup. From the codebase checks I've been performing I don't see any errors or descriptions output in H1 tags. Would it be possible for you to open a WordPress support topic, where initial user reported issues are investigated? Within the support topic it would be great if you could state whether you're able to reproduce this consistently. If so we may ask you to perform a check using the Health Check & Troubleshooting plugin, to ensure there are no third party plugins or hosting related features impacting this description being output.

I'll also keep this GitHub issue open while we can perform additional checks and discuss this with the team.

jamesozzie commented 3 years ago

I was able to recreate this on a test site today. The error appears in my case in the below scenarios:

None of the above occur consistently and in my case I tested from a hosting platform whereby a user did report the same previously. Since opening this support topic I no longer encounter the same on this site - which uses a core theme and additional plugins other than SK (SH info below)

image

Console Errors:

JQMIGRATE: Migrate is installed, version 3.3.2
/wp-json/google-site-kit/v1/modules/analytics/data/report?metrics%5B0%5D%5Bexpression%5D=ga%3Ausers&metrics%5B0%5D%5Balias%5D=Total%20Users&startDate=2021-09-02&endDate=2021-09-29&url=&compareStartDate=2021-08-05&compareEndDate=2021-09-01&_locale=user:1 Failed to load resource: the server responded with a status of 500 ()
/wp-json/google-site-kit/v1/modules/analytics/data/report?metrics%5B0%5D%5Bexpression%5D=ga%3Apageviews&metrics%5B0%5D%5Balias%5D=Pageviews&dimensions%5B0%5D%5Bname%5D=ga%3ApagePath&startDate=2021-09-02&endDate=2021-09-29&orderby%5B0%5D%5BfieldName%5D=ga%3Apageviews&orderby%5B0%5D%5BsortOrder%5D=DESCENDING&limit=10&_locale=user:1 Failed to load resource: the server responded with a status of 500 ()
/wp-json/google-site-kit/v1/modules/pagespeed-insights/data/pagespeed?strategy=mobile&url=https%3A%2F%2Fmytakes.net%2F&_locale=user:1 Failed to load resource: the server responded with a status of 500 ()
/wp-json/google-site-kit/v1/modules/pagespeed-insights/data/pagespeed?strategy=desktop&url=https%3A%2F%2Fmytakes.net%2F&_locale=user:1 Failed to load resource: the server responded with a status of 500 ()
2googlesitekit-api.8da02732db5b91a5377c.js:1 Google Site Kit API Error method:GET datapoint:report type:modules identifier:analytics error:"<h1>Error establishing a database connection</h1>"
(anonymous) @ googlesitekit-api.8da02732db5b91a5377c.js:1
2googlesitekit-api.8da02732db5b91a5377c.js:1 Google Site Kit API Error method:GET datapoint:pagespeed type:modules identifier:pagespeed-insights error:"<h1>Error establishing a database connection</h1>"
(anonymous) @ googlesitekit-api.8da02732db5b91a5377c.js:1
SH information ` ### wp-core ### version: 5.8.1 site_language: en_US user_language: en_US timezone: +00:00 permalink: /%year%/%monthnum%/%day%/%postname%/ https_status: true multisite: false user_registration: 0 blog_public: 1 default_comment_status: open environment_type: production user_count: 1 dotorg_communication: true ### wp-paths-sizes ### wordpress_path: /home/sakione/public_html wordpress_size: 43.72 MB (45844906 bytes) uploads_path: /home/sakione/public_html/wp-content/uploads uploads_size: 0.00 B (0 bytes) themes_path: /home/sakione/public_html/wp-content/themes themes_size: 6.47 MB (6780262 bytes) plugins_path: /home/sakione/public_html/wp-content/plugins plugins_size: 8.74 MB (9166985 bytes) database_size: 906.97 KB (928733 bytes) total_size: 59.82 MB (62720886 bytes) ### wp-active-theme ### name: Twenty Twenty-One (twentytwentyone) version: 1.4 author: the WordPress team author_website: https://wordpress.org/ parent_theme: none theme_features: core-block-patterns, widgets-block-editor, automatic-feed-links, title-tag, post-formats, post-thumbnails, menus, html5, custom-logo, customize-selective-refresh-widgets, wp-block-styles, align-wide, editor-styles, editor-style, editor-font-sizes, custom-background, editor-color-palette, editor-gradient-presets, responsive-embeds, custom-line-height, experimental-link-color, custom-spacing, custom-units, widgets theme_path: /home/sakione/public_html/wp-content/themes/twentytwentyone auto_update: Disabled ### wp-themes-inactive (2) ### Twenty Nineteen: version: 2.1, author: the WordPress team, Auto-updates disabled Twenty Twenty: version: 1.8, author: the WordPress team, Auto-updates disabled ### wp-plugins-active (1) ### Site Kit by Google: version: 1.42.0, author: Google, Auto-updates disabled ### wp-plugins-inactive (2) ### Akismet Anti-Spam: version: 4.1.12, author: Automattic, Auto-updates disabled Hello Dolly: version: 1.7.2, author: Matt Mullenweg, Auto-updates disabled ### wp-media ### image_editor: WP_Image_Editor_Imagick imagick_module_version: 1802 imagemagick_version: ImageMagick 7.0.10-10 Q16 x86_64 2020-07-09 https://imagemagick.org imagick_version: 3.4.4 file_uploads: File uploads is turned off post_max_size: 512M upload_max_filesize: 512M max_effective_size: 512 MB max_file_uploads: 20 imagick_limits: imagick::RESOURCETYPE_AREA: 31 GB imagick::RESOURCETYPE_DISK: 9.2233720368548E+18 imagick::RESOURCETYPE_FILE: 12288 imagick::RESOURCETYPE_MAP: 31 GB imagick::RESOURCETYPE_MEMORY: 16 GB imagick::RESOURCETYPE_THREAD: 1 imagemagick_file_formats: 3FR, 3G2, 3GP, AAI, AI, ART, ARW, AVI, AVS, BGR, BGRA, BGRO, BIE, BMP, BMP2, BMP3, BRF, CAL, CALS, CANVAS, CAPTION, CIN, CIP, CLIP, CMYK, CMYKA, CR2, CR3, CRW, CUBE, CUR, CUT, DATA, DCM, DCR, DCRAW, DCX, DDS, DFONT, DNG, DOT, DPX, DXT1, DXT5, EPDF, EPI, EPS, EPS2, EPS3, EPSF, EPSI, EPT, EPT2, EPT3, ERF, EXR, FAX, FILE, FITS, FLV, FRACTAL, FTP, FTS, G3, G4, GIF, GIF87, GRADIENT, GRAY, GRAYA, GROUP4, GV, HALD, HDR, HISTOGRAM, HRZ, HTM, HTML, HTTP, HTTPS, ICB, ICO, ICON, IIQ, INFO, INLINE, IPL, ISOBRL, ISOBRL6, J2C, J2K, JBG, JBIG, JNG, JNX, JP2, JPC, JPE, JPEG, JPG, JPM, JPS, JPT, JSON, K25, KDC, LABEL, M2V, M4V, MAC, MAP, MASK, MAT, MATTE, MEF, MIFF, MKV, MNG, MONO, MOV, MP4, MPC, MPEG, MPG, MRW, MSL, MSVG, MTV, MVG, NEF, NRW, NULL, ORF, OTB, OTF, PAL, PALM, PAM, PANGO, PATTERN, PBM, PCD, PCDS, PCL, PCT, PCX, PDB, PDF, PDFA, PEF, PES, PFA, PFB, PFM, PGM, PGX, PICON, PICT, PIX, PJPEG, PLASMA, PNG, PNG00, PNG24, PNG32, PNG48, PNG64, PNG8, PNM, POCKETMOD, PPM, PS, PS2, PS3, PSB, PSD, PTIF, PWP, RADIAL-GRADIENT, RAF, RAS, RAW, RGB, RGB565, RGBA, RGBO, RGF, RLA, RLE, RMF, RW2, SCR, SCT, SFW, SGI, SHTML, SIX, SIXEL, SPARSE-COLOR, SR2, SRF, STEGANO, SUN, SVG, SVGZ, TEXT, TGA, THUMBNAIL, TIFF, TIFF64, TILE, TIM, TM2, TTC, TTF, TXT, UBRL, UBRL6, UIL, UYVY, VDA, VICAR, VID, VIFF, VIPS, VST, WBMP, WEBP, WMF, WMV, WMZ, WPG, X, X3F, XBM, XC, XCF, XPM, XPS, XV, XWD, YCbCr, YCbCrA, YUV gd_version: 2.2.5 gd_formats: GIF, JPEG, PNG, WebP, BMP, XPM ghostscript_version: 9.27 ### wp-server ### server_architecture: Linux 4.18.0-305.lve.el8.x86_64 x86_64 httpd_software: Apache php_version: 7.4.23 64bit php_sapi: litespeed max_input_variables: 1000 time_limit: 30 memory_limit: 128M admin_memory_limit: 256M max_input_time: -1 upload_max_filesize: 512M php_post_max_size: 512M curl_version: 7.71.0 OpenSSL/1.1.1d suhosin: false imagick_availability: true pretty_permalinks: true htaccess_extra_rules: false ### wp-database ### extension: mysqli server_version: 10.3.31-MariaDB client_version: 10.3.31-MariaDB ### wp-constants ### WP_HOME: undefined WP_SITEURL: undefined WP_CONTENT_DIR: /home/sakione/public_html/wp-content WP_PLUGIN_DIR: /home/sakione/public_html/wp-content/plugins WP_MEMORY_LIMIT: 40M WP_MAX_MEMORY_LIMIT: 256M WP_DEBUG: false WP_DEBUG_DISPLAY: true WP_DEBUG_LOG: false SCRIPT_DEBUG: false WP_CACHE: false CONCATENATE_SCRIPTS: undefined COMPRESS_SCRIPTS: undefined COMPRESS_CSS: undefined WP_LOCAL_DEV: undefined DB_CHARSET: utf8mb4 DB_COLLATE: undefined ### wp-filesystem ### wordpress: writable wp-content: writable uploads: writable plugins: writable themes: writable ### google-site-kit ### version: 1.42.0 php_version: 7.4.23 wp_version: 5.8.1 reference_url: https://mytakes.net amp_mode: no site_status: connected-site user_status: authenticated verification_status: verified-non-site-kit connected_user_count: 1 active_modules: site-verification, search-console, analytics, analytics-4, pagespeed-insights required_scopes: openid: ✅ https://www.googleapis.com/auth/userinfo.profile: ✅ https://www.googleapis.com/auth/userinfo.email: ✅ https://www.googleapis.com/auth/siteverification: ✅ https://www.googleapis.com/auth/webmasters: ✅ https://www.googleapis.com/auth/analytics.readonly: ✅ capabilities: googlesitekit_authenticate: ✅ googlesitekit_setup: ✅ googlesitekit_view_posts_insights: ✅ googlesitekit_view_dashboard: ✅ googlesitekit_view_module_details: ✅ googlesitekit_manage_options: ✅ enabled_features: ga4setup: ✅ ideaHubModule: ⭕ serviceSetupV2: ⭕ swgModule: ⭕ userInput: ⭕ unifiedDashboard: ⭕ search_console_property: https://mytakes.net/ analytics_account_id: 1955••••• analytics_property_id: UA-1955••••••• analytics_profile_id: 2522••••• analytics_use_snippet: yes analytics_4_property_id: 2883139•• analytics_4_web_data_stream_id: 2821•••••• analytics_4_measurement_id: G-H8•••••••• analytics_4_use_snippet: yes `
jamesozzie commented 3 years ago

@oscarssanchez I'm going to escalate this based on the information kindly you provided and the additional insights gathered. In the meantime if you could share your current hosting provider and your Site Health information that would be great.

Feel free to share privately if preferred. You will however need to reference a WordPress support forum topic which you can create here.

oscarssanchez commented 3 years ago

Hi @jamesozzie ,

Thanks, I have filled the questionnaire.

Personally, I think the error is expected, but what perhaps is not an expected behavior is that the response(?) stored in the description variable comes with some extra html

Let me know if I can provide more details. I'm also happy to submit a PR if you don't have the bandwidth/resources to tackle it, as It is probably not super concerning.

jamesozzie commented 3 years ago

Many thanks for sharing your Site Health information.

Personally, I think the error is expected, but what perhaps is not an expected behavior is that the response(?) stored in the description variable comes with some extra html

Correct. I'm unsure why the tag is output but the team will review this and provide an update here. They'll also be able to provide any suggestions on a PR, appreciate your assistance on this.

jamesozzie commented 3 years ago

One additional report, for this impacted user this is occurring within the AdSense widget:

adamdunnage commented 3 years ago

As part of some internal testing with free hosting providers I have encountered this error message on two of my test sites hosted with Byet.host & InfinityFreeHosting. Asana task and notes for this can be found here.

In both cases these error messages appear sporadically and are usually cleared with a page refresh. Screenshots of the error messages and console errors can be seen below for both sites.

Byet.host: SiteKitH1ErrorByet image

InfinityFree: SiteKitH1ErrorInfinity image

mxbclang commented 3 years ago

@tofumatt Assigning over to you per our discussion in today's sprint planning meeting. Thank you!

tofumatt commented 3 years ago

More specifically, it's likely that the message in https://github.com/google/site-kit-wp/blob/eed009c134ae533e307d65390e3af84d5551a9e7/assets/js/components/ReportError.js#L51 is what's causing this issue.

In summation: the error message sent from WordPress (specifically the WP REST API) here contains an error page with <h1>Some database error</h1>. That's the message our code is receiving and displaying—the literal "\<h1>" string appears because React (helpfully) escapes HTML in strings by default. But we should strip HTML tags entirely from error message strings to avoid things like this happening in the future.

I've added as much to the ACs. We should be able to use DOMPurify (our instance is in https://github.com/google/site-kit-wp/blob/eed009c134ae533e307d65390e3af84d5551a9e7/assets/js/util/purify.js#L6) to sanitise HTML strings to regular strings.

eugene-manuilov commented 3 years ago

Thanks, @kuasha420, but let's better update the ErrorText component to strip tags from the incoming message property instead of updating only the ReportError component.

kuasha420 commented 3 years ago

@eugene-manuilov Than it will not work when there is no reconnectUrl. See: https://github.com/google/site-kit-wp/blob/b6da750ce6c41ba2b3740874192bdafa82573dca/assets/js/components/ReportError.js#L63-L67

This will leave out the issue raised here unfixed.

Any thoughts?

eugene-manuilov commented 3 years ago

@kuasha420 ah... yes, you are right. Then let's strip tags from the message too if there is no reconnectURL. So, we need to add sanitization to both ErrorText and ReportError components, but for the ReportError component we need to sanitize the error message only if there is no reconnectionURL.

kuasha420 commented 3 years ago

@eugene-manuilov The ErrorText component already have html sanitization. I have updated the IB to only clean the message when there is no reconnectURL. Should be good now.

eugene-manuilov commented 3 years ago

Ok, you are right @kuasha420. 🤦‍♂️ IB ✔️

wpdarren commented 2 years ago

QA Update: ⚠️

@kuasha420 @asvinb please could you suggest how we might:

Ensure tests for ReportError pass.

I've gone into the network tab and selected slow 3G and played around with Chrome settings but I am unable to trigger the reported issue. Reading through the comments if does seem that we've not been able to recreate consistently, so could be down too slow servers on free hosting. Any advice how it could be tested would be appreciated.

eugene-manuilov commented 2 years ago

@asvinb could you please, help, Darren with his question?

eugene-manuilov commented 2 years ago

please could you suggest how we might:

Ensure tests for ReportError pass.

I've gone into the network tab and selected slow 3G and played around with Chrome settings but I am unable to trigger the reported issue. Reading through the comments if does seem that we've not been able to recreate consistently, so could be down too slow servers on free hosting. Any advice how it could be tested would be appreciated.

Actually, I think I can help too :smile:. @wpdarren it means that all js tests should pass which has already been confirmed during code review.

wpdarren commented 2 years ago

@eugene-manuilov so I can assume that it's only the story for me to look at? 👍

asvinb commented 2 years ago

Thanks for jumping in @eugene-manuilov . That's correct, I intentionally added the tests even though it was not mentioned in the IB just to make sure that HTML tags are stripped. So the tests passing only should be fine. And you're correct @wpdarren , that's the only story to look at.

wpdarren commented 2 years ago

QA Update: ✅

Verified:

Checked new stories (ReportError with HTML tags) for ReportError and ensured no HTML tags are displayed.

image