Add class_exists() check before defining the PasswordHash class

[DavidAnderson]

I have a project that calls wp-load.php, but after already loading the PasswordHash class. (It authenticates the user before loading WP).

After the update from WP 4.6.1 to WP 4.7, this now causes a PHP fatal error, because now wp-settings.php unconditionally calls require().

The attached patch will not define the PasswordHash class if it already exists.

[DavidAnderson]

Do not define the PasswordHash class if it already exists

[dd32]

We should probably just revert [38371] here honestly. I can't see any benefits in loading the class on every pageload when it's not needed.

Failing that, switching it back to a require_once().

I'm milestoning this for 4.7.x for review.

[dd32]

Alternative: Revert [38371] in trunk/4.8, switch to require_once() in wp-settings.php for 4.7 to avoid extra file churn in the minor, but also avoid this scenario.

The con of changing this back, and even more of delaying it until 4.8, is that if anyone who between 4.7.0 and $release who expects the class to exist will hit a fatal. I can't see a reason for most users to need it.

[jbpaul17]

Per today's bug scrub in #core: we're going to punt this to 4.7.2 given that there's no agreement on approach. @aaroncampbell will check with @dd32 on this later today in case an agreement on approach is made quickly and this then lands for 4.7.1.

[aaroncampbell]

Replying to dd32:

Alternative: Revert [38371] in trunk/4.8, switch to require_once() in wp-settings.php for 4.7 to avoid extra file churn in the minor, but also avoid this scenario.

The con of changing this back, and even more of delaying it until 4.8, is that if anyone who between 4.7.0 and $release who expects the class to exist will hit a fatal. I can't see a reason for most users to need it.

I'm with @dd32 here (even though I understand the desire to clean things up like this). Unless @wonderboymusic has reasoning for this beyond "cleaner files", I'd say we should just put it back how it was. I get that it was required in a handful of places, but that's because it was only used in a handful of places.

[wonderboymusic]

I don't think we should do anything to support this use case, because calling wp-load.php in the wild means every single class in the codebase would require this same treatment. However, I understand the desire to revert to mitigate breakage. I don't see WordPress moving in the direction my original change intended, so the original change becomes mostly unnecessary.

[jnylen0]

It looks like there is mostly a consensus here to fix this by reverting [38371]. If someone wants to handle this for 4.7.3, ideally today, feel free to move back into the 4.7.3 milestone.

[ketuchetan]

reverting [38371]

[ketuchetan]

As per above discussion, I have reverted the [38371] in my patch. please let me know if any changes on this or if I am wrong anywhere.

[jnylen0]

@ketuchetan there is still a require line in wp-settings.php that needs to be removed. Also, your patch removes a blank line in wp-includes/post-template.php that was not introduced by [38371].

[ketuchetan]

Made the changes as per the @jnylen0 suggestions.

[swissspidy]

39445-alternative.diff​ is an alternative patch for 4.7 to use require_once(). As suggested earlier, we could do a full revert in 4.8 and this smaller change in the minor release.

[ocean90]

Replying to dd32:

Alternative: Revert [38371] in trunk/4.8, switch to require_once() in wp-settings.php for 4.7 to avoid extra file churn in the minor, but also avoid this scenario.

+1 for this approach.

[swissspidy]

In 40387:

Load: Only load PasswordHash class when needed.

This reverts [38371] which loaded class-phpass.php early in wp-settings.php and in turn caused backward compatibility problems.

Props DavidAnderson, ketuchetan.
Fixes #39445.

[swissspidy]

In 40389:

Bootstrap/Load: Only load PasswordHash class once.

require_once prevents errors when loading WordPress and the class already exists.

See [40387].
Fixes #39445.