Opened 4 weeks ago
Last modified 3 weeks ago
#62824 reviewing defect (bug)
single_month_title() can cause a warning.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | 2.1 |
Component: | Date/Time | Keywords: | has-patch changes-requested |
Focuses: | Cc: |
Description
In my sentry setup I got this error message over the weekend.
Warning: Undefined array key "00"
caused by single_month_title()
This is the relevante piece of code.
<?php $m = get_query_var( 'm' ); // ... } elseif ( ! empty( $m ) ) { $my_year = substr( $m, 0, 4 ); $my_month = $wp_locale->get_month( substr( $m, 4, 2 ) ); }
This assumes that get_query_var( 'm' )
is always at least 6 digits, and always contains the year and the month.
But comparing to https://core.trac.wordpress.org/browser/tags/6.7/src/wp-includes/class-wp-query.php#L2048, this shows that this is not necessarily true.
I think that the most robust solution would be to add a sanitization to $wp_locale->get_month()
.
I will open a PR for that.
Change History (7)
This ticket was mentioned in PR #8142 on WordPress/wordpress-develop by @apermo.
4 weeks ago
#1
- Keywords has-patch added
#2
@
4 weeks ago
- Owner set to audrasjb
- Status changed from new to reviewing
Thanks for the ticket and the PR.
Maybe it would be more straightforward and performant to directly set a default value just in case? Like public function get_month( $month_number = '01' )
. What do you think?
#5
@
4 weeks ago
Hey @audrasjb.
I should have copied a little bit more.
Here is the bigger context.
<?php /** * Displays or retrieves page title for post archive based on date. * * Useful for when the template only needs to display the month and year, * if either are available. The prefix does not automatically place a space * between the prefix, so if there should be a space, the parameter value * will need to have it at the end. * * @since 0.71 * * @global WP_Locale $wp_locale WordPress date and time locale object. * * @param string $prefix Optional. What to display before the title. * @param bool $display Optional. Whether to display or retrieve title. Default true. * @return string|false|void False if there's no valid title for the month. Title when retrieving. */ function single_month_title( $prefix = '', $display = true ) { global $wp_locale; $m = get_query_var( 'm' ); $year = get_query_var( 'year' ); $monthnum = get_query_var( 'monthnum' ); if ( ! empty( $monthnum ) && ! empty( $year ) ) { $my_year = $year; $my_month = $wp_locale->get_month( $monthnum ); } elseif ( ! empty( $m ) ) { $my_year = substr( $m, 0, 4 ); $my_month = $wp_locale->get_month( substr( $m, 4, 2 ) ); } if ( empty( $my_month ) ) { return false; } $result = $prefix . $my_month . $prefix . $my_year; if ( ! $display ) { return $result; } echo $result; }
In that case that was caught by sentry, $m
was just a 4 digit representation of a year.
So I ended up in the elseif()
but sent an empty string to $wp_locale->get_month( substr( $m, 4, 2 ) )
resulting the warning and $my_month
to be empty, and thus the whole function to return false.
Returning January by default would be wrong in my eyes, and as I wanted to point out, not necessary, as single_month_title()
already accounts for it. I understand your point, and I also think this function as a whole could use an overhaul, but the one backwards compatible way me and my colleague (Props to @xate for digging into it with me) found was to add the sanitization as we did in the PR.
refactor(class-wp-locale.php): Added sanitization to WP_Locale::get_month()
This prevents a Warning: Undefined array key "00"
Co-authored-by: xateman
Refs: #62824
Trac ticket: https://core.trac.wordpress.org/ticket/62824#ticket