WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 2 months ago

#43559 reviewing enhancement

wp_ajax_add_meta() does not allow empty values

Reported by: charlestonsw Owned by: SergeyBiryukov
Milestone: 5.0 Priority: normal
Severity: normal Version: trunk
Component: Options, Meta APIs Keywords: has-unit-tests has-patch
Focuses: rest-api Cc:

Description

Users should be allowed to set post meta keys to an empty string.

With custom post types there are many ways to use the custom meta data. In one of my use cases I use the meta values to drive a remote application. The REST API sends back a JSON response. If a key is present in the response it sets the corresponding value in the remote app.

Having the ability to send back a key with an EMPTY STRING value is critical for writing simple reactive applications (Vue , React, etc.) that call upon the WordPress REST API without having to craft a completely custom solution.

I am trying to use a standard WP REST endpoint to fetch a custom post type. Without the ability to set a custom post type "custom field" to an empty string I am going to have to write a custom post type so a user can enter something crazy like   into that field , catch that, and then convert it to before sending to the remote app. That is a bad hack -- when doing HTML/CSS values back to the remote app a user might actually WANT   in the output.

Why are values being rejected in WP Posts custom meta in the first place?

IMO this should be removed from \wp_ajax_add_meta() in wp-admin/includes/ajax-actions.php

		if ( '' == trim($key) )
			wp_die( __( 'Please provide a custom field name.' ) );

Or at least add a filter/hook so this default behavior can be mitigated:

		if ( apply_filters( 'wp_reject_meta_values' , '' , $pid ) == trim($key) )
			wp_die( __( 'Please provide a custom field name.' ) );

Even better - allow users to provide an array of values to reject:

                $rejected_values = (array) apply_filters( 'wp_reject_meta_values' , '' , $pid );
		if ( in_array( trim( $key ) , $rejected_values )  )
			wp_die( __( 'Please provide a custom field name.' ) );

Attachments (1)

43559.diff (3.6 KB) - added by soulseekah 3 months ago.
Tests + implementation

Download all attachments as: .zip

Change History (10)

#1 @soulseekah
3 months ago

Thank you for opening this issue.

I think you're mixing up keys and values. Meta keys cannot be empty, and shouldn't be. Meta values can be empty all you want.

Can you please provide a fuller example of what you're trying to do via REST? What's the call?

#2 @SergeyBiryukov
3 months ago

  • Component changed from Posts, Post Types to Options, Meta APIs
  • Focuses rest-api added
  • Keywords reporter-feedback added

#3 @charlestonsw
3 months ago

Sorry - late night coding.

Same thing happens for values... copied the wrong code block.

		if ( '' == trim($value) )
			wp_die( __( 'Please provide a custom field value.' ) );

To reproduce:

Create a custom post type (probably not necessary based on the code from WP Core). Add Custom Field. Enter a key, "test". Leave value empty. Save.

You'll get the error above in the notification box.

So you have no way to add a custom field with a blank value.

I have a JS listener that does a standard WP REST call to fetch the post with full meta. I use this to remotely configure plugins, so if a setting for a text field, call it "Search Box Title" should be set to BLANK I cannot do this. I am now sending   and in the listener I then have to do if $json_data[ 'search_box_title' ] === ' ' $json_data['search_box_title']='';

Not I cannot set my "custom_html' setting to  

Granted that is unlikely to happen in my particular instance but this is a notable shortcoming in the custom fields/post meta.

I'd much rather use core REST functionality and send back a standard response and consume the meta fields with a reactive app than have to use custom settings and document for my clients "well, if you really want to set that value to blank enter   as the value in the field". Only code geeks will even know why I used that when really I wanted them to be able to turn the search box title OFF completely setting it from the default 'Enter Your Search Parameters' to and skipping the header rendering entirely.

Same premise as above, swap the word 'key' with 'value'.

Last edited 2 months ago by SergeyBiryukov (previous) (diff)

@soulseekah
3 months ago

Tests + implementation

#4 @soulseekah
3 months ago

  • Keywords has-unit-tests has-patch added; reporter-feedback removed

Apparently, historically the Meta API did not allow empty values to be saved (see #11650).

The current version of WordPress, however, does allow empty values to be saved so with a couple of fixes here and there, empty values can now be saved. Attached patch with tests. Patch lifts the limitations imposed by both add_meta and wp_ajax_add_meta.

P.S. It's still unclear why REST has anything to do with it, though. Care to clarify?

Special thanks to @versusbassz for helping on the patch.

Last edited 3 months ago by soulseekah (previous) (diff)

#5 @charlestonsw
3 months ago

REST is just one example use case.

The primary issue is not being able to save empty values on custom fields.

There are hundreds of use cases where empty values on custom fields would be desirable. A reactive app built on top of the standard WP REST API is one of many. I don't think I tagged this originally as a rest-api issue as that was not the primary issue being reported.

As for the current version of WordPress allowing empty meta values -- that may be the theory but that is not how the WordPress admin interfaces are working for pages/posts today (production or 5.0-alpha builds).

#6 @soulseekah
3 months ago

Reading the REST API code, I'm pretty sure there are no limitations on the meta value being empty. The REST API code uses the Meta API to store the values, which gladly accepts empty values. The schema validation does not appear to have any sort of limitations on the value.

If you have the time, please investigate the related REST API issue in a separate issue (and link it here, if possible). This issue is for add_meta and wp_ajax_add_meta (i.e. stuff being done in the admin).

Other than that, please feel free to try out our patches. Cheers.

#7 @charlestonsw
3 months ago

I will try the patches -- I"m sure they'll be fine as I've modified WP Core myself on my VVV box to test the theory behind the original request.

The question is will these patches make into core someday? My app is working fine , I wrote my own workaround so my users can work with the current limitations of WordPress as per the full use case here:

The full use case --

1a) Site admin logs into the main services site. 1b ) Open a custom post type from the WP Admin page. 1c) Add a custom field "box_title". 1d) Save the custom field (leaving value as blank) -- PROCESS FAILS per WP Core restriction.

2a) An end user consumes the meta from a remote app via the REST API. 2b) The app does not set the text inside a HTML element per the Vue app to blank because the post has not matching key (see 1b above).

Could I build a new app that uses the REST API to force meta onto the original post and set it to blank, sure can. But why would I go about writing a web app to replicate the base WordPress app with a new admin UX just so they can set a custom field value to blank? I could write an entire app to replace WordPress admin and front end UX -- possibly even using the REST API much like Calpyso. But why?

#8 @soulseekah
3 months ago

Understood. Thanks for your clarifications.

The patch and tests that I wrote should address both cases. It should make it into core, 5.0 very probably, so stay tuned ;)

Cheers.

#9 @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.