![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#39042 closed defect (bug) (fixed)
REST API: Allow sanitization_callback to be set to null to bypass `rest_parse_request_arg()`
| Reported by: | rachelbaker | Owned by: | kkoppenhaver |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.7.1 |
| Component: | REST API | Version: | 4.7 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
In #38593 we use the default callback for a property type if it is set, but you cannot override this behavior.
As an example, if you have a property schema like:
'some_email' => array( 'description' => __( 'Email address for ...' ), 'type' => 'string', 'format' => 'email', 'arg_options' => array( 'sanitize_callback' => null, // SHOULD skip built-in saniziation of 'email' type. 'validate_callback' => 'custom_callback', ), ),
The logic in WP_REST_Request->sanitize_params() that was added in [39091] does not account for null being the sanitization_callback which then results in rest_parse_request_arg() being set to the callback, which runs both default sanitization and validation functions.
Attachments (3)
Change History (16)
#5
@
10 years ago
Thanks for the patch @kkoppenhaver.
I wish there was a better way to structure this logic, so we didn't need to nest the ! array_key_exists() conditional, but I didn't see an obvious way around it.
#7
@
10 years ago
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
39042.2.diff adds unit tests for null and false sanitization_callback values.
#8
@
10 years ago
@joehoyle would like your eyes on 39042.2.diff, would love a better approach than the nested conditional.
#9
@
10 years ago
This looks good to me, I had incorrectly assumed isset was going to fail on null, but I guess that's not the case.
#10
@
10 years ago
In 39042.3.diff:
- Restructure this code block a bit more to get rid of the nested conditional and shorten up some long lines
- One assertion per test (separate tests for
nullandfalse)
I also removed the @ticket annotation from the tests as I don't think it adds much value: I'm not sure why you'd need this information, but if you do, you can find it via blame.
One way we can check if
sanitization_callbackisnullwould be to add a check forarray_key_exists( 'sanitize_callback', $attributes['args'][ $key ] ).We should also add a unit test as @jnylen0 suggested in the original ticket: https://core.trac.wordpress.org/ticket/38593#comment:4