Opened 5 years ago
Last modified 3 years ago
#49277 assigned enhancement
Implement email sanitize in REST API
Reported by: | spacedmonkey | Owned by: | spacedmonkey |
---|---|---|---|
Milestone: | Future Release | 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)
Change History (16)
#2
@
5 years 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.
#4
@
5 years ago
Does that line actually get executed? It looks like it prefers regex which I think needs to be available to run WP?
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
.
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
5 years ago
#7
@
5 years 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
This ticket was mentioned in PR #786 on WordPress/wordpress-develop by lukaspawlik.
4 years ago
#8
Trac ticket: https://core.trac.wordpress.org/ticket/49277
@TimothyBJacobs @spacedmonkey this is the refreshed work of patch attached into Trac ticket. Based on your discussion I've ported behaviour of filter_var
into PHP code. Additionally I've added an extra check to avoid sanitization of email when is_email
function returns true what is most likely an indicator that value is recognized correctly and doesn't need to be sanitized.
Please let me know if you have any questions.
#9
@
4 years ago
@TimothyBlynJacobs I think the provided patch seems good. Any chance of getting this in 5.8?
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
3 years ago
3 years ago
#13
I see a few characters that don't fit my mental model of an email address, e.g. {
and }
. Is the regex accurate? Also, should the same logic be used for validation?
Is this stricter than
sanitize_text_field
? If so, I think we'd want to make sure it isn't stricter thanis_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.