Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#45595 closed defect (bug) (fixed)

Filter 'sanitize_email' provides 2 times the same param

Reported by: chrico's profile ChriCo Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.1 Priority: normal
Severity: normal Version: 5.0
Component: Formatting Keywords: has-patch
Focuses: Cc:


The function sanitize_email() in wp-includes/formatting.php L3447 provides a filter which allows to hook into the sanitize-function with a given context. The docBlock for this filter is saying following:

* @param string $email   The sanitized email address.
* @param string $email   The email address, as provided to sanitize_email().
* @param string $message A message to pass to the user.

Following errors are present here:

duplicated var

The first and second param are the same var, while the description says something completly different.

(maybe) wrong assigment

On top of that, if the sanitization is successful, the $email is build together again before returning it by assigning it via: $email = $local . '@' . $domain;.

This means, that the either the documentation is wrong, or the $email before last return is wrongly assigned.

wrong type hint

string $message is not correct, since the last applied filter uses null. This should be either '', a message or the docBlock has to be updated. Since we propably have to deal with "BC" and some users are already checking this filter with if($message === null), i'll just update the docBlock.

My patch contains following:

  1. Rename docblock first $email to $sanitized_email.
  2. Update docBlock for string|null $message.
  3. Change assigment before return to $sanitized_email = $local . '@' . $domain;.
  4. Inject sanitized_email first param after hook name.

Attachments (1)

45595.patch (1.3 KB) - added by ChriCo 5 years ago.

Download all attachments as: .zip

Change History (3)

5 years ago

#1 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#2 @pento
5 years ago

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

In 44620:

Docs: Improve the sanitize_email filter.

  • Remove duplicate parameter names.
  • Fix the wrong value being passed in the second parameter.
  • Correct the type of the $message parameter.

Props ChriCo.
Fixes #45595.

Note: See TracTickets for help on using tickets.