Pass raw data into save_post/wp_insert_post

[rmccue]

Almost all tutorials about custom metaboxes (in addition to post meta) use $_POST to get the data that was submitted. Unfortunately, this breaks if a plugin uses anything that calls wp_insert_post() in the same request.

The patch attached here includes the $postarr array, which is the raw data passed in, and which is $_POST from wp-admin/post.php. This allows one to use the third parameter to the callback to set meta data instead of having to use $_POST directly.

[rmccue]

Add $postarr to the save_post and wp_insert_post actions.

[rmccue]

To note, this also makes it much safer, and avoids metaboxes having to use their own nonces (though they probably should), and avoids the need for checking permissions.

[scribu]

Interesting suggestion. I think we should deprecate one of those filters by passing the $postarr arg to only one of them.

[sirzooro]

+1 for this.

BTW, metaboxes needs to check its own nonces (or something similar) because wp_insert_post() is called from many contexts, and many of them does not include metaboxes at all (e.g. quickedit).

[rmccue]

Replying to scribu:

Interesting suggestion. I think we should deprecate one of those filters by passing the $postarr arg to only one of them.

I agree (although, I left the patch with both), and I'd say save_post is the most appropriate, given that it is consistent with the edit_post filter also used in the function.

Replying to sirzooro:

BTW, metaboxes needs to check its own nonces (or something similar) because wp_insert_post() is called from many contexts, and many of them does not include metaboxes at all (e.g. quickedit).

Instead of checking for a nonce, it can simply check for the values it actually uses (i.e. if (empty($postarr['myplugin_value'])) return; ) which is what functions should be doing anyway.

[rmccue]

Example usage:

function rm_savemydata($id, $post, $raw) {
	// Use `isset` here if it can be blank
	if (!empty($raw['rm_myfield'])) {
		return;
	}

	update_post_meta($id, 'rm_myfield', $raw['rm_myfield']);
}
add_action('save_post', 'rm_savemydata', 10, 3);

[scribu]

The only case when that won't work is if you have a single checkbox.

[sirzooro]

Replying to scribu:

The only case when that won't work is if you have a single checkbox.

The same is for radio buttons and selection lists, when nothing is selected.

[scribu]

Yeah. So you wouldn't need a nonce necessarily, just a hidden input.

[dd32]

My only thoughts on this is that this ties us to passing the complete $_POST array in as $postarr. If passing $_POST in is a shortcut, I'd personally prefer not to go down this road, of course, if it already relies upon ($postarr === $_POST) that might be a different case.

[rmccue]

Replying to sirzooro:

The same is for radio buttons and selection lists, when nothing is selected.

Those should still just be equivalent to $_POST['something'] = ''; (i.e. empty value, but still set). I'm not sure for checkboxes, however.

Replying to dd32:

My only thoughts on this is that this ties us to passing the complete $_POST array in as $postarr. If passing $_POST in is a shortcut, I'd personally prefer not to go down this road, of course, if it already relies upon ($postarr === $_POST) that might be a different case.

It is already passed in, as this is where wp_insert_post gets the actual data from. The only change (as noted in the patch) is to add it to the two actions.

[scribu]

Almost all tutorials about custom metaboxes (in addition to post meta) use $_POST to get the data that was submitted. Unfortunately, this breaks if a plugin uses anything that calls wp_insert_post() in the same request.

Why is that?

[rmccue]

Replying to scribu:

Almost all tutorials about custom metaboxes (in addition to post meta) use $_POST to get the data that was submitted. Unfortunately, this breaks if a plugin uses anything that calls wp_insert_post() in the same request.

Why is that?

If a plugin tries to add a post manually (by passing in a data array), all data will be taken from this for wp_insert_post. However, these metabox callbacks will instead get the data from $_POST, and hence, the wrong data will be added.

This patch enables the metaboxes to use the data that is passed in. For a POST request, this is the $_POST data. For a manual insert by a plugin, it will be the data that is passed in.

[dd32]

It is already passed in, as this is where wp_insert_post gets the actual data from. The only change (as noted in the patch) is to add it to the two actions.

Yes, However nothing but the core post fields are used from the array. Passing it to functions like this will introduce a reliance upon having to (forever) pass the complete $_POST array in. Right now for example, it is entirely possible to only pass select fields in (although we don't).

Where this comes up, is from #18322, As part of switching from the slashed to unslashed globals, certain functions which expect slashed data will need some special handling. One suggestion which I have seen floating around (which I personally prefer) would result in only passing select elements through functions like this. I'm thinking of the future use of the functions, not just what's easiest today.

[scribu]

Replying to rmccue:

If a plugin tries to add a post manually (by passing in a data array), all data will be taken from this for wp_insert_post. However, these metabox callbacks will instead get the data from $_POST, and hence, the wrong data will be added.

In other words, the problem is that you can't distinguish between the original call made by Core and other calls.

It's similar to the problem of differentiating the main WP_Query instance from others.

I think what we actually need here is a new action that's fired only after the "Publish/save" button is pushed.

[scribu]

Ideally, we would move the 'save_post' action out of wp_insert_post(), but that's probably not possible, due to back-compat.

[rmccue]

Replying to dd32:

Where this comes up, is from #18322, As part of switching from the slashed to unslashed globals, certain functions which expect slashed data will need some special handling. One suggestion which I have seen floating around (which I personally prefer) would result in only passing select elements through functions like this. I'm thinking of the future use of the functions, not just what's easiest today.

In which case, the fields that are passed in could be selected through a filter. In addition, in terms of backwards compatibility, this is a new addition, and hence carries no expectation of unslashed/slashed globals.

It's also not about what's easiest (there's no real change in terms of that), but more about making wp_insert_post safe to call twice.

Replying to scribu:

In other words, the problem is that you can't distinguish between the original call made by Core and other calls.

Also, calls to wp_insert_post can't allow custom meta data without setting $_POST manually.

[dd32]

In which case, the fields that are passed in could be selected through a filter.
In addition, in terms of backwards compatibility, this is a new addition, and hence carries no expectation of unslashed/slashed globals.

I'm assuming that this would be implemented before #18322 here, #18322 has a longer term goal which would need to be planned, this, at first glance just looks like a easy nice thing to do - but one that has future backcompat potential issues