-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix Message::bodySummary
when preg_match
fails
#554
Conversation
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.
I think the right fix here is just to add a === 0
?
preg_replace returns "false" on failure which interpreted as OK result which is wrong. Reform statement in positive manner makes it right
0565b25
to
900d1fd
Compare
I guess you right |
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 we did as you suggest, the function might return non-printable bytes when preg_match
is “broken”. Note that the documentation specifically says “Will return null
if the response is not printable.” and we just cannot tell without preg_match
.
By the way, how does preg_match
even break?
For example when you has utf-8 malformed symbol in tested string (which you get when cut utf-8 string code in the middle of multibyte UTF character).
No, it will not. When preg_match breaks you get if (preg_match('/[^\pL\pM\pN\pP\pS\pZ\n\r\t]/u', $summary) === 0) {
return $summary;
}
return null; |
Message::bodySummary
when preg_match
fails
Right, but that is precisely what I would expect based on “Will return null if the response is not printable.” UTF-8 malformed symbol is not printable. |
Turns out I was misunderstanding you. It was previously returning a malformed UTF-8 and your change makes it return |
preg_replace returns "false" on failure which interpreted as OK result which is wrong.
Reform statement in positive manner makes it right