Warning in wp_salt() since 6.7

[juliobox]

Hey there
This is the error I get:
Warning: Undefined array key "..." in /wp-includes/pluggable.php on line 2517
with the "..." being my salt key, got this messages for each key.

How to trigger:

1. Move your salt keys to a mu-plugin
2. add a simple plugin doing this:

add_filter( 'gettext', function( $translation ) {
get_user_locale();
return $translation;
});

What's missing in the new code adde in wp_salt:
if ( ! defined( $const ) | |true === $duplicated_keys[ constant( $const ) ] ) {
here we check true === but not if isset. we should right?

[ankitkumarshah]

Hi @juliobox,

Thank you for bringing this up. I was able to reproduce the issue, and I agree that adding an additional check would help address it. I'm currently working on a fix for this.

Trac Ticket: core 62424

### Description
Enhances the validation logic for duplicated keys by adding proper existence checks before accessing array values. This prevents potential PHP notices/warnings for undefined array indices.

### Changes

• Added isset() check before accessing $duplicated_keys array values
• Maintains strict comparison (===) with boolean true
• Preserves original conditional logic while making it more robust

[ankitkumarshah]

Here is the screenshot before and after adding the patch:
​https://postimg.cc/9RxJHgJs

[desrosj]

Introduced in [58837] (see #59871). Adding to 6.7.1 to evaluate.

[dilipbheda]

## Test Report
### Description
This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/7801

### Environment

• WordPress: 6.8-alpha-59274-src
• PHP: 8.2.25
• Server: nginx/1.27.2
• Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.25)
• Browser: Chrome 130.0.0.0
• OS: Linux
• Theme: Twenty Twenty-Five 1.0
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

### Actual Results

1. ✅ Issue resolved with patch.

### Additional Notes

• Fixed warning after applying patch

[desrosj]

The PR does fix the issue, but I'm trying to narrow down exactly why the $duplicated_keys array is not properly populated.

wp_salts() is called multiple times (5 when logged in viewing the home page on a default install), and the keys are only missing the first time. We should understand why and be confident this is not a problem before protecting against the warning.

This adds a check to see if wp_salt() related options have already been primed before attempting to prime them again. This is necessary because the wp_salt() function can be called multiple times during the bootstrap process, and we need to ensure that the options are only primed once.

This also avoids an error that can occur if the salt related constants are defined after the first time wp_salt() is called, in which case the salt will not be present in the $duplicated_keys array.

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

[desrosj]

This seems to be caused by the fact that get_user_locale() is called within the gettext filter. Removing that function silences the warnings.

In my testing, it also does not matter where the constants are defined (wp-config.php, MU plugin, etc.). It is only important that they are defined and salts stored in the options table are not being used.

The problem is that there is one string translated within wp_salts(), but it's only translated the first time the function runs when $duplicate_keys is null. So the order goes something like this:

• wp_salts() called first time.
• the static $duplicated_keys is null, so that conditional statement is true.
• The first thing that block of code does is sets $duplicated_keys to an array containing the default put your unique phrase here value.
• Then that phrase is passed through __() in case the config file is localized.
• The provided example gettext filter is run, and get_user_locale() eventually calls wp_salt() again while determining who the current user is.
• When the null === $duplicate_keys condition is encountered, it's no longer true because it's static and now set to the default.
• The function continues, and the condition throws the warnings because the constant is in fact set, it's just not populated into $duplicate_keys as expected.
• Eventually, the initial call to the function completes and the static variable is fully populated with all of the defined constants. All subsequent wp_salt() calls work as expected without throwing warnings.

I'm now of the opinion that an isset() check is not the correct change to make here as this new code added in [58837] is clearly surfacing an edge case that was not previously exposed.

One way to fix this issue is to establish the current user before wp_salt() is called. There may be better hooks to use, but this worked in my testing:

add_filter( 'setup_theme', function() {
	wp_get_current_user();
});

I'm not familiar enough with how gettext is used in the wild for filtering translations. Could you share more context around your use case @juliobox?

[joemcgill]

While reviewing this with @desrosj, my hunch was that this error condition was being triggered only if the salts are defined AFTER the first time that wp_salt() is called. In this scenario, the foreach loop that sets the $duplicated_keys values would not include the salts and all of the options will be primed, and then the second time wp_salt() is called, the constants would be defined but not in the $duplicated_keys array. To avoid such a scenario, we could add a check to make sure that the options only get primed once, which I put together ​a quick PR example to demonstrate.

However when testing this with the reproduction steps from the ticket, that scenario is not the cause of this bug, but instead it's due to another condition where this can happen, which this would not address.

While creating the $duplicated_keys array, gettext is ​called here to translate the 'put your unique phrase here' key. By adding a call to get_user_locale() in the gettext filter, it causes wp_salt() to be recursively called from before it has completed running the first time, but after $duplicated_keys is no longer null, so the recursive call skips filling out the $duplicate_keys array. This always causes the options to be primed, even when they likely don't need to be. We should probably try to avoid this scenario as well if possible.

[johnbillion]

I wonder if moving the translation for put your unique phrase here so it runs after the population of the rest of the array elements would be enough? I've opened ​https://github.com/WordPress/wordpress-develop/pull/7843 to find out.

An alternate idea to avoid the edge case where wp_salt() is called recursively on gettext before the $duplicated_keys array has completed being constructed is to move the one call to __() while constructing $duplicated_keys to the end of the array.

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

@johnbillion! Closing in favor of ​https://github.com/WordPress/wordpress-develop/pull/7843

One other thing that I tested. I was wondering if the first recursive call of wp_salt() that occurred would not contain the localized version of the string. With a site localized into Japanese, this would be ここに独自のフレーズを入力します. In my testing, I was unable to confirm this to be the case. It seems like the localized version of the string will be in the array as expected with this PR.

[desrosj]

Did a little digging to see when this was added/changed to be this way. It seems that [54249]/#55937 wrapped the string in __() for WP 6.1, so it's relatively new.

#57121 dealt with a similar edge case where get_current_user_id() cause out of memory errors. [55433] resolved this in 6.2.

I did some testing with the attached pull request, and it fixes the warnings and behaved as expected. This should hopefully catch the remaining edge cases.

[desrosj]

In 59427:

Options, Meta APIs: Ensure duplicate salts are properly flagged.

Improvements were made in 6.7 to ensure that salts stored in the database were primed more efficiently.

The logic added to accomplish this suffered from an edge case where array indexes were unexpectedly missing when wp_salt() was called recursively.

Follow up to [58837].

Props juliobox, ankitkumarshah, dilipbheda, johnbillion, joemcgill, desrosj.
Fixes #62424.

[desrosj]

Reopening for a second approval to backport.

https://core.trac.wordpress.org/changeset/59427

[davidbaumwald]

Looks good to backport to the 6.7 branch.

[desrosj]

In 59434:

Options, Meta APIs: Ensure duplicate salts are properly flagged.

Improvements were made in 6.7 to ensure that salts stored in the database were primed more efficiently.

The logic added to accomplish this suffered from an edge case where array indexes were unexpectedly missing when wp_salt() was called recursively.

Follow up to [58837].

Reviewed by davidbaumwald.
Merges [59427] to the 6.7 branch.

Props juliobox, ankitkumarshah, dilipbheda, johnbillion, joemcgill, desrosj.
Fixes #62424.