Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 9 days ago

#59613 closed defect (bug) (duplicate)

Ensure $atts passed to $callback of add_shortcode() is always an array

Reported by: bedas's profile bedas Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Shortcodes Keywords: has-patch 2nd-opinion dev-feedback
Focuses: Cc:

Description

Problem

According to the documentation:

$callback callable Required
The callback function to run when the shortcode is found.
Every shortcode callback is passed three parameters by default, including an array of attributes ($atts), the shortcode content or null if not set ($content), and finally the shortcode tag itself ($shortcode_tag), in that order.

However, this part...

including an array of attributes ($atts)

...is not always true:

  • if the shortcode is used without any attributes, then $atts is actually an empty string.
  • apparently (according to code docbloc) it would also be a string of original arguments if it could not be parsed.

This discrepancy causes other dependant code to always fumble with the type of $atts, for example the source code of shortcode_atts has to typecasts $atts to array.
This would not be necessary, would $atts indeed be always an array from the start.

Further, in more complex programming scenarios where we assume $atts to be an array (as stated by the documentation), again we have to fumble with $atts, and in some cases, we cannot do anything but just accept that it may or may not be an array.
You can read more on that here

Cause

Thanks to the kind help of @bcworkz and further testing, I can confirm that the issue is within shortcode_parse_atts function - to be precise in the else statement here.
If the checks for if ( preg_match_all( $pattern, $text, $match, PREG_SET_ORDER ) ) fail, $atts is transformed to string using ltrim.

This makes thus poor sense to me:

  1. First of all, the documentation mentions that $atts is an array. It never mentions other possible types.
  2. It seem quite clear to me that $atts is always supposed and intended to be an array - never a string.
  3. Even if the docblock for shortcode_parse_atts states that the returned value can be array or string (and if string, then the original arguments string if it cannot be parsed)... I do not see the actual reason or justification for this.

What is the usefulness of returning a string of arguments if it cannot be parsed or is empty?
It should then safely fail into an empty array, to keep consistency and not force dependant code to typecast $atts.

Proposal

Simply remove the else block in shortcode_parse_atts.
It is redundant and creates issues in particular coding structures + clear discrepancies in documentation.

Change History (3)

This ticket was mentioned in PR #5480 on WordPress/wordpress-develop by @bedas.


4 months ago
#1

Ensures $atts passed to $callback of add_shortcode() is always an array

#2 @swissspidy
4 months ago

  • Focuses docs performance sustainability php-compatibility removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #59249.

Note: See TracTickets for help on using tickets.