WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 2 weeks ago

#49145 reviewing defect (bug)

Undefined index: host in/wp-includes/l10n.php on line 989, suggested fix

Reported by: SeBsZ Owned by: SergeyBiryukov
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.3.2
Component: I18N Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

We're using WordPress 5.3.2 and I'm getting a lot of notices: "Notice: Undefined index: host" in l10n.php on line 989:

} elseif ( ! isset( $src_url['host'] ) || $src_url['host'] === $site_url['host'] ) {

The cause in our case, is that $site_url does not have a 'host' index. I'd like to suggest that instead of only checking whether
$src_url['host'] is set, also check whether $site_url['host'] is set.

We have a filter on site_url() which makes all URLs relative. This is what causes 'host' not to be set. Regardless of the cause though, I don't think l10n.php should generate a notice in this case.

Attachments (1)

49145.diff (1.6 KB) - added by SergeyBiryukov 10 months ago.

Download all attachments as: .zip

Change History (18)

#1 @SergeyBiryukov
10 months ago

  • Component changed from General to I18N
  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#2 @archon810
10 months ago

  • Component changed from I18N to General

#46387 seems similar.

Last edited 10 months ago by SergeyBiryukov (previous) (diff)

#3 @archon810
10 months ago

  • Component changed from General to I18N

#4 @archon810
10 months ago

@SergeyBiryukov Think you can target 5.4 for this one? It's the #1 issue that keeps pinging our Sentry by a huge margin.

#5 @SergeyBiryukov
10 months ago

  • Keywords has-patch added

#6 @SeBsZ
10 months ago

I've tested the patch, and can confirm the PHP Notice is gone. I can't vouch for the correctness of the implementation though, as I didn't look into the inner workings. Thanks for the quick response @SergeyBiryukov, looking forward to seeing this released.

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


8 months ago

#8 @SergeyBiryukov
8 months ago

  • Keywords commit added

#9 @ocean90
8 months ago

  • Keywords needs-unit-tests added; commit removed

#10 @SergeyBiryukov
8 months ago

  • Milestone changed from 5.4 to 5.5

#11 @SergeyBiryukov
3 months ago

  • Milestone changed from 5.5 to 5.6

This still needs tests, and I'm not quite sure the patch is correct as is.

Moving to 5.6 for a closer look.

#12 @SergeyBiryukov
8 weeks ago

#51172 was marked as a duplicate.

#13 @SergeyBiryukov
6 weeks ago

#51286 was marked as a duplicate.

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


5 weeks ago

#15 follow-up: @justinahinon
5 weeks ago

The initial patch proposed by Sergey still needs a look.
@nourma proposed to work on it during yesterday's i18n bug scrub.

And I can write the unit tests when ready.

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


3 weeks ago

#17 in reply to: ↑ 15 @nourma
2 weeks ago

Replying to justinahinon:

The initial patch proposed by Sergey still needs a look.
@nourma proposed to work on it during yesterday's i18n bug scrub.

And I can write the unit tests when ready.

I am unable to duplicate the undefined index in l10n.php. When I use wp_make_link_relative to filter site_url to make URLs relative, I get undefined indexes in theme.php(lines 3479 & 3544), class-wp-customize-manager.php(line 4598), and http.php(lines 421 and 422). In each case, a function is trying to access $admin_origin['host'], which has no host index. On a side note, I was able to successfully apply the patch 49145.diff (but obviously was unable to test that it actually solved the problem, since I was unable to replicate the error in l10n.php). I applied the patch on the latest build WordPress 5.6.

Note: See TracTickets for help on using tickets.