Make WordPress Core

Opened 9 months ago

Closed 7 weeks ago

#63147 closed defect (bug) (fixed)

Enhanced verification of $_REQUEST['reauth'] in the authentication process.

Reported by: wplmillet's profile wplmillet Owned by: audrasjb's profile audrasjb
Milestone: 6.9 Priority: low
Severity: normal Version:
Component: Login and Registration Keywords: has-patch commit
Focuses: Cc:

Description

The $_REQUEST['reauth'] check currently uses a more up-to-date syntax:

<?php
$reauth = empty( $_REQUEST['reauth'] ) ? false : true;

This syntax could be updated to improve the code's readability and robustness. Two options are possible:

Use empty() :

<?php
$reauth = !empty($_REQUEST['reauth']);

Use coalescent Null operator :

<?php
$reauth = $_REQUEST['reauth'] ?? false;

Advantages :

  • Better readability.
  • Reduces redundant code.
  • Modern PHP practices.

I'll be using more of the first solution with empty() seems to be better. It avoids the undesirable behavior associated with falsy values (0,'', ).

Impact:
No negative impact expected. No change in behavior and better code readability. and maintenance.

Thank you 🙂

Attachments (2)

fix-reauth-check.patch (373 bytes) - added by wplmillet 9 months ago.
fix-reauth-check-wpcs.patch (376 bytes) - added by wplmillet 9 months ago.
Fix and OK with WPCS

Download all attachments as: .zip

Change History (10)

#1 @audrasjb
9 months ago

  • Keywords needs-testing removed
  • Milestone changed from Awaiting Review to 6.8
  • Owner set to audrasjb
  • Status changed from new to accepted
  • Type changed from enhancement to defect (bug)
  • Version trunk deleted

Hello and thanks for the ticket @wplmillet,

Switching this to a bugfix and milestoning to 6.8.

Please note that the patch doesn't pass WordPress Coding Standards tests because we'll need spaces after and before opening and closing parenthesis, and also between ! and empty() :)

Last edited 9 months ago by audrasjb (previous) (diff)

@wplmillet
9 months ago

Fix and OK with WPCS

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


9 months ago

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


9 months ago

#4 @audrasjb
9 months ago

  • Milestone changed from 6.8 to 6.9

We are a couple hour before 6.8 RC1, moving to 6.9 as this is low priority.

#5 @audrasjb
9 months ago

  • Priority changed from normal to low

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


7 weeks ago

#7 @wildworks
7 weeks ago

  • Keywords commit added

This ticket was brought up in today's 6.9 bug scrub. The patch seems ready to be committed, but what do you think, @audrasjb?

#8 @SergeyBiryukov
7 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 61085:

Coding Standards: Simplify the $_REQUEST['reauth'] check in wp-login.php.

Follow-up to [14556].

Props wplmillet, audrasjb.
Fixes #63147.

Note: See TracTickets for help on using tickets.