-
Notifications
You must be signed in to change notification settings - Fork 112
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
fixes core#4047 - htmlspecialchars called on empty array #356
fixes core#4047 - htmlspecialchars called on empty array #356
Conversation
(Standard links)
|
HTML/Common.php
Outdated
@@ -141,7 +141,7 @@ function _getAttrString($attributes) | |||
if (is_array($attributes)) { | |||
$charset = HTML_Common::charset(); | |||
foreach ($attributes as $key => $value) { | |||
$strAttr .= ' ' . $key . '="' . htmlspecialchars(($value ?? ''), ENT_COMPAT, $charset) . '"'; | |||
$strAttr .= ' ' . $key . '="' . htmlspecialchars(($value ?: ''), ENT_COMPAT, $charset) . '"'; |
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.
Is 0 a possible legit value here?
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.
Hmm - good point. I can't say definitively but I'll account for that.
ad6f08b
to
4bf4eca
Compare
HTML/Common.php
Outdated
@@ -141,7 +141,7 @@ function _getAttrString($attributes) | |||
if (is_array($attributes)) { | |||
$charset = HTML_Common::charset(); | |||
foreach ($attributes as $key => $value) { | |||
$strAttr .= ' ' . $key . '="' . htmlspecialchars(($value ?? ''), ENT_COMPAT, $charset) . '"'; | |||
$strAttr .= ' ' . $key . '="' . htmlspecialchars((is_array($value) ? '' : $value ?? ''), ENT_COMPAT, $charset) . '"'; |
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.
@MegaphoneJon just for the sake of clarity are you able to put (
and )
around the $value ?? '' in the false part of the first ternary operator
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.
Sorry @seamuslee001, just saw this.
4bf4eca
to
67d6294
Compare
thanks @MegaphoneJon |
Full details on https://lab.civicrm.org/dev/core/-/issues/4047, but basically - we are sometimes passing empty arrays to this function. Seems like the other instances that were changed on #346 will always be strings, unlike this one.