WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#31246 closed defect (bug) (fixed)

Notice: Undefined offset: 0 in wp-includes/query.php on line 3877

Reported by: Mediastuttgart Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: trivial Version: 4.1
Component: Query Keywords:
Focuses: Cc:
PR Number:

Description

Working with a fresh wordpress 4.1 installation and only installing the polylang plugin, you will receive a notice: undefined offset: 0 in wp-includes/query.php on line 3877 when using the language switcher and select "show all languages".

The backend URL for this request is wp-admin/edit.php?post_type=page&lang=all.

I assume this issue is not plugin related, as there maybe other scenarios where this issue could appear. Maybe the $tax_query_in_and array should be key-validated before trying to access its contents.

Possible reference https://github.com/WordPress/WordPress/commit/7e69e5484fa350a7451403f10d50129ffed0e18a

Environment
-----------

WordPress.........: 4.1
PHP...............: 5.5.10 (Darwin 14.0.0 Darwin Kernel Version 14.0.0: Fri Sep 19 00:26:44 PDT 2014; root:xnu-2782.1.97~2/RELEASE_X86_64 x86_64)
Database..........: mysql (v5.5.34)
Webserver.........: Apache
User Agent........: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.94 Safari/537.36

PHP-Settings
------------

register_globals..: disabled
magic_quotes_gpc..: disabled
safe_mode.........: disabled
error_reporting...: E_ALL
max_execution_time: 240 Seconds
memory_limit......: 128,00 MBytes
Extensions........: apache2handler, apc, apcu, bcmath, bz2, calendar, Core
                    ctype, curl, date, dom, ereg, exif, fileinfo
                    filter, ftp, gd, gettext, hash, iconv, imap
                    intl, json, ldap, libxml, mbstring, mcrypt, mysql
                    mysqli, openssl, pcre, PDO, pdo_mysql, pdo_pgsql, pdo_sqlite
                    pgsql, Phar, posix, Reflection, session, SimpleXML, soap
                    sockets, SPL, sqlite3, standard, tokenizer, xml, xmlreader
                    xmlwriter, xsl, yaz, zip, zlib

Plugins
-------

polylang            (1.6.4)

Change History (12)

#1 @boonebgorges
5 years ago

  • Keywords reporter-feedback added

Mediastuttgart - Thanks for the report. What you've described is a little odd. The logic in question is:

$queried_taxonomies = array_keys( $tax_query_in_and );
$matched_taxonomy = reset( $queried_taxonomies );
$query = $tax_query_in_and[ $matched_taxonomy ];

Line 3877 is the last line. It's not clear to me how we could fetch array keys on the first line, but then on the last line the system could complain that the array key doesn't exist :) Could you do some debugging to show the contents of the following variables at this point? $tax_query_in_and, $this->tax_query

#2 @Chouby
5 years ago

Hi!

I started to debug since I am the author of Polylang.
Here is the content of $this->tax_query:

Array
(
    [post_type] => post
    [lang] => all
    [posts_per_page] => 20
)

$tax_query_in_and is just an empty array at this point.

For you to get the full picture, I unset the query_var 'lang' in the parse_query action. Up to now it was sufficient to remove the tax query I wanted to remove from the query.

Maybe that's not sufficient anymore.

#3 follow-up: @boonebgorges
5 years ago

Hi Chouby - Thanks for chiming in! Could you please send a link to the place in your plugin where you are unsetting the query_var?

#5 follow-up: @boonebgorges
5 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.1.1
  • Severity changed from normal to trivial

Thanks for the link, Chouby.

Your use of 'lang=all' is confusing WP_Query a bit. Earlier in WP_Query::parse_query(), WP_Query::parse_tax_query() is detecting that the 'lang=all' query looks like a taxonomy term query, and so sets the is_tax flag to true. Then, at the 'parse_query' hook, you are unsetting the 'lang' argument, so that when the tax query is reparsed in get_posts(), no 'lang' parameter is found. In other words: WP_Query first thinks that this is going to be a taxonomy query, but by unsetting 'lang' later on, you're sorta pulling the rug out.

The trick here (and the reason you're unsetting the query var) is that 'all' is, of course, not actually a term in the 'language' taxonomy. I think your trick is OK, but you might consider running it a little sooner, maybe at 'parse_request', so that it would be sorted out before WP_Query has initialized.

In any case, this is strictly speaking a regression in get_queried_object() - the new logic in 4.1 is slightly less forgiving in odd cases like this - so let's go ahead and fix it for 4.1.1.

#6 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 31366:

In WP_Query::get_queried_object(), avoid PHP notices when is_tax is paired with an empty tax_query.

It's possible to have an empty tax_query and is_tax=true when the initial
query contains a taxonomy var (and is processed as such during
WP_Query::parse_query()) but the taxonomy var is unset during a 'parse_query'
callback. While this kind of behavior is not necessarily something we need to
support, we should continue to avoid PHP notices in such cases, as we did prior
to WP 4.1.

Fixes #31246.

#7 @boonebgorges
5 years ago

  • Keywords has-patch fixed-major commit added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.1.1 consideration. Technically a regression, though very minor.

#8 in reply to: ↑ 5 @Chouby
5 years ago

Replying to boonebgorges:

The trick here (and the reason you're unsetting the query var) is that 'all' is, of course, not actually a term in the 'language' taxonomy. I think your trick is OK, but you might consider running it a little sooner, maybe at 'parse_request', so that it would be sorted out before WP_Query has initialized.

Yes. parse_query is a very convenient action to modify the query when I need to. Here however, since the 'all' value can come only from the main request, you are right, it's better to use the request filter. Thanks for the suggestion.

#9 @Mediastuttgart
5 years ago

Thank you for addressing this topic. I am new to the wordpress universe and i am super impressed with your responsiveness.

#10 @dd32
5 years ago

@boonebgorges is this just a PHP Notice? or does it have a knock on effect in the query somewhere?

I'm inclined to leave it for 4.2 if it's only a notice.

#11 @Mediastuttgart
5 years ago

The author of the plugin just released a dev version (1.6.4.2, https://downloads.wordpress.org/plugin/polylang.zip) which fixes the issue.

#12 @boonebgorges
5 years ago

  • Keywords has-patch fixed-major commit removed
  • Milestone changed from 4.1.1 to 4.2
  • Resolution set to fixed
  • Status changed from reopened to closed

dd32 - Just a PHP notice. Return values have not changed. I have no problem moving it to 4.2, as it's quite an edge case anyway.

Mediastuttgart - Thanks for the update. If the plugin is updated, then it makes it even less urgent to fix this for 4.1.1.

Note: See TracTickets for help on using tickets.