WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 weeks ago

#39133 new enhancement

Check email length in is_email()

Reported by: PranaliPatel Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

In reference to this ticket - https://core.trac.wordpress.org/ticket/38506#comment:12

It would be better to check email length within the is_email() itself instead of
validating it at various other places.

Attachments (5)

39133.diff (458 bytes) - added by PranaliPatel 4 years ago.
Check email length ( to minimum of 5 )
39133-1.diff (731 bytes) - added by PranaliPatel 4 years ago.
As we are checking validation in is_email(), we can remove the string length check from the wp_handle_comment_submission().
39133-2.diff (551 bytes) - added by milindmore22 4 years ago.
Removed email length check from rest-api as we are checking length in is_email() function already
39133-11.diff (1.1 KB) - added by ashishtv 4 years ago.
Unit test
39133.5.diff (855 bytes) - added by donmhico 2 weeks ago.

Download all attachments as: .zip

Change History (13)

@PranaliPatel
4 years ago

Check email length ( to minimum of 5 )

#1 @PranaliPatel
4 years ago

  • Keywords has-patch added

@PranaliPatel
4 years ago

As we are checking validation in is_email(), we can remove the string length check from the wp_handle_comment_submission().

#2 @swissspidy
4 years ago

  • Keywords needs-unit-tests added

#3 @swissspidy
4 years ago

#39136 was marked as a duplicate.

@milindmore22
4 years ago

Removed email length check from rest-api as we are checking length in is_email() function already

@ashishtv
4 years ago

Unit test

#4 @milindmore22
4 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#5 @swissspidy
4 years ago

There should be a dedicated test for the new length test in is_email() (see Tests_Formatting_IsEmail), not just one handling comments. Also, since one patch changes code in the REST API, that part should get coverage as well.

#6 @andraganescu
9 months ago

  • Keywords needs-unit-tests added; has-unit-tests removed

Just re-adding the needs unit tests label back.

#7 @donmhico
2 weeks ago

I added a patch via GitHub PR but for some reason it's not showing here. https://github.com/WordPress/wordpress-develop/pull/413

Here's the patch - https://github.com/WordPress/wordpress-develop/pull/413.patch

I'll attach it as patch here as well.

@donmhico
2 weeks ago

#8 @donmhico
2 weeks ago

  • Keywords has-unit-tests added; needs-unit-tests removed
Note: See TracTickets for help on using tickets.