Make WordPress Core

Opened 7 years ago

Last modified 5 months ago

#39827 reopened defect (bug)

notice in wp-includes/canonical.php:392

Reported by: jakubbis's profile jakubbis Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7.2
Component: Canonical Keywords: has-patch needs-testing needs-unit-tests needs-testing-info
Focuses: Cc:

Description

Hello,

We're getting notice in wp-includes/canonical.php:392, but it requires some specific steps to obtain it.

Steps:

  1. Setup a clean install of WPML
  2. Set permalinks to postname without ending slash: /%postname%
  3. Create a page in secondary language
  4. Set this page as Home Page (under Settings/Reading) but only in the secondary language

When you enter "home page", you get 404 because a page for original language does not exist.
In line 121, you get $redirect_url = get_permalink($redirect_post); and value of $redirect_url is for instance "http://testsite.dev?lang=fr". As you see, there is no "/" before "?", that's why path is empty.

You have already fixed the similar issue in line:77-79

// Notice fixing
if ( !isset($redirect['path']) )
   $redirect['path'] = '';


I suspect, we need the same fix before line: 392

$redirect['path'] = preg_replace('|/' . preg_quote( $wp_rewrite->index, '|' ) . '/*?$|', '/', $redirect['path']);

Attachments (1)

39827.diff (1.0 KB) - added by jipmoors 7 years ago.
Avoid notice on non-existing key.

Download all attachments as: .zip

Change History (32)

#1 @SergeyBiryukov
7 years ago

  • Component changed from General to Canonical

#2 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 4.8

#4 @jipmoors
7 years ago

Hi @jakubbis,

Thank you for submitting this report and the detailed information with it.

I have taken a look at the file and the locations you mentioned and I have determined where this problem is coming from.
There are two error suppressed calls to parse_url which both can cause this issue.

@jipmoors
7 years ago

Avoid notice on non-existing key.

#5 @jipmoors
7 years ago

  • Keywords has-patch needs-testing added

This ticket was mentioned in Slack in #core by obenland. View the logs.


7 years ago

#7 @obenland
7 years ago

  • Milestone changed from 4.8 to Future Release

#8 @SergeyBiryukov
3 years ago

#52864 was marked as a duplicate.

#10 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by francina. View the logs.


3 years ago

#14 @hellofromTonya
3 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. As there's been no activity, punting to 5.9.

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#17 @sayedulsayem
3 years ago

  • Resolution set to invalid
  • Status changed from reviewing to closed

There is no need to assign an empty string after empty checking. Going to close this ticket. from bug scrub discussion.

#18 @SergeyBiryukov
3 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

It looks like there was some confusion during the bug scrub, as someone thought the problem is already solved in lines 83-85 of canonical.php.

However, that part was introduced in [9506] and adjusted in [13866]. It already existed when this ticket was created. The patch here touches a different area of the code, after the parse_url() call in line 568 further down.

The ticket still seems valid. Reopening for additional review.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#20 @audrasjb
3 years ago

  • Milestone changed from 5.9 to Future Release

As per today's bug scrub, it appears this ticket needs investigating to confirm the problem is there. If yes, then manual and automated testing. Moving for Future release consideration for now.

#21 follow-up: @psykro
2 years ago

@jakubbis thanks for sharing the set up steps.

I hope you don't mind me asking this, but I'm curious as to step 2

Set permalinks to postname without ending slash: /%postname%

Is there a specific reason you're creating this permalink. I only ask because if you click on the Post name option under Permalinks, it adds the trailing slash, which is generally expected when setting permalinks and using WordPress rewrite URLs. So I'm interested in the specific use case to need to remove it.

#22 in reply to: ↑ 21 @jakubbis
2 years ago

@psykro I can't remember exactly, but I think a customer who used our plugin - WPML complained about this issue. Initially, I thought it was on our side, but I came across the problem I reported here after further debugging.

I can't tell why that guy decided to skip an ending slash, but according to my experience, it is not such a rare situation.

#23 @psykro
2 years ago

Thanks @jakubbis I just tested this ticket and I was not able to replicate the issue. Attempting to access my local site at https://wordpress.test?lang=fr automatically redirects me to https://wordpress.test/?lang=fr.

Are you able to confirm this is still an issue under your test conditions?

#24 follow-up: @ultraflux79
2 years ago

Confirmed here

WP 5.9.3, Litespeed Enterprise, PHP8

[23-Jun-2022 20:01:24 UTC] Backtrace from warning 'Undefined array key "path"' at /wp-includes/canonical.php 590: /wp-includes/canonical.php 798 calling redirect_canonical() | /wp-content/plugins/perfmatters/inc/functions.php 104 calling redirect_canonical() | /wp-includes/class-wp-hook.php 307 calling perfmatters_disable_rss_feeds() | /wp-includes/class-wp-hook.php 331 calling apply_filters() | /wp-includes/plugin.php 474 calling do_action() | /wp-includes/template-loader.php 13 calling do_action() | /wp-blog-header.php 19 calling require_once() | /index.php 17 calling require()

#25 in reply to: ↑ 24 ; follow-up: @psykro
2 years ago

Replying to ultraflux79:

Confirmed here

WP 5.9.3, Litespeed Enterprise, PHP8

[23-Jun-2022 20:01:24 UTC] Backtrace from warning 'Undefined array key "path"' at /wp-includes/canonical.php 590: /wp-includes/canonical.php 798 calling redirect_canonical() | /wp-content/plugins/perfmatters/inc/functions.php 104 calling redirect_canonical() | /wp-includes/class-wp-hook.php 307 calling perfmatters_disable_rss_feeds() | /wp-includes/class-wp-hook.php 331 calling apply_filters() | /wp-includes/plugin.php 474 calling do_action() | /wp-includes/template-loader.php 13 calling do_action() | /wp-blog-header.php 19 calling require_once() | /index.php 17 calling require()

Thanks, are you able to share the steps you followed to replicate the issue?

#26 in reply to: ↑ 25 @ultraflux79
2 years ago

Replying to psykro:

Replying to ultraflux79:

Confirmed here

WP 5.9.3, Litespeed Enterprise, PHP8

[23-Jun-2022 20:01:24 UTC] Backtrace from warning 'Undefined array key "path"' at /wp-includes/canonical.php 590: /wp-includes/canonical.php 798 calling redirect_canonical() | /wp-content/plugins/perfmatters/inc/functions.php 104 calling redirect_canonical() | /wp-includes/class-wp-hook.php 307 calling perfmatters_disable_rss_feeds() | /wp-includes/class-wp-hook.php 331 calling apply_filters() | /wp-includes/plugin.php 474 calling do_action() | /wp-includes/template-loader.php 13 calling do_action() | /wp-blog-header.php 19 calling require_once() | /index.php 17 calling require()

Thanks, are you able to share the steps you followed to replicate the issue?

Unfortunately, I do not, I just took this website on.

#27 @RavanH
2 years ago

Hi all, I'm able to replicate by visiting the site URL /?feed=xyz (non-existant feed). I've reported this already 4 year ago on the related (duplicate?) ticket https://core.trac.wordpress.org/ticket/34353#comment:14

My proposed patch https://github.com/WordPress/wordpress-develop/pull/2687 should address all these cases.

#28 @ironprogrammer
2 years ago

  • Keywords needs-testing-info added

I've also been unable to reproduce this issue so far. I tried using the Plain permalink setting suggested by @RavanH (http://wp-test.test/?p=123), but couldn't repro 😔

For anyone still experiencing this issue, it would be most excellent if you would provide a list of test instructions and reproduction environment information on this ticket 🙌🏻 Please make sure to include any environment setup information that is necessary to reproduce the problem (e.g. a specific plugin or site configuration).

Please note that @ultraflux79's backtrace comment references a (paid?) plugin that is calling redirect_canonical(). In that context, is the optional $requested_url parameter being set? Is it only a relative path being passed, like /another-page/, or a full URL? What's happening there may qualify it as related to this issue, or identify a possible plugin issue that should be resolved separately from this ticket.

#29 @bozzmedia
22 months ago

I am encountering the following PHP warnings after moving to PHP 8.0:

PHP Warning:  Undefined array key "path" in /home/redacted/public_html/wp-includes/canonical.php on line 590.

WP 6.0.2
PHP 8.0.23
Linux version 3.10.0-1160.53.1.el7.x86_64 (mockbuild@…) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-44) (GCC) ) #1 SMP
Server version: Apache/2.4.54 (cPanel)
Database Management System MySQL Version 5.7.39
Database Character Set utf8
Database Collation utf8_general_ci


Possibly related errors adjacent to, though not part of this ticket:

PHP Warning:  Undefined array key "comment_post_ID" in /home/redacted/public_html/wp-content/plugins/akismet/class.akismet.php on line 187
PHP Warning:  Undefined array key "comment_post_ID" in /home/redacted/public_html/wp-content/plugins/akismet/class.akismet.php on line 198
PHP Warning:  Undefined array key "comment_post_ID" in /home/redacted/public_html/wp-content/plugins/akismet/class.akismet.php on line 230

PHP Warning:  Undefined array key "comment_author" in /home/redacted/public_html/wp-includes/comment.php on line 2094
PHP Warning:  Undefined array key "comment_author_url" in /home/redacted/public_html/wp-includes/comment.php on line 2112
PHP Warning:  Undefined array key "comment_author_email" in /home/redacted/public_html/wp-includes/comment.php on line 2114
Last edited 22 months ago by bozzmedia (previous) (diff)

#30 @okvee
18 months ago

This warning message occur to me.

PHP Warning: Undefined array key "path" in /.../wp-includes/canonical.php on line 701

PHP 8.0
WordPress 6.1.1

#31 @jstepak
5 months ago

Starting on Feb 9, 2024, we're seeing the following warning message over 20 times per day:

[11-Feb-2024 18:20:58 UTC] PHP Warning: Undefined array key "path" in /mysite/wp-includes/canonical.php on line 728

The code in canonical.php at line 728 is:

$compare_original = array( $originalhost?, $originalpath? );

Our site is on WP 6.4.3 and PHP 8.0.30

If you have an ideas about what is causing this error or how to narrow down the source, I'd appreciate learning more.

Thank you.

Note: See TracTickets for help on using tickets.