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.

Adds tests to first validate the bug exists (test fail when the fix is not applied).

Fix:

• Move substr() to assign to the variable $my_month as it can return an empty string.
• If a month was found (not an empty string), then pass it to $wp_locale->get_month().

Makes the long concatenated title string multiline for readability.

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

[hellofromTonya]

Hello folks,

I too was able to reproduce the issue and 31521.4.diff​ resolves it.

I created ​PR 1861 which:

• adds unit tests (which do fail when the fix is not applied)
• applies the 31521.4.diff​ fix
• for the $title concatenated string: puts each date part on a separate line for improved readability

Marking for commit. Props to @costdev for the code review.

[hellofromTonya]

Removing needs-testing as @audrasjb tested the original patch (which is used in the PR).

[hellofromTonya]

In 52136:

Template: Fix "undefined index: 00" when archive month query is empty in wp_title().

When m query_tag has a valid year, i.e. ?m=2021, and there are posts for that year, substr() returns a false on PHP 5.6 and an empty string on PHP 7.0+. Passing either of those values to $wp_locale->get_month() results in a PHP notice on PHP 5.6 to PHP 7.4 and a PHP Warning on PHP 8.0+.

Why? The $month lookup table has zeroized keys from '01' to '12'. A empty value is passed to zeroise() returns '00' which is directly passed as a key in the month property. That key does not exist.

While $wp_locale->get_month() would benefit from guarding/validation, this fix ensures a falsey value is not passed as a month.

Tests are added including a test that fails with this fix not applied.

Follow-up to [801], [35294], [35624].

Props antpb, audrasjb, costdev, davidmosterd, drewapicture, herregroen, hellofromTonya, michelwppi, sergeybiryukov.
Fixes #31521.

[prbot]

​hellofromtonya commented on ​PR #1861:

Committed via changeset https://core.trac.wordpress.org/changeset/52136.

[hellofromTonya]

Thank you everyone for contributing! The fix will ship with 5.9.

Also note, on PHP 8.0+, this fixes a PHP Warning.