Make WordPress Core

Opened 6 months ago

Last modified 4 weeks ago

#59310 assigned defect (bug)

Parse blocks being applied unnecessarily

Reported by: spacedmonkey's profile spacedmonkey Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.3.1
Component: Editor Keywords: has-patch
Focuses: performance Cc:

Description

The filter wp_pre_kses_block_attributes is applied to any field that runs through wp_kses_hook. Any fields on any object field that is run through wp_filter_kses. This includes fields like comment_author_email, user_email, term_name and user_last_name. It is extremely unlikely that these text fields will contain block data. This filter should be changed.

Attachments (1)

Screenshot 2023-09-07 at 14.04.12.png (190.8 KB) - added by spacedmonkey 6 months ago.

Download all attachments as: .zip

Change History (10)

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


6 months ago

#2 @joemcgill
6 months ago

This was added in [46896], which seems to have been done for security purposes, so whomever looks into this should ensure that any adjustments made here don't expose the security bug that this was added to fix.

#3 @spacedmonkey
6 months ago

I wonder if @aduth or @whyisjake can add some context here.

#4 @aduth
6 months ago

Yes, the referenced patch was related to a security vulnerability, so I’d exercise caution with any revisions. That said, there were additional tests in the private security test suite beyond what’s included in the public commit, which may provide some assurances to avoid a regression.

As far as fields where block content isn’t expected or supported, I don’t specifically recall if those were susceptible in the same way, so it might be possible to exclude them specifically for some gains.

There may also be some options to optimize such that it’s only applied at save-time, but it’d need to be done in a way that handles legacy content.

I’d suggest collaborating with someone on the security team to navigate any potential regressions, as well as to help run the security test suite against any proposed patches.

This ticket was mentioned in PR #5174 on WordPress/wordpress-develop by @spacedmonkey.


6 months ago
#5

  • Keywords has-patch added

#6 @spacedmonkey
6 months ago

I have put together a PR as a POC of remove filter on term name. This reduced the number of times blocks are parsed. See https://github.com/WordPress/wordpress-develop/pull/5174

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


6 months ago

#8 @spacedmonkey
6 months ago

  • Owner set to spacedmonkey
  • Status changed from new to assigned

#9 @spacedmonkey
4 weeks ago

  • Owner spacedmonkey deleted

I am unable to work on this ATM. I wonder if @joemcgill or @flixos90 has time to look into this.

Note: See TracTickets for help on using tickets.