WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#39295 new enhancement

Prevent infinite loop when calling get_user_locale() in a 'locale' filter

Reported by: yoavf Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 4.7
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

Using get_user_locale() in a locale filter (for get_locale()) might sound like a good idea, for example to change the site locale depending on our user.

<?php
        if ( $condition) {
                add_filter( 'locale', 'get_user_locale' );
        }

However, if one does that and a non-logged in user (or a user without a locale value) visits the site, a infinite loop will be triggered, since get_user_locale() will itself call get_locale() when no user->locale value exists.

Since this only affects logged out users, it feels like an easy trap to miss, and we should prevent that.

Attachments (3)

39295.diff (1.4 KB) - added by yoavf 4 years ago.
39295.2.diff (1.7 KB) - added by SergeyBiryukov 2 months ago.
39295.2.alt.diff (1.8 KB) - added by SergeyBiryukov 2 months ago.

Download all attachments as: .zip

Change History (6)

@yoavf
4 years ago

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


3 months ago

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


2 months ago

#3 @SergeyBiryukov
2 months ago

39295.2.diff is a refresh that follows the approach of 39295.diff with some improvements:

  • Simplified the logic for consistency with get_locale().
  • Moved the filter name out of the translatable string.
  • The test performs the assertions after restoring the current locale, to avoid affecting other tests in case of failure.

39295.2.alt.diff is an alternative patch that just makes add_filter( 'locale', 'get_user_locale' ) work as expected, without the _doing_it_wrong() message, by using the existing $locale global as a fallback to avoid the infinite loop. Perhaps that could be the preferred fix here?

Last edited 2 months ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.