WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 6 months ago

#49277 new enhancement

Implement email sanitize in REST API

Reported by: spacedmonkey Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch 2nd-opinion has-unit-tests
Focuses: Cc:

Description

Implement email sanitize in REST API over just using sanitize_text_field

Attachments (1)

49277.diff (2.5 KB) - added by spacedmonkey 8 months ago.

Download all attachments as: .zip

Change History (8)

@spacedmonkey
8 months ago

#1 @TimothyBlynJacobs
8 months ago

  • Keywords 2nd-opinion added

Is this stricter than sanitize_text_field? If so, I think we'd want to make sure it isn't stricter than is_email would allow, right?

Separately, as I understand it, filter_var historically hasn't been used in WordPress since I think it can be disabled and we don't list it as a required extension.

Cc: @SergeyBiryukov, @jrf.

#2 @spacedmonkey
8 months ago

filter_var is used elsewhere in core.

sanitize_text_field is applied to emails first then filter_var is run. The idea is to just remove character that are not valid in an email. It doesn't do a lot of validation that is_email, checking domain length etc.

#3 @spacedmonkey
8 months ago

  • Keywords has-unit-tests added

#4 @TimothyBlynJacobs
8 months ago

Does that line actually get executed? It looks like it prefers regex which I think needs to be available to run WP?

Related #28170, #16867

The idea is to just remove character that are not valid in an email.

Right, what I'm trying to figure out, is if there is a case where the sanitization would not allow an email that would have previously been allowed by is_email(). I understand filter_var doesn't do the complex checking is_email does. But I'm wondering if any of the characters it strips would've been allowed by is_email.

#5 @spacedmonkey
8 months ago

I am not locked to using filter_var. Regex will work just as well.

This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.


6 months ago

#7 @kadamwhite
6 months ago

@SergeyBiryukov After discussion in the REST API weekly meeting we'd love to get your eyes on this briefly, it'd make us more confidant in the solution

Note: See TracTickets for help on using tickets.