Make WordPress Core

Opened 7 weeks ago

Closed 4 weeks ago

#65914 closed defect (bug) (fixed)

wp_get_tooltip() treats custom button markup as a sprintf() format string

Reported by: ekamran Owned by: joedolson
Priority: normal Milestone: 7.1.1
Component: General Version: 7.1
Severity: major Keywords: tooltips has-patch has-unit-tests fixed-major dev-reviewed
Cc: Focuses:

Description

wp_get_tooltip() and wp_get_toggletip() accept a button argument. It is documented as:

@type string $button      Existing `button` or `a` markup. Used instead of generated button.
                          Default standard button HTML.

Inside wp_get_tooltip_helper(), that markup is concatenated into the sprintf() format string rather than passed as a value. So any percent sign in the caller's markup is read as a conversion specification.

Valid HTML can contain a percent sign. A percent-encoded URL is a common case, and wp_get_tooltip() accepts a markup, so this is easy to hit.

Steps to reproduce

A percent-encoded URL causes a fatal error:

echo wp_get_tooltip(
        'Helpful text.',
        array( 'button' => '<a href="/wp-admin/edit.php?s=hello%20world">Search</a>' )
);

Result:

Fatal error: Uncaught ValueError: Unknown format specifier "w"
in wp-includes/general-template.php:533

Ordinary text is silently corrupted:

echo wp_get_tooltip(
        'Helpful text.',
        array( 'button' => '<button type="button">100% done</button>' )
);

The button renders as 1000one instead of 100% done.

A percent-encoded path is silently rewritten. href="/x?p=a%2Fb" becomes href="/x?p=a0.000000b", so the link points somewhere else.

Core impact

do_meta_boxes() in wp-admin/includes/template.php builds the Move up, Move down, and Show or hide buttons by putting the meta box ID into the button markup, then passes that markup to wp_get_tooltip().

Meta box IDs come from plugins. add_meta_box() does not reject a percent sign in the ID, and this worked before the 7.1 tooltip button markup was introduced.

So a plugin doing only this is enough to cause a fatal error on the admin Dashboard in 7.1:

add_action( 'wp_dashboard_setup', function () {
        add_meta_box( 'sale_100%_off', 'Sale Report', function () {
                echo 'Hello.';
        }, 'dashboard', 'normal' );
} );

The page dies at that meta box, so every widget after it stops rendering.

Affected versions

This is new in 7.1 and is also present in trunk. Before 7.1, those buttons were echoed directly, so a percent sign in a meta box ID was not parsed by sprintf(). Checked on 7.0.4, where the same plugin renders the meta box normally.

Change History (27)

This ticket was mentioned in ​PR #13199 on ​WordPress/wordpress-develop by ​@ekamran.


7 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Trac ticket: https://core.trac.wordpress.org/ticket/65914

This PR fixes a regression in wp_get_tooltip() and wp_get_toggletip() where custom button markup passed via the button argument is concatenated into a sprintf() format string.

Because of that, literal percent signs in valid button or anchor markup can be treated as format specifiers. This can cause fatal errors or silently corrupt output, for example percent-encoded URLs or button text like 100% done.

The fix resolves the tooltip-supported placeholders in the button markup first, then passes the resulting button HTML to sprintf() as a value instead of as part of the format string.

Testing:

  • npm run test:php -- --group tooltip
  • composer lint:errors -- src/wp-includes/general-template.php tests/phpunit/tests/general/wpGetTooltip.php

Results:

  • Tooltip tests passed: 16 tests, 45 assertions.
  • PHPCS errors check passed for both touched files.

#2 @ekamran
7 weeks ago

  • Focuses tests coding-standards added

This ticket was mentioned in ​PR #13201 on ​WordPress/wordpress-develop by ​parthjogi-nyusoft.


7 weeks ago
#3

Fixes #65914

wp_get_tooltip_helper() concatenated caller-supplied button markup directly
into a sprintf() format string. Any % in that markup (e.g. a percent-encoded
URL, or a meta box ID like sale_100%_off) was parsed as a conversion
specification, causing either a fatal ValueError or silently corrupted output.

This patch passes the button markup to sprintf() as a plain argument instead
of concatenating it into the format string. The default core-generated
button's label/icon/popovertarget placeholders are now filled via
str_replace() on internal tokens before that point, so default-button
behavior is unchanged.

Added 5 tests to tests/phpunit/tests/general/wpGetTooltip.php covering the
exact repro cases from the ticket plus a regression check for the default
button. Full test file passes locally: 16 tests, 43 assertions.

Trac ticket: https://core.trac.wordpress.org/ticket/65914

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude (Anthropic)
Used for: Diagnosis of the root cause, implementation of the fix, and the
regression test suite. I reviewed the code, applied it in my local
wordpress-develop environment, and ran the full test suite
(16 tests, 43 assertions, all passing) before submitting.

#4 @parthjoginyusoft
7 weeks ago

Attached a patch (65914.diff) that fixes this by passing the button markup
to sprintf() as a plain argument instead of concatenating it into the format
string, so any % in caller-supplied markup is no longer parsed as a
conversion specification. The default core-generated button's own
label/icon/popovertarget placeholders are now filled via str_replace() on
internal tokens before that point, preserving existing default-button
behavior.

Added 5 tests to tests/phpunit/tests/general/wpGetTooltip.php covering the
exact repro cases from the ticket description plus a regression check for
the default button. Full test file passes locally: 16 tests, 43 assertions.

Also opened a companion PR for code review: ​https://github.com/WordPress/wordpress-develop/pull/13201

#5 @ekamran
7 weeks ago

  • Focuses tests coding-standards removed
  • Keywords tooltips added

#6 @joedolson
7 weeks ago

  • Milestone Awaiting Review → 7.1.1
  • Owner set to joedolson
  • Status new → accepted

#7 @sc0ttkclark
7 weeks ago

We saw an issue with Pods where we attempted to sanitize_title() on the meta box ID we generated but because non-ASCII labels can end up with % characters it causes that fatal for those cases too.

See: ​https://github.com/pods-framework/pods/issues/7615

Going to introduce a fix in Pods to get around this but glad to see this ticket is in the works.

#8 @jack960330
5 weeks ago

I encountered this problem, which caused all metaboxes to fail to display at certain times. I temporarily resolved the issue using the following code:

                $markup = sprintf(
                        '<span class="%1$s"> %6$s <span popover="hint" id="%2$s" class="wp-tooltip__bubble" role="tooltip">' .
                        '<span id="%2$s-text" class="wp-tooltip__text">%5$s</span>' .
                        '</span>' .
                        '</span>',
                        esc_attr( $classes ),
                        esc_attr( $id ),
                        esc_attr( $label ),
                        esc_attr( $icon ),
                        esc_html( $content ),
                        $button // 【修复点】:将 $button 作为第 6 个参数安全传入
                );

It will be used to override the following code:

/var/www/wordpress/wp-includes/general-template.php:533

I believe this problem can be solved, and it's worth mentioning that the solution comes from AI-generated content.

This ticket was mentioned in ​PR #13321 on ​WordPress/wordpress-develop by ​jack9603301.


5 weeks ago
#9

Trac ticket: [](https://core.trac.wordpress.org/ticket/65914)

When $button has a structure like %a, a PHP parser error can cause Pods or other similar custom metadata blocks to fail to display, resulting in a serious problem. This often disrupts the normal management and use of WordPress. This patch corrects this issue in a way that has been verified to be effective!

It's worth noting that, whether we like it or not, this solution is generated by AI combined with internet data retrieval. This solution is very simple and therefore involves no human creativity; I must admit, it was generated by AI.

Hopefully, it can be merged quickly to resolve the issue as much as possible. It aims to fix the following bugs:

  thrown in /var/www/wordpress/wp-includes/general-template.php on line
533" while reading response header from upstream, client: 10.20.245.1,
server: www.qhjack.top, request: "GET
/wp-admin/post.php?post=582&action=edit HTTP/1.0", upstream:
"fastcgi://unix:///run/php/php8.5-fpm.sock:", host: "www.qhjack.top",
referrer: "https://www.qhjack.top/wp-admin/edit.php?post_type=services"
2026/08/30 16:25:15 [error] 289643#289643: *106436 FastCGI sent in
stderr: "PHP message: PHP Fatal error:  Uncaught ValueError: Unknown
format specifier "a" in
/var/www/wordpress/wp-includes/general-template.php:533
Stack trace:
#0 /var/www/wordpress/wp-includes/general-template.php(533): sprintf()
#1 /var/www/wordpress/wp-includes/general-template.php(400):
wp_get_tooltip_helper()
#2 /var/www/wordpress/wp-admin/includes/template.php(1410):
wp_get_tooltip()
#3 /var/www/wordpress/wp-admin/includes/post.php(2392): do_meta_boxes()
#4 /var/www/wordpress/wp-admin/edit-form-blocks.php(401):
the_block_editor_meta_boxes()
#5 /var/www/wordpress/wp-admin/post.php(199): require('...')
#6 {main}

## Use of AI Tools

AI assistance: Yes
Tool(s): chrome
Model(s): Gemini
Used for: Analyzing the server error logs revealed that the patch was extremely simple, and therefore was entirely generated by AI.

#10 @joedolson
5 weeks ago

In reviewing the three different PRs provided, I'm going to close the PRs by @jack960330 and by @parthjoginyusoft, as they both introduce custom templating that I think is best avoided - one introducing brace-enclosed tokens and the other a custom replacement for sprintf markers. These are certainly very appreciated options to explore, but obviously there can be only one...

The PR by @ekamran looks like the better direction to go, and I'm going to focus attention there.

​@jack960330 commented on ​PR #13321:


5 weeks ago
#13

@joedolson You mean this issue can be resolved in #13199, so this PR isn't necessary, right?

This ticket was mentioned in ​Slack in #core by adrianduffell. ​View the logs.


5 weeks ago

​@joedolson commented on ​PR #13321:


4 weeks ago
#15

@jack9603301 Yes, that's what I mean! :)

This ticket was mentioned in ​Slack in #core by adamsilverstein. ​View the logs.


4 weeks ago

#17 @adamsilverstein
4 weeks ago

Please note the RC for 7.1.1 is September 10th if we want it included in that version. Otherwise it is fine to punt to 7.1.2.

This ticket was mentioned in ​Slack in #core by adrianduffell. ​View the logs.


4 weeks ago

#19 @joedolson
4 weeks ago

I think this is ready to go; but a second test would be appreciated before committing.

​@ekamran commented on ​PR #13199:


4 weeks ago
#20

Thank you for the approval and the doc cleanup. While reading your doc commit I noticed the same wording also sits in the wp_get_tooltip() and wp_get_toggletip() docblocks, both still say "Default standard button HTML". Since the default is now an empty string, should I push the same small fix to both? Happy to do it right away if you think it is worth it, or leave it to you.

​@joedolson commented on ​PR #13199:


4 weeks ago
#21

Good catch! Yes, please do update those, as well.

#22 @joedolson
4 weeks ago

  • Keywords commit added

I think this is ready to go. Marking for commit.

#23 @joedolson
4 weeks ago

  • Resolution → fixed
  • Status accepted → closed

In 63566:

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.

Props ekamran, parthjoginyusoft, sc0ttkclark, jack960330, adamsilverstein, joedolson.
Fixes #65914.

#24 @joedolson
4 weeks ago

  • Keywords fixed-major dev-feedback added; commit removed
  • Resolution fixed
  • Status closed → reopened

Reopening for 2nd committer approval for merge commit to 7.1

#25 @adamsilverstein
4 weeks ago

+1 to backporting, thanks @joedolson

#26 @adamsilverstein
4 weeks ago

  • Keywords dev-reviewed added; dev-feedback removed

#27 @joedolson
4 weeks ago

  • Resolution → fixed
  • Status reopened → closed

In 63571:

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.

Note: See TracTickets for help on using tickets.