Skip to content

Commit fb84620

Browse files
joedolsonjoedolson
committed
Administration: Prevent fatal error in sprintf replacement in tooltips.
If HTML passed directly the `button` argument of `wp_get_tooltip_helper()` contained a `%` character, this was interpreted as a placeholder for `sprintf()`, causing a PHP fatal due to a mismatch in argument count. Use the WP_HTML_Tag_Processor to fully construct the button HTML before running sprintf, so that supplied `%` characters are part of the value instead of the format string. Reviewed by adamsilverstein. Merges [63566] to the 7.1 branch. Props ekamran, parthjoginyusoft, sc0ttkclark, jack960330, adamsilverstein, joedolson. Fixes #65914. git-svn-id: https://develop.svn.wordpress.org/branches/7.1@63571 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 28b26a0 commit fb84620

2 files changed

Lines changed: 177 additions & 34 deletions

File tree

‎src/wp-includes/general-template.php‎

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ function get_search_form( $args = array() ) {
384384
* @type string $id Unique ID for the popover element. Default is a
385385
* generated unique ID.
386386
* @type string $button Existing `button` or `a` markup. Used instead of generated button.
387-
* Default standard button HTML.
387+
* Default empty string.
388388
* @type string $label Not used for tooltips.
389389
* @type string $close_label Not used for tooltips.
390390
* @type string $icon Dashicons icon class for the toggle button.
@@ -414,7 +414,7 @@ function wp_get_tooltip( $content, $args = array() ) {
414414
* @type string $id Unique ID for the popover element. Default is a
415415
* generated unique ID.
416416
* @type string $button Existing `button` markup. Used instead of generated button.
417-
* Default standard button HTML.
417+
* Default empty string.
418418
* @type string $label Accessible label for the toggle button.
419419
* Default 'Help', matching the default icon.
420420
* Ignored for tooltips.
@@ -449,7 +449,7 @@ function wp_get_toggletip( $content, $args = array() ) {
449449
* @type string $id Unique ID for the popover element. Default is a
450450
* generated unique ID.
451451
* @type string $button Existing `button` or `a` markup. Used instead of generated button.
452-
* Default standard button HTML.
452+
* Default empty string.
453453
* @type string $label Accessible label for the toggle button.
454454
* Default 'Help', matching the default icon.
455455
* Ignored for tooltips.
@@ -473,7 +473,7 @@ function wp_get_tooltip_helper( $content, $args = array() ) {
473473

474474
$defaults = array(
475475
'id' => wp_unique_id( 'wp-tooltip-' ),
476-
'button' => '<button type="button" aria-label="%3$s"><span class="dashicons %4$s" aria-hidden="true"></span></button>',
476+
'button' => '',
477477
'label' => __( 'Help' ),
478478
'close_label' => __( 'Close' ),
479479
'icon' => 'dashicons-editor-help',
@@ -488,51 +488,77 @@ function wp_get_tooltip_helper( $content, $args = array() ) {
488488
$classes .= ' ' . $args['class'];
489489
}
490490

491-
$icon = ( $args['icon'] ) ? trim( $args['icon'] ) : $defaults['icon'];
492-
$id = ( $args['id'] ) ? $args['id'] : $defaults['id'];
493-
$button = ( $args['button'] ) ? $args['button'] : $defaults['button'];
494-
$processed = false;
491+
$icon = ( $args['icon'] ) ? trim( $args['icon'] ) : $defaults['icon'];
492+
$id = ( $args['id'] ) ? $args['id'] : $defaults['id'];
493+
494+
// Tooltips use the content as the accessible name; toggletips use the label.
495+
$label = ( 'tooltip' === $args['type'] ) ? wp_strip_all_tags( $content, true ) : $args['label'];
496+
497+
/*
498+
* The generated button is a plain skeleton. Every dynamic attribute is
499+
* added through the tag processor below, so caller-supplied markup is
500+
* never scanned or substituted and a percent sign in custom markup,
501+
* such as a percent-encoded URL, is never treated as a conversion
502+
* specification.
503+
*/
504+
$default_button = '<button type="button"><span></span></button>';
505+
506+
$is_default = ! $args['button'];
507+
$button = ( $args['button'] ) ? $args['button'] : $default_button;
508+
509+
// The accepted root element is a `button`, or an `a` for tooltips.
510+
$tag = false;
495511
$processor = new WP_HTML_Tag_Processor( $button );
496512
if ( true === $processor->next_tag( 'button' ) ) {
497-
$processor->add_class( 'wp-tooltip__toggle' );
498-
if ( 'tooltip' !== $args['type'] ) {
499-
$processor->set_attribute( 'popovertarget', '%2$s' );
500-
$processor->set_attribute( 'aria-haspopup', 'dialog' );
501-
}
502-
$button = $processor->get_updated_html();
503-
$processed = true;
513+
$tag = 'button';
504514
} else {
505-
// Reset processor.
506515
$processor = new WP_HTML_Tag_Processor( $button );
507-
if ( true === $processor->next_tag( 'a' ) && 'tooltip' === $args['type'] ) {
508-
$processor->add_class( 'wp-tooltip__toggle' );
509-
$button = $processor->get_updated_html();
510-
$processed = true;
516+
if ( 'tooltip' === $args['type'] && true === $processor->next_tag( 'a' ) ) {
517+
$tag = 'a';
511518
}
512519
}
513-
if ( ! $processed ) {
514-
// Button HTML passed was not valid.
515-
$processor = new WP_HTML_Tag_Processor( $defaults['button'] );
516-
$processor->add_class( 'wp-tooltip__toggle' );
517-
if ( 'tooltip' !== $args['type'] ) {
518-
$processor->set_attribute( 'popovertarget', '%2$s' );
519-
$processor->set_attribute( 'aria-haspopup', 'dialog' );
520-
}
521-
$button = $processor->get_updated_html();
520+
521+
if ( false === $tag ) {
522+
// Button HTML passed was not valid. Reset to default.
523+
$is_default = true;
524+
$button = $default_button;
525+
$processor = new WP_HTML_Tag_Processor( $button );
526+
$processor->next_tag( 'button' );
527+
$tag = 'button';
522528
}
523529

530+
/*
531+
* Attributes that apply to every accepted button are added in one pass.
532+
* Attributes that name the control are only added when the caller's
533+
* markup did not already provide them.
534+
*/
535+
if ( null === $processor->get_attribute( 'aria-label' ) ) {
536+
$processor->set_attribute( 'aria-label', $label );
537+
}
538+
$processor->add_class( 'wp-tooltip__toggle' );
539+
if ( 'button' === $tag && 'tooltip' !== $args['type'] ) {
540+
$processor->set_attribute( 'popovertarget', $id );
541+
$processor->set_attribute( 'aria-haspopup', 'dialog' );
542+
}
543+
544+
// The generated button also carries the dashicon on its inner span.
545+
if ( $is_default && true === $processor->next_tag( 'span' ) ) {
546+
$processor->set_attribute( 'class', 'dashicons ' . $icon );
547+
$processor->set_attribute( 'aria-hidden', 'true' );
548+
}
549+
550+
$button = $processor->get_updated_html();
551+
524552
/*
525553
* The markup only uses phrasing content so it is valid when nested
526554
* in a phrasing context. Sectioning content (e.g. `div`, `dialog`) will
527555
* cause the parser to close an open `p`, creating an empty and breaking
528556
* the layout. See #65660.
529557
*/
530558
if ( 'tooltip' === $args['type'] ) {
531-
// Tooltips are only used to visually display labels.
532-
$label = wp_strip_all_tags( $content, true );
533559
$markup = sprintf(
534560
'<span class="%1$s">
535-
' . $button . '
561+
%6$s
536562
<span popover="hint" id="%2$s" class="wp-tooltip__bubble" role="tooltip">' .
537563
'<span id="%2$s-text" class="wp-tooltip__text">%5$s</span>' .
538564
'</span>' .
@@ -542,6 +568,7 @@ function wp_get_tooltip_helper( $content, $args = array() ) {
542568
esc_attr( $label ),
543569
esc_attr( $icon ),
544570
esc_html( $content ),
571+
$button,
545572
);
546573
} else {
547574
/*
@@ -551,7 +578,7 @@ function wp_get_tooltip_helper( $content, $args = array() ) {
551578
*/
552579
$markup = sprintf(
553580
'<span class="%1$s">
554-
' . $button . '
581+
%7$s
555582
<span popover="auto" id="%2$s" class="wp-tooltip__bubble" role="dialog" aria-label="%3$s" tabindex="-1" autofocus>' .
556583
'<span id="%2$s-text" class="wp-tooltip__text">%5$s</span>' .
557584
'<button type="button" class="wp-tooltip__close" popovertarget="%2$s" popovertargetaction="hide" aria-label="%6$s">' .
@@ -561,10 +588,11 @@ function wp_get_tooltip_helper( $content, $args = array() ) {
561588
'</span>',
562589
esc_attr( $classes ),
563590
esc_attr( $id ),
564-
esc_attr( $args['label'] ),
591+
esc_attr( $label ),
565592
esc_attr( $icon ),
566593
esc_html( $content ),
567594
esc_attr( $args['close_label'] ),
595+
$button,
568596
);
569597
}
570598

‎tests/phpunit/tests/general/wpGetTooltip.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,33 @@ public function test_wp_get_tooltip_has_custom_button_markup() {
110110
$this->assertStringNotContainsString( '<button', $html2, 'String does not contain button element.' );
111111
}
112112

113+
/**
114+
* Tests that a custom button without an accessible name gains one, and
115+
* that a caller-supplied aria-label is never overwritten.
116+
*
117+
* @ticket 65914
118+
*/
119+
public function test_wp_get_tooltip_custom_button_aria_label() {
120+
$html = wp_get_tooltip(
121+
'Helpful text.',
122+
array(
123+
'button' => '<button type="button"><span aria-hidden="true">?</span></button>',
124+
)
125+
);
126+
127+
$this->assertStringContainsString( 'aria-label="Helpful text."', $html, 'A custom button without an accessible name should gain one from the content.' );
128+
129+
$html2 = wp_get_tooltip(
130+
'Helpful text.',
131+
array(
132+
'button' => '<button type="button" aria-label="Custom name">?</button>',
133+
)
134+
);
135+
136+
$this->assertStringContainsString( 'aria-label="Custom name"', $html2, 'A caller-supplied aria-label should be preserved.' );
137+
$this->assertStringNotContainsString( 'aria-label="Helpful text."', $html2, 'The generated label should not overwrite the caller-supplied one.' );
138+
}
139+
113140
/**
114141
* Tests that the accessible labels are output and escaped in attributes.
115142
*
@@ -206,6 +233,94 @@ public function test_wp_get_toggletip_bubble_uses_dialog_role_and_autofocus() {
206233
$this->assertStringContainsString( 'tabindex="-1" autofocus>', $html );
207234
}
208235

236+
/**
237+
* Tests that a percent sign in the button markup is not treated as a
238+
* `sprintf()` conversion specification.
239+
*
240+
* @ticket 65914
241+
*
242+
* @dataProvider data_button_markup_containing_percent_signs
243+
*
244+
* @param string $button Button markup containing a percent sign.
245+
* @param string $expected Substring that must be preserved in the output.
246+
*/
247+
public function test_wp_get_tooltip_preserves_percent_signs_in_button_markup( $button, $expected ) {
248+
$this->assertStringContainsString(
249+
$expected,
250+
wp_get_tooltip( 'Helpful text.', array( 'button' => $button ) ),
251+
'The tooltip did not preserve the percent sign.'
252+
);
253+
$this->assertStringContainsString(
254+
$expected,
255+
wp_get_toggletip( 'Helpful text.', array( 'button' => $button ) ),
256+
'The toggletip did not preserve the percent sign.'
257+
);
258+
}
259+
260+
/**
261+
* Tests that a percent-encoded URL in anchor markup is not rewritten.
262+
*
263+
* @ticket 65914
264+
*
265+
* @dataProvider data_percent_encoded_hrefs
266+
*
267+
* @param string $href A percent-encoded URL.
268+
*/
269+
public function test_wp_get_tooltip_preserves_percent_encoding_in_anchor_href( $href ) {
270+
$html = wp_get_tooltip(
271+
'Helpful text.',
272+
array( 'button' => '<a href="' . $href . '">Search</a>' )
273+
);
274+
275+
$this->assertStringContainsString( 'href="' . $href . '"', $html );
276+
}
277+
278+
/**
279+
* Data provider.
280+
*
281+
* @return array[]
282+
*/
283+
public function data_percent_encoded_hrefs() {
284+
return array(
285+
'an encoded space' => array( '/wp-admin/edit.php?s=hello%20world' ),
286+
'an encoded slash' => array( '/x?p=a%2Fb' ),
287+
);
288+
}
289+
290+
/**
291+
* Data provider.
292+
*
293+
* @return array[]
294+
*/
295+
public function data_button_markup_containing_percent_signs() {
296+
return array(
297+
'a literal percent sign' => array(
298+
'<button type="button">100% done</button>',
299+
'100% done',
300+
),
301+
'a percent sign then a word' => array(
302+
'<button type="button">Save 20%!</button>',
303+
'Save 20%!',
304+
),
305+
'an unknown format specifier' => array(
306+
'<button type="button">Buy %q now</button>',
307+
'Buy %q now',
308+
),
309+
'a percent sign in an ID' => array(
310+
'<button type="button" aria-describedby="box_100%_complete-title">Move up</button>',
311+
'aria-describedby="box_100%_complete-title"',
312+
),
313+
'a percent-encoded ID' => array(
314+
'<button type="button" aria-describedby="meta-%d0%b8%d0%b3%d1%80%d0%b0-title">Move up</button>',
315+
'aria-describedby="meta-%d0%b8%d0%b3%d1%80%d0%b0-title"',
316+
),
317+
'text resembling a argnum' => array(
318+
'<button type="button">Use %2$s in your code</button>',
319+
'Use %2$s in your code',
320+
),
321+
);
322+
}
323+
209324
/**
210325
* Data provider.
211326
*

0 commit comments

Comments
 (0)