wp_title if archive of year w/o permalink fires php notice in locale.php

[michelwppi]

PHP Notice : Undefined index: 00 in /Applications/MAMP/htdocs/wp_svn41/wp-includes/locale.php on line 271

The concerned function : $wp_locale->get_month

Permalinks = default like: www.mysite.com/?m=2015

The calling function = wp_title in general-template.php

Here:

// If there's a month
        if ( is_archive() && !empty($m) ) {
                $my_year = substr($m, 0, 4);
                $my_month = $wp_locale->get_month(substr($m, 4, 2)); error_log("here calling...");
                $my_day = intval(substr($m, 6, 2));
                $title = $my_year . ( $my_month ? $t_sep . $my_month : '') . ( $my_day ? $t_sep . $my_day : '' );
        }

This part of function forget that 'm' query_tag can have a length from 4 to 9+ as well defined in query.php - not only month...

if ( $qv['m'] ) {
                                $this->is_date = true;
                                if ( strlen($qv['m']) > 9 ) {
                                        $this->is_time = true;
                                } else if ( strlen($qv['m']) > 7 ) {
                                        $this->is_day = true;
                                } else if ( strlen($qv['m']) > 5 ) {
                                        $this->is_month = true;
                                } else {
                                        $this->is_year = true;
                                }
                        }

and $wp_locale->get_month do not like empty string giving '00' index...

Suggestion (raw):

Add test ( strlen($m) >= 5 ) before to call $wp_locale->get_month

like

$my_month = ( strlen($m) >= 5 ) ? $wp_locale->get_month(substr($m, 4, 2))
: "";

Cheers,

M.

[DrewAPicture]

Hi @michelwppi, are you still experiencing this issue in 4.2?

[michelwppi]

@DrewAPicture
This issue remains in WP 4.2 because the test of

strlen($m) >=5

is not done in function wp_title line 829 like done in wp_query of wp_query.php.

Cheers

[herregroen]

Currently looking at this.

To help others, the steps to reproduce:

1. set permalinks to plain.
2. Add wp_title somewhere on your archive page.
3. Go to yourdomain.com?m=2017.
4. See the error before wp_title is output.

[herregroen]

Cleans up date parsing in wp_title.

[davidmosterd]

I was discussing with @herregroen that we could also just move the $wp_locale->get_month( $my_month ) check to the conditional echo. That would minimize the adjustment to the code and solve the issue as well. Like so:

$title = $my_year . ( $my_month ? $t_sep . $wp_locale->get_month( $my_month ) : '' ) . ( $my_day ? $t_sep . $my_day : '' );

[herregroen]

Added a patch that cleans up the date parsing by reusing the same WP_Query globals is_time, is_day and is_month to do the checks.

Used an array to implode to avoid a huge assignment to $title.

More changes but I think overall more readable code.

[davidmosterd]

Patch with my comment

[DrewAPicture]

Assigning to mark the good-first-bug as "claimed".

[audrasjb]

patch refreshed against trunk

[audrasjb]

Before patch

[audrasjb]

After patch

[audrasjb]

I was able to reproduce the issue and I can confirm the proposed patch solves it. In 31521.3.diff, I just refreshed the patch against trunk.

[audrasjb]

Reassigning to @SergeyBiryukov for review, sorry I didn't see you were reviewing.

[audrasjb]

Patch refresh

[audrasjb]

Patch refreshed against trunk. Marking for commit.

[hellofromTonya]

In reviewing the patch, noticed it is using substr() and does not have guarding to check for failed state. In PHP 8, substr() return type changed for failure from boolean false to an '' empty string. This change could have impact on the patch or upstream from it.

Today is 5.8 Beta 1. After discussing with @antpb, decided to punt to 5.9 to give time for deeper review and adding tests.

Reassigning ownership to me to follow-up on the review and tests.