-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Use preg_last_error() inside convert_smilies() to check for any preg_split errors before running count(). #1124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…o avoid issues in ticket #51019
Fast-forward for to match upstream
|
I am having the same issue. I have multiple images with base64 as url. While loading the page the images are missing because preg_split() can't handle the bigger size of the string and thus returns false. Please add a check for this function and merge this pull request. Thank you! |
|
Yeah, I've also seen the issue reported by @petri8 . It happens when the I believe it's an easy fix. |
|
Same, +1 Up 🙌 |
|
Is there any ETA on this..? |
src/wp-includes/formatting.php
Outdated
| $textarr = preg_split( '/(<.*>)/U', $text, -1, PREG_SPLIT_DELIM_CAPTURE ); // Capture the tags as well as in between. | ||
|
|
||
| if ( preg_last_error() != PREG_NO_ERROR ) { | ||
| trigger_error( preg_last_error_msg(), E_USER_WARNING ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using trigger_error, wp_trigger_error should be used. First pass at a more informative error message.
| trigger_error( preg_last_error_msg(), E_USER_WARNING ); | |
| $message = sprintf( '<code>convert_smilies</code> could not split text. Error Message: %s', preg_last_error_message() ); | |
| wp_trigger_error( 'convert_smilies', $message, E_USER_WARNING ); |
|
@nathkrill are you able to refresh your PR with the requested changes? Thank you! |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @nathancrowd, @nathkrill, @petri8, @alex-lucaci, @borzaszto, @lzudor-omnitech. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Is this issue abandoned now..? |
| // HTML loop taken from texturize function, could possible be consolidated. | ||
| $textarr = preg_split( '/(<.*>)/U', $text, -1, PREG_SPLIT_DELIM_CAPTURE ); // Capture the tags as well as in between. | ||
|
|
||
| if ( preg_last_error() != PREG_NO_ERROR ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if ( preg_last_error() != PREG_NO_ERROR ) { | |
| if ( preg_last_error() !== PREG_NO_ERROR ) { |
Should we use strict checking here? Note that PREG_NO_ERROR is 0.
|
Thanks for the PR! Merged a slightly different approach in r59515. |
Use preg_last_error() inside convert_smilies() to check for any preg_split errors before running count().
If there are errors, sends a PHP warning with the error message, then returns the original text.
Running count() on the array after there are preg_split errors can cause post content to not display (see ticket).
This is a reformat of a previous pull request to get the PHP tests to pass.
Trac ticket: https://core.trac.wordpress.org/ticket/51019
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.