Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#52932 closed defect (bug) (fixed)

Rest Api enum validation does not work correctly WordPress 5.7

Reported by: stefanjoebstl's profile stefanjoebstl Owned by: yakimun's profile yakimun
Milestone: 5.7.1 Priority: normal
Severity: normal Version: 5.7
Component: REST API Keywords: has-patch has-unit-tests commit fixed-major
Focuses: rest-api Cc:

Description

The following code produces an error when sending a json request with 1, 2 or 3. If I don't use the enum validation it works fine...

<?php
  register_rest_route($this->namespace, '/someendpoint', array(
    array(
      'methods' => WP_REST_Server::CREATABLE,
      'callback' => array($this, 'createEndpoint'),
      'args' => array(
        'gender' => array(
          'type' => 'number',
          'enum' => array(
            1,
            2,
            3,
          ), 
        ),
      ),
    ),
));

The error says: gender isn't one of 1, 2 or 3.

Kind regards
Stefan

Change History (13)

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.7.1

Hi there, welcome to WordPress Trac! Thanks for the report.

Moving to 5.7.1 for investigation.

#2 @TimothyBlynJacobs
3 years ago

Thanks for the report @stefanjoebstl.

The issue here is that you are using a number type with integer values for the enum cases. This causes the incoming value to be sanitized to 1.0. The results are then compared strictly and 1.0 === 1 will fail in PHP.

You can fix this by specifying your type as an integer.

I think, though, we should allow a number with a zero fractional part to be equivalent to an integer of the same value. @yakimun do you have any thoughts on this?

See #51911.

#3 @stefanjoebstl
3 years ago

Thanks for the really fast answer @TimothyBlynJacobs ,

with integer it works as expected. But maybe you should consider the number with zero fractional part to work again, because in earlier versions it also worked if I used the number with the integer enum.

#4 @TimothyBlynJacobs
3 years ago

Yep, definitely. To clarify what changed, previously a strict comparison was also used, but the incoming value wasn't sanitized before checking that the enum value matched.

#5 @yakimun
3 years ago

@TimothyBlynJacobs Hi!

I think that it would be correct to consider such values equal.

Some test cases from the official "JSON Schema Test Suite" confirm this.
Example: https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft2020-12/enum.json#L215

I can fix it if you don't mind.

#6 @TimothyBlynJacobs
3 years ago

  • Owner set to yakimun
  • Status changed from new to assigned

That'd be great, thanks @yakimun!

This ticket was mentioned in PR #1135 on WordPress/wordpress-develop by yakimun.


3 years ago
#7

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

With this fix, if rest_are_values_equal compares two values of type int or float (in any combination), we first cast both of them to float and then compare.

Trac ticket: https://core.trac.wordpress.org/ticket/52932

#8 @rachelbaker
3 years ago

  • Keywords commit added

The patch provided looks solid to me. Marking it for commit for 5.7.1.

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


3 years ago

#10 @SergeyBiryukov
3 years ago

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

In 50653:

REST API: Correct enum validation for numeric values.

When validating enum values as integer or number, consider a number with a zero fractional part to be equivalent to an integer of the same value.

In rest_are_values_equal(), when comparing two values of type int or float (in any combination), first cast both of them to float and then compare.

This matches some test cases from the official JSON Schema test suite.

Follow-up to [50010].

Props yakimun, stefanjoebstl, TimothyBlynJacobs, rachelbaker.
Fixes #52932.

#11 @SergeyBiryukov
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.7 branch.

#12 @SergeyBiryukov
3 years ago

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

In 50656:

REST API: Correct enum validation for numeric values.

When validating enum values as integer or number, consider a number with a zero fractional part to be equivalent to an integer of the same value.

In rest_are_values_equal(), when comparing two values of type int or float (in any combination), first cast both of them to float and then compare.

This matches some test cases from the official JSON Schema test suite.

Follow-up to [50010].

Props yakimun, stefanjoebstl, TimothyBlynJacobs, rachelbaker.
Merges [50653] to the 5.7 branch.
Fixes #52932.

Note: See TracTickets for help on using tickets.