Skip to content

Commit

Permalink
Protect against new term data changing pre-insert
Browse files Browse the repository at this point in the history
  • Loading branch information
@mboynes
mboynes committed Mar 19, 2022
1 parent 29d306d commit c299d4d
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 27 deletions.
96 changes: 91 additions & 5 deletions php/context/class-fieldmanager-context-term.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ class Fieldmanager_Context_Term extends Fieldmanager_Context_Storable {
*/
private $current_taxonomy;

/**
* Store data for inserted terms to ensure, as much as possible, that FM
* only stores data to the term being created and not any other term created
* as a side effect.
*
* @var array|null
*/
protected $inserting_term_data;

/**
* Instantiate this context. You can either pass an array of all args
* (preferred), or pass them individually (deprecated).
Expand Down Expand Up @@ -162,6 +171,7 @@ public function __construct( $args, $taxonomies = array(), $show_on_add = true,
if ( $this->show_on_add ) {
add_action( $taxonomy . '_add_form_fields', array( $this, 'add_term_fields' ), 10, 1 );
add_action( 'created_term', array( $this, 'save_term_fields' ), 10, 3 );
add_filter( 'pre_insert_term', array( $this, 'maybe_hook_into_create_term' ), 0, 2 );
}

if ( $this->show_on_edit ) {
Expand Down Expand Up @@ -298,17 +308,27 @@ public function save_term_fields( $term_id, $tt_id, $taxonomy ) {
&& $taxonomy === $_POST['taxonomy']
&& isset( $_POST['tag-name'], $_POST['parent'] )
) {
// This confirms that the term that was created reflects the term in the form.
// Ensure this was the created_term action.
if ( ! doing_action( 'created_term' ) ) {
return;
}

if ( empty( $this->inserting_term_data ) ) {
// Something has gone awry, this shouldn't happen, so bail.
return;
}

// Core expects terms to have a unique combination of [taxonomy, name, parent].
$term = get_term( $term_id );
if ( $term->name !== $_POST['tag-name'] ) {
if ( $term->name !== $this->inserting_term_data['name'] ) {
return;
}

$posted_parent = max( 0, (int) $_POST['parent'] );
if ( $posted_parent !== $term->parent ) {
if ( $this->inserting_term_data['parent'] !== $term->parent ) {
return;
}

// This term appears to check out.
unset( $this->inserting_term_data );
}
// phpcs:enable WordPress.Security.NonceVerification.Missing

Expand All @@ -334,6 +354,72 @@ public function save_term_fields( $term_id, $tt_id, $taxonomy ) {
$this->save_to_term_meta( $term_id, $taxonomy );
}

/**
* Hook into create_term if the term being created matches the postdata.
*
* @param string|WP_Error $name The term name to add, or a WP_Error object if there's an error.
* @param string $taxonomy Taxonomy slug.
* @return string|WP_Error $name, unmodified.
*/
public function maybe_hook_into_create_term( $name, $taxonomy ) {
// phpcs:disable WordPress.Security.NonceVerification.Missing
if (
! is_wp_error( $name )
&& ! empty( $_POST['tag-name'] ) && $_POST['tag-name'] === $name
&& ! empty( $_POST['taxonomy'] ) && $_POST['taxonomy'] === $taxonomy
) {
add_action( 'create_term', array( $this, 'verify_new_term_data_didnt_change' ), 10, 1 );
// Append the data to the queued insert.
$this->inserting_term_data = array(
'depth' => 1,
'name' => $name,
'taxonomy' => $taxonomy,
'parent' => ! empty( $_POST['parent'] ) ? max( (int) $_POST['parent'], 0 ) : 0,
);
} elseif ( isset( $this->inserting_term_data['depth'] ) ) {
// If a term is being insert between `pre_insert_term` and `create_term`, note it.
$this->inserting_term_data['depth']++;
}

return $name;
// phpcs:enable WordPress.Security.NonceVerification.Missing
}

/**
* Verify the term data didn't change prior to insert.
*
* @param int $term_id Term ID.
*/
public function verify_new_term_data_didnt_change( $term_id ) {
// Early escape for a situation which shouldn't happen.
if ( empty( $this->inserting_term_data ) ) {
return;
}

// If depth > 1, it means that this term is being inserted while the target
// term is, as a side effect. This will ignore it and reduce the tracked depth.
if ( $this->inserting_term_data['depth'] > 1 ) {
$this->inserting_term_data['depth']--;
return;
}

$term = get_term( $term_id );
if (
$term->name !== $this->inserting_term_data['name']
|| $term->taxonomy !== $this->inserting_term_data['taxonomy']
|| $term->parent !== $this->inserting_term_data['parent']
) {
// The data was manipualted prior to insert.
$this->inserting_term_data['name'] = $term->name;
$this->inserting_term_data['taxonomy'] = $term->taxonomy;
$this->inserting_term_data['parent'] = $term->parent;
}

// Since the term is now confirmed, skip any additional checks.
remove_filter( 'pre_insert_term', array( $this, 'maybe_hook_into_create_term' ), 0, 2 );
remove_action( 'create_term', array( $this, 'verify_new_term_data_didnt_change' ), 10, 1 );
}

/**
* Helper to save an array of data to term meta.
*
Expand Down
77 changes: 55 additions & 22 deletions tests/php/test-fieldmanager-context-term.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,14 @@ public function test_term_saving_side_effects_on_term_update() {

// Set the POST data.
$_POST = [
'tag_ID' => $this->term_id,
'taxonomy' => $this->taxonomy,
'name' => 'News',
'slug' => 'news',
'description' => 'General news',
'parent' => '-1',
'tag_ID' => $this->term_id,
'taxonomy' => $this->taxonomy,
'name' => 'News',
'slug' => 'news',
'description' => 'General news',
'parent' => '-1',
"fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ),
$name => $value,
$name => $value,
];

// Trigger the intended save.
Expand Down Expand Up @@ -461,31 +461,27 @@ public function test_term_saving_side_effects_on_term_update() {
* @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831
*/
public function test_term_saving_side_effects_on_term_create() {
$name = 'side-effect';
$value = 'Fieldmanager was here';
$name = 'side-effect';
$value = 'Fieldmanager was here';
$new_term_name = 'New Term';

// Register a Fieldmanager Field and add the term context.
$fm = new Fieldmanager_TextField( compact( 'name' ) );
$fm->add_term_meta_box( 'Testing Side Effects', [ $this->taxonomy ] );

// Set the POST data.
$_POST = [
'taxonomy' => $this->taxonomy,
'tag-name' => $this->term->name,
'slug' => $this->term->slug,
'description' => $this->term->description,
'parent' => '-1',
'taxonomy' => $this->taxonomy,
'tag-name' => $new_term_name,
'slug' => '',
'description' => '',
'parent' => '-1',
"fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ),
$name => $value,
$name => $value,
];

// Trigger the intended save.
do_action(
'created_term',
$this->term_id,
$this->tt_id,
$this->taxonomy
);
$new_term = wp_insert_term( $new_term_name, $this->taxonomy );

// Fake a side effect.
$side_effect_term = self::factory()->term->create_and_get(
Expand All @@ -500,7 +496,44 @@ public function test_term_saving_side_effects_on_term_create() {
$this->taxonomy
);

$this->assertSame( $value, get_term_meta( $this->term_id, $name, true ) );
$this->assertSame( $value, get_term_meta( $new_term['term_id'], $name, true ) );
$this->assertSame( [], get_term_meta( $side_effect_term->term_id, $name ) );
}

/**
* @see https://github.com/alleyinteractive/wordpress-fieldmanager/issues/831
*/
public function test_term_meta_saving_on_term_create_when_a_filter_alters_the_term_name() {
$name = 'testing';
$value = 'Fieldmanager was here';
$new_term_name = 'New Term';

// Register a Fieldmanager Field and add the term context.
$fm = new Fieldmanager_TextField( compact( 'name' ) );
$fm->add_term_meta_box( 'Testing', [ $this->taxonomy ] );

// Set the POST data.
$_POST = [
'taxonomy' => $this->taxonomy,
'tag-name' => $new_term_name,
'slug' => '',
'description' => '',
'parent' => '-1',
"fieldmanager-{$name}-nonce" => wp_create_nonce( "fieldmanager-save-{$name}" ),
$name => $value,
];

// Manipualte the term name prior to insert.
add_filter(
'pre_insert_term',
function( $term_name ) {
return "Edited: {$term_name}";
}
);

// Insert the term.
$term = wp_insert_term( $new_term_name, $this->taxonomy );

$this->assertSame( $value, get_term_meta( $term['term_id'], $name, true ) );
}
}

0 comments on commit c299d4d

Please sign in to comment.