Opened 6 years ago
Closed 6 years ago
#47867 closed defect (bug) (fixed)
wp_trim_excerpt and wp_trim_words don't validate the excerpt length (int)
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 5.2.2 |
Component: | Posts, Post Types | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
We noticed that our error log was filled with messages like this:
PHP message: PHP Warning: A non-numeric value encountered in [...]/public_html/wp-includes/formatting.php on line 3770
The immediate culprit turned out to be Elementor Pro, which allows the user to input an excerpt length for its "Cards" element, but doesn't validate the value at all, so if the user's input is blank then it will set the excerpt length to a blank string value rather than 0. It does so via the excerpt_length
filter.
I've submitted the fix to their support, but shouldn't WordPress's core be validating that value as well? I can't think of any possible reason why you would want WordPress to attempt to use a non-numeric value in those functions.
Here are the relevant lines in wp-includes\formatting.php that prompt the warning:
$excerpt_more = apply_filters( 'excerpt_more', ' ' . '[…]' ); $text = wp_trim_words( $text, $excerpt_length, $excerpt_more ); //if... $words_array = array_slice( $words_array[0], 0, $num_words + 1 ); //else... $words_array = preg_split( "/[\n\r\t ]+/", $text, $num_words + 1, PREG_SPLIT_NO_EMPTY );
Basically, the WordPress core sets the default excerpt length to 55, then applies the filters, then doesn't check the resulting value to make sure that it's valid.
I think that the sanitization logic should look something like this:
- If the resulting value is neither and integer nor a float, replace it with 0.
- Else if the resulting value is a float, cast it to an integer
- Else use the value without any modification (since it's an integer and therefore valid)
Assuming that that all sounds good, I'm not sure whether it would be more appropriate to put the check at the end of wp_trim_excerpt
or at the start of wp_trim_words
(or both?).
Attachments (2)
Change History (8)
#1
@
6 years ago
- Keywords has-patch has-unit-tests added
@pikamander2 - Welcome in the trac. Congratulations and thank you for your very first ticket.
I've attached a patched - 47867.diff - that should fix your concern. However instead of using 0
as length for non-numeric $num_words
passed to wp_trim_words()
(e.g. ' ', 'abc'), I think it's more reasonable to use the default 55
length.
Also, I didn't apply the fix in wp_trim_excerpt()
since it's ultimately uses wp_trim_words()
.
#2
@
6 years ago
Hi @donmhico, thanks for the quick fix.
My main concern with defaulting the value to 55 is that it changes a longstanding behavior. Any plugins or themes that are passing "" or null to that function are currently receiving zero words, but once the patch is applied they'll be receiving 55 words.
In the case of Elementor Pro, for example, many users' Post Cards will suddenly change from this:
https://i.imgur.com/qljC2Ec.png
to this:
https://i.imgur.com/SNnqn5g.png
And because it's an obscure issue, they're gonna have no idea what caused it or how to fix it. That's going to lead to a lot of frustration.
Granted, that's not necessarily WordPress's problem, and it might be better to just go ahead and "rip off the band-aid" so to speak, but it's definitely something to consider.
#3
@
6 years ago
Hey @pikamander2, I see your point. I supposed that if existing plugins / themes do want to use the default 55, they should have not passed anything as $num_words
instead of passing empty string or null. And WP.org stays true to provide backward compatibility and not breaking any existing behaviour as much as possible.
I'll upload an updated patch shortly.
Patch + unit tests.