Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#39778 closed enhancement (fixed)

Introduce helper function to validate UUID V4

Reported by: jonathanbardo's profile jonathanbardo Owned by: westonruter's profile westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.8
Component: Customize Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

WordPress 4.7.0 introduced a new helper function that would help with the creation of UUID V4 for customizer transactions but never exposed a way to quickly validate UUID V4 without manually copying over the regex found in class-wp-customize-manager.php. I propose we should introduce an helper function to validate UUID so plugins can quickly generate and validate UUID v4.

Attachments (6)

39778.diff (1.1 KB) - added by jonathanbardo 8 years ago.
unit-tests.39778.diff (2.8 KB) - added by jonathanbardo 8 years ago.
generic.39778.diff (3.5 KB) - added by jonathanbardo 8 years ago.
generic.39778.2.diff (3.4 KB) - added by jonathanbardo 8 years ago.
39778.3.diff (3.5 KB) - added by westonruter 8 years ago.
Refreshed patch
39778.4.diff (3.6 KB) - added by westonruter 8 years ago.
https://github.com/xwp/wordpress-develop/pull/259

Download all attachments as: .zip

Change History (18)

@jonathanbardo
8 years ago

#1 @jonathanbardo
8 years ago

  • Keywords has-patch added
  • Summary changed from Introduce wp_validate_uuid4 to Introduce wp_is_valid_uuid4

#2 @jonathanbardo
8 years ago

  • Summary changed from Introduce wp_is_valid_uuid4 to Introduce helper function to validate UUID V4

#3 @westonruter
8 years ago

  • Keywords needs-unit-tests needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 4.8

Yeah, I opted to not introduce a new global function because WP has enough globals. But I was probably being overly conservative.

Needs unit tests, good to test with wp_generate_uuid4, that the function returns a valid UUID4.

The Tests_Functions::test_wp_generate_uuid4() test then also should be refactored similarly.

#4 @jonathanbardo
8 years ago

It's true that WordPress does have a lot of global functions! I just recently started using that function in a plugin and was going to duplicate the regex myself and I thought that it would have been useful to have a validating function in core since we have a generating function.

Something I noticed while writing the unit test is that the regex used in class-wp-customize-manager.php wasn't the same as in the functions.php unit test and was way more permissive. Any reason for that @westonruter ? I think the regex that was in the unit test is the right one for UUID v4.

#5 @westonruter
8 years ago

I'm trying to recall. Either the the manager was just a generic UUID regex I found, or it was also intended to allow for a system to generate a UUID without it necessarily having to be a UUID4.

#6 @jonathanbardo
8 years ago

@westonruter Latest patch will only enforce validating a UUID V4 if the version parameter is provided and set to 4 and will be checking if we are dealing with a general UUID string otherwise.

#7 @jbpaul17
8 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

This needs a review and an owner if its going to land in 4.8.

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


8 years ago

#9 @westonruter
8 years ago

  • Milestone changed from 4.8 to 4.8.1

#10 @westonruter
8 years ago

  • Milestone changed from 4.8.1 to 4.9

@westonruter
8 years ago

Refreshed patch

#11 @westonruter
8 years ago

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

In 39778.4.diff I decided to rename it to just wp_is_uuid().

Other minor changes to generic.39778.2.diff can be seen here: https://github.com/xwp/wordpress-develop/pull/259/files/8918834..3356962

#12 @westonruter
8 years ago

  • Owner set to westonruter
  • Resolution set to fixed
  • Status changed from new to closed

In 41388:

Customize: Add wp_is_uuid() validation function with optional second $version=4 parameter to enforce v4 random UUIDs.

Props jonathanbardo.
Fixes #39778.

Note: See TracTickets for help on using tickets.