Instantly share code, notes, and snippets.

Embed
What would you like to do?
An example function used to demonstrate how meta data is typically saved in a WordPress theme or plugin. The gist is made public so that developers can contribute to the standard security boilerplate functionality in order to simplify, reduce, and improve our serialization functions.
<?php
/**
* An example function used to demonstrate how to use the `user_can_save` function
* that provides boilerplate security checks when saving custom post meta data.
*
* The ultimate goal is provide a simple helper function to be used in themes and
* plugins without the need to use a set of complex conditionals and constants.
*
* Instead, the aim is to have a simplified function that's easy to read and that uses
* WordPress APIs.
*
* The DocBlocks should provide all information needed to understand how the function works.
*/
public function save_meta_data( $post_id ) {
if( user_can_save( $post_id, 'meta_data_nonce' ) ) {
/* ---------------------------------------- */
/* -- Actual serialization work occurs here */
/* ---------------------------------------- */
} // end if
} // end save_meta_data
/**
* Determines whether or not the current user has the ability to save meta data associated with this post.
*
* @param int $post_id The ID of the post being save
* @param bool Whether or not the user has the ability to save this post.
*/
function user_can_save( $post_id, $nonce ) {
$is_autosave = wp_is_post_autosave( $post_id );
$is_revision = wp_is_post_revision( $post_id );
$is_valid_nonce = ( isset( $_POST[ $nonce ] ) && wp_verify_nonce( $_POST[ $nonce ], plugin_basename( __FILE__ ) ) );
// Return true if the user is able to save; otherwise, false.
return ! ( $is_autosave || $is_revision ) && $is_valid_nonce;
} // end user_can_save
@curtismchale

This comment has been minimized.

Show comment
Hide comment
@curtismchale

curtismchale Jan 8, 2013

We really should be checking the post type by getting the post type from the post_id and then checking it against the $_POST value. As it stands this would not work with CPT's.

$post_type_slug = get_post_type( $post_id );
$post_type = get_post_type_object( $post_type_slug );

if( $post_type_slug === $_POST['post_type'] ){
    if( !current_user_can( $post_type->cap->edit_post, $post_id ) ) return;
}

curtismchale commented Jan 8, 2013

We really should be checking the post type by getting the post type from the post_id and then checking it against the $_POST value. As it stands this would not work with CPT's.

$post_type_slug = get_post_type( $post_id );
$post_type = get_post_type_object( $post_type_slug );

if( $post_type_slug === $_POST['post_type'] ){
    if( !current_user_can( $post_type->cap->edit_post, $post_id ) ) return;
}
@GeertDD

This comment has been minimized.

Show comment
Hide comment
@GeertDD

GeertDD Jan 8, 2013

Just a matter of coding style. Instead of wrapping the whole function within that initial if statement, I'd just exit the function early if the required post data does not exist. It improves readability.

if ( ! isset( $_POST['meta_data_nonce'] ) || ! isset( $_POST['post_type'] ) ) {
    return;
}

GeertDD commented Jan 8, 2013

Just a matter of coding style. Instead of wrapping the whole function within that initial if statement, I'd just exit the function early if the required post data does not exist. It improves readability.

if ( ! isset( $_POST['meta_data_nonce'] ) || ! isset( $_POST['post_type'] ) ) {
    return;
}
@bradyvercher

This comment has been minimized.

Show comment
Hide comment
@bradyvercher

bradyvercher Jan 9, 2013

I think the requirements for something like this change often enough that trying to abstract it may not really be necessary.

Sometimes you'll want to save meta data (or do any processing) on revisions, sometimes you won't. Capability checks are probably overkill because save_post won't get fired if the user doesn't have the capability in the first place. Nonces should weed out other scenarios and should make the post type check unnecessary in most cases.

Building on the formatting comment by @GeertDD, I've taken to this basic structure for my save_post hooks and it's seemed to work well. The variable names make it absolutely clear what the ternary operations do and they make the final conditional short and easy to read. Additional checks can be easily added in the same format without making the code more complex.

Then you're left to implement your logic however you see fit.

<?php
function save_meta_data( $post_id ) {
    $is_autosave = ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) ? true : false;
    $is_revision = wp_is_post_revision( $post_id );
    $is_valid_nonce = ( isset( $_POST['meta_data_nonce'] ) && wp_verify_nonce( $_POST['meta_data_nonce'], 'update-meta-data_' . $post_id ) ) ? true : false;

    // Bail if the data shouldn't be saved or intention can't be verified.
    if( $is_autosave || $is_revision || ! $is_valid_nonce ) {
        return;
    }

    // Save data
}

That's just my take. I'm interested to see what everyone else thinks about capability and post type checks. The actual saving logic could probably even be wrapped up in a separate function so that it could be used as an API for saving the data programmatically (otherwise, you don't get past the nonce).

I also hadn't seen plugin_basename( __FILE__ ) used as the second parameter in wp_verify_nonce() before, so I'm curious about that as well.

bradyvercher commented Jan 9, 2013

I think the requirements for something like this change often enough that trying to abstract it may not really be necessary.

Sometimes you'll want to save meta data (or do any processing) on revisions, sometimes you won't. Capability checks are probably overkill because save_post won't get fired if the user doesn't have the capability in the first place. Nonces should weed out other scenarios and should make the post type check unnecessary in most cases.

Building on the formatting comment by @GeertDD, I've taken to this basic structure for my save_post hooks and it's seemed to work well. The variable names make it absolutely clear what the ternary operations do and they make the final conditional short and easy to read. Additional checks can be easily added in the same format without making the code more complex.

Then you're left to implement your logic however you see fit.

<?php
function save_meta_data( $post_id ) {
    $is_autosave = ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) ? true : false;
    $is_revision = wp_is_post_revision( $post_id );
    $is_valid_nonce = ( isset( $_POST['meta_data_nonce'] ) && wp_verify_nonce( $_POST['meta_data_nonce'], 'update-meta-data_' . $post_id ) ) ? true : false;

    // Bail if the data shouldn't be saved or intention can't be verified.
    if( $is_autosave || $is_revision || ! $is_valid_nonce ) {
        return;
    }

    // Save data
}

That's just my take. I'm interested to see what everyone else thinks about capability and post type checks. The actual saving logic could probably even be wrapped up in a separate function so that it could be used as an API for saving the data programmatically (otherwise, you don't get past the nonce).

I also hadn't seen plugin_basename( __FILE__ ) used as the second parameter in wp_verify_nonce() before, so I'm curious about that as well.

@helen

This comment has been minimized.

Show comment
Hide comment
@helen

helen Jan 10, 2013

Why do you need to check the post type? edit_post is a meta cap about whether or not the person can edit that post of any post type, not that post of the post type post :)

helen commented Jan 10, 2013

Why do you need to check the post type? edit_post is a meta cap about whether or not the person can edit that post of any post type, not that post of the post type post :)

@tommcfarlin

This comment has been minimized.

Show comment
Hide comment
@tommcfarlin

tommcfarlin Jan 30, 2013

Adding these comments from the blog comment to make sure they are captured as well so this can continued to be refactored:

  • The gist embedding seems to be double encoding some characters, so ampersands are showing as a double amp; HTML entity
  • You’re checking to see if the user has permissions to post – I’ve got two comments on this. Firstly – I thought WordPress already handled this check for you? Second – if it doesn’t, then you’re only performing the check if the post_type is “post” and not any other post type (E.g. page, or custom post types) – is there a deliberate reason for that
  • The check for isset( $_POST['meta_data_nonce']) is redundant since you’re calling wp_verify_nonce later anyway?
Owner

tommcfarlin commented Jan 30, 2013

Adding these comments from the blog comment to make sure they are captured as well so this can continued to be refactored:

  • The gist embedding seems to be double encoding some characters, so ampersands are showing as a double amp; HTML entity
  • You’re checking to see if the user has permissions to post – I’ve got two comments on this. Firstly – I thought WordPress already handled this check for you? Second – if it doesn’t, then you’re only performing the check if the post_type is “post” and not any other post type (E.g. page, or custom post types) – is there a deliberate reason for that
  • The check for isset( $_POST['meta_data_nonce']) is redundant since you’re calling wp_verify_nonce later anyway?
@studioromeo

This comment has been minimized.

Show comment
Hide comment
@studioromeo

studioromeo Feb 5, 2013

Chiming in, you don't need to do this since it is already a boolean.

$is_valid_nonce ... ? true : false;

Instead you can write something like this:
$is_valid_nonce = ( isset( $_POST[ $nonce ] ) && wp_verify_nonce( $_POST[ $nonce ], plugin_basename( __FILE__ ) ) );

studioromeo commented Feb 5, 2013

Chiming in, you don't need to do this since it is already a boolean.

$is_valid_nonce ... ? true : false;

Instead you can write something like this:
$is_valid_nonce = ( isset( $_POST[ $nonce ] ) && wp_verify_nonce( $_POST[ $nonce ], plugin_basename( __FILE__ ) ) );

@dukizuki

This comment has been minimized.

Show comment
Hide comment
@dukizuki

dukizuki May 29, 2014

Hi Tom. maybe it's not correct place where I should ask this, but may I use this code snippet in commercial(Themeforest Theme) use? Thanks in advance..
^ Duke

dukizuki commented May 29, 2014

Hi Tom. maybe it's not correct place where I should ask this, but may I use this code snippet in commercial(Themeforest Theme) use? Thanks in advance..
^ Duke

@tamas-web

This comment has been minimized.

Show comment
Hide comment
@tamas-web

tamas-web Oct 21, 2015

Just out of curiosity...
Why wouldn't I want to save an auto save as well?
Maybe I have some meta data, that may be needed with auto save as well.
Lastly I'm fairly new to Wordpress, experienced coder, but still new to it's API, so which is the code part that checks if the user can save or not?

From what i can make out:
$is_autosave = wp_is_post_autosave( $post_id );
check if the current save state is an auto save or not, not related to user_can stuff, I think
$is_revision = wp_is_post_revision( $post_id );
check if the current save state is a revision
$is_valid_nonce =...
well no arguing there

tamas-web commented Oct 21, 2015

Just out of curiosity...
Why wouldn't I want to save an auto save as well?
Maybe I have some meta data, that may be needed with auto save as well.
Lastly I'm fairly new to Wordpress, experienced coder, but still new to it's API, so which is the code part that checks if the user can save or not?

From what i can make out:
$is_autosave = wp_is_post_autosave( $post_id );
check if the current save state is an auto save or not, not related to user_can stuff, I think
$is_revision = wp_is_post_revision( $post_id );
check if the current save state is a revision
$is_valid_nonce =...
well no arguing there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment