WordPress.org

Make WordPress Core

Opened 5 weeks ago

Last modified 5 weeks ago

#44177 new enhancement

Enhance wp_debug_backtrace_summary() with the optional ability to include arguments

Reported by: DavidAnderson Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

The attached patch for wp_debug_backtrace_summary():

  • Adds the optional ability to include arguments in the output
  • Leaves the default behaviour as-is
  • Removes an unnecessary check on whether the PHP version is at least a value which is still lower than the minimum one that WP supports

Attachments (2)

wp_debug_backtrace_summary.diff (3.1 KB) - added by DavidAnderson 5 weeks ago.
wp_debug_backtrace_summary-2.diff (2.4 KB) - added by DavidAnderson 5 weeks ago.
Updated patch - previous version was not based off current SVN trunk

Download all attachments as: .zip

Change History (6)

#1 @johnbillion
5 weeks ago

  • Version trunk deleted

I think this is a good enhancement.

Query Monitor does a similar thing: https://github.com/johnbillion/query-monitor/blob/143c31484656495b024144b9e8697c863f8ec159/classes/Util.php#L336-L368 . After a skim over your patch, I think the main difference is that QM adds the object ID for some known object types.

@DavidAnderson
5 weeks ago

Updated patch - previous version was not based off current SVN trunk

#2 follow-up: @DavidAnderson
5 weeks ago

Though changing default behaviour is usually undesirable, I wonder if in the case of a debugging function whose output was, and remains, a textual string, if there's not a good case for changing it in this case? The extra info is harmless, and it's hard to envisage anybody parsing the output and relying upon it in a way that would be breaking. Having to pass all 4 parameters to get the info is a bit annoying.

#3 in reply to: ↑ 2 @jdgrimes
5 weeks ago

Replying to DavidAnderson:

Though changing default behaviour is usually undesirable, I wonder if in the case of a debugging function whose output was, and remains, a textual string, if there's not a good case for changing it in this case? The extra info is harmless, and it's hard to envisage anybody parsing the output and relying upon it in a way that would be breaking. Having to pass all 4 parameters to get the info is a bit annoying.

The extra info is not necessarily harmless. I can see how this could inadvertently expose passwords or be used for XSS under the correct circumstances.

Also, I don't know about parsing the output, but core itself searches the output for a particular function name in one of the unit tests. I don't know if any plugins do that in production code, but I wouldn't be surprised. If they do, it would potentially be possible for the user to alter behavior if they control the content of any of the parameters in the backtrace stack, by including a function name in the argument value.

#4 @DavidAnderson
5 weeks ago

@jdgrimes Good points.

Note: See TracTickets for help on using tickets.