search

Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.58k stars 799 forks source link

Google Fonts: formating the fontFamily block attribute fails if it's not a string #38311

Closed jhnstn closed 1 month ago

jhnstn commented 1 month ago

Impacted plugin

Jetpack

Quick summary

Any block can add the attribute fontFamily as something other than a string. However, format_font() method assumes the attribute is a string https://github.com/Automattic/jetpack/blame/trunk/projects/plugins/jetpack/modules/google-fonts/current/class-jetpack-google-font-face.php#L149

This can cause issues when trying to render a block which can in turn prevent updating a post.

This was found while trying to solve an issue where a widget contained a block that used fontFamily object attribute. The specific block attribute can be seen here https://plugins.trac.wordpress.org/browser/easy-quotes/trunk/build/block.json#L93

Trying to remove that block from a widget results in the error PHP Fatal error: Uncaught TypeError: strtolower(): Argument #1 ($string) must be of type string, array given in /wordpress/plugins/jetpack/13.7-a.1/modules/google-fonts/current/class-jetpack-google-font-face.php:149

Steps to reproduce

Steps

  1. Install a plugin that has adds a block which defines a fontFamily that is not a string ( https://wordpress.org/plugins/easy-quotes/ for example )
  2. Add the block to a page of a WordPress.com site
  3. Try saving the page

A clear and concise description of what you expected to happen.

The page should be saved

What actually happened

The API returns an error and the post is not saved.

Impact

One

Available workarounds?

No but the platform is still usable

Platform (Simple and/or Atomic)

Atomic

Logs or notes

Probably just need to do a string check here https://github.com/Automattic/jetpack/blame/trunk/projects/plugins/jetpack/modules/google-fonts/current/class-jetpack-google-font-face.php#L124

jhnstn commented 1 month ago

Originally reported here: 8470578-zd-a8c

github-actions[bot] commented 1 month ago

Support References

This comment is automatically generated. Please do not edit it.

jeherve commented 1 month ago

@Automattic/lego Is that something you could look at?

Thank you!

fushar commented 1 month ago

Thanks @jeherve.

@arthur791004 -- since it looks like an easy fix, and you're the one most familiar with this area, would mind looking at it sometime next week? Let's still prioritize the ETK migration, though. cc: @taipeicoder

jhnstn commented 1 month ago

I opened a PR with the fix. I added tests but was having a hard time running them locally. I'll be AFK next week so you can take over that PR.

Edit: Tests are passing