Skip to content

Commit fe2b33f

Browse files
westonruterwestonruter
committed
Posts, Post Types: Short-circuit wp_post_delete() when $post_id arg is not above zero after casting to int.
The casting to `int` ensures that the action callbacks for post deletion can safely use the `int` type hint for the `$post_id` argument, as otherwise a fatal error occurs when an integer string is passed. This function also originally had casting of the argument to an integer, going back to at least WP 1.5.0, since it was passed directly into an SQL query. The casting was removed in [6180] with the introduction of prepared SQL statements. The `wp_delete_post()` function had `$post_id = 0` defined as its argument, also going back at least to WP 1.5.0, perhaps as a way to indicate the type of the argument as being an integer before there was PHPDoc. Unlike with functions like `get_post()` which have `$post = null` as the default argument to fall back to getting the global post, no such fallback logic was added to `wp_delete_post()`, meaning that passing no argument would always result in a DB query to locate the post with an `ID` of `0`, which will never happen. So this introduces a `_doing_it_wrong()` in case `0` is passed, and yet the default value of `0` is not removed from the function signature to not introduce a fatal error in case any existing code is not supplying the `$post_id` parameter (however unlikely this may be). Unit tests have been fleshed out for `wp_delete_post()` to add coverage for what was previously missing. Props SirLouen, kkmuffme, fakhriaz, sajjad67, siliconforks, peterwilsoncc, westonruter. Fixes #63975. git-svn-id: https://develop.svn.wordpress.org/trunk@60906 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 799b2d0 commit fe2b33f

File tree

4 files changed

+249
-54
lines changed

4 files changed

+249
-54
lines changed

‎src/wp-includes/post.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3743,14 +3743,20 @@ function wp_post_mime_type_where( $post_mime_types, $table_alias = '' ) {
37433743
* @see wp_delete_attachment()
37443744
* @see wp_trash_post()
37453745
*
3746-
* @param int $post_id Optional. Post ID. Default 0.
3746+
* @param int $post_id Post ID. (The default of 0 is for historical reasons; providing it is incorrect.)
37473747
* @param bool $force_delete Optional. Whether to bypass Trash and force deletion.
37483748
* Default false.
37493749
* @return WP_Post|false|null Post data on success, false or null on failure.
37503750
*/
37513751
function wp_delete_post( $post_id = 0, $force_delete = false ) {
37523752
global $wpdb;
37533753

3754+
$post_id = (int) $post_id;
3755+
if ( $post_id <= 0 ) {
3756+
_doing_it_wrong( __FUNCTION__, __( 'The post ID must be greater than 0.' ), '6.9.0' );
3757+
return false;
3758+
}
3759+
37543760
$post = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM $wpdb->posts WHERE ID = %d", $post_id ) );
37553761

37563762
if ( ! $post ) {
@@ -3775,7 +3781,7 @@ function wp_delete_post( $post_id = 0, $force_delete = false ) {
37753781
*
37763782
* @since 4.4.0
37773783
*
3778-
* @param WP_Post|false|null $delete Whether to go forward with deletion.
3784+
* @param WP_Post|false|null $check Whether to go forward with deletion. Anything other than null will short-circuit deletion.
37793785
* @param WP_Post $post Post object.
37803786
* @param bool $force_delete Whether to bypass the Trash.
37813787
*/

‎tests/phpunit/tests/post.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -502,30 +502,6 @@ public function test_hooks_fire_when_post_gets_stuck_and_unstuck() {
502502
$this->assertSame( 1, $a2->get_call_count() );
503503
}
504504

505-
public function test_wp_delete_post_reassign_hierarchical_post_type() {
506-
$grandparent_page_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
507-
$parent_page_id = self::factory()->post->create(
508-
array(
509-
'post_type' => 'page',
510-
'post_parent' => $grandparent_page_id,
511-
)
512-
);
513-
$page_id = self::factory()->post->create(
514-
array(
515-
'post_type' => 'page',
516-
'post_parent' => $parent_page_id,
517-
)
518-
);
519-
520-
$this->assertSame( $parent_page_id, get_post( $page_id )->post_parent );
521-
522-
wp_delete_post( $parent_page_id, true );
523-
$this->assertSame( $grandparent_page_id, get_post( $page_id )->post_parent );
524-
525-
wp_delete_post( $grandparent_page_id, true );
526-
$this->assertSame( 0, get_post( $page_id )->post_parent );
527-
}
528-
529505
/**
530506
* Test ensuring that the post_slug can be filtered with a custom value short circuiting the built in
531507
* function that tries to create a unique name based on the post name.

‎tests/phpunit/tests/post/wpDeletePost.php‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
<?php
2+
/**
3+
* Test wp_delete_post() function
4+
*
5+
* @package WordPress
6+
* @subpackage Post
7+
*
8+
* @since 6.9.0
9+
*/
10+
11+
/**
12+
* Class to Test wp_delete_post() function
13+
*
14+
* @group post
15+
* @covers ::wp_delete_post
16+
*/
17+
class Tests_Post_WpDeletePost extends WP_UnitTestCase {
18+
19+
/**
20+
* User IDs for the test.
21+
*
22+
* @var array{administrator: int, editor: int, contributor: int}
23+
*/
24+
protected static $user_ids;
25+
26+
/**
27+
* Set up before class.
28+
*
29+
* @param WP_UnitTest_Factory $factory The Unit Test Factory.
30+
*/
31+
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
32+
self::$user_ids = array(
33+
'administrator' => $factory->user->create(
34+
array(
35+
'role' => 'administrator',
36+
)
37+
),
38+
'editor' => $factory->user->create(
39+
array(
40+
'role' => 'editor',
41+
)
42+
),
43+
'contributor' => $factory->user->create(
44+
array(
45+
'role' => 'contributor',
46+
)
47+
),
48+
);
49+
}
50+
51+
/**
52+
* Tests wp_delete_post reassign hierarchical post type.
53+
*/
54+
public function test_wp_delete_post_reassign_hierarchical_post_type() {
55+
$grandparent_page_id = self::factory()->post->create( array( 'post_type' => 'page' ) );
56+
$parent_page_id = self::factory()->post->create(
57+
array(
58+
'post_type' => 'page',
59+
'post_parent' => $grandparent_page_id,
60+
)
61+
);
62+
$page_id = self::factory()->post->create(
63+
array(
64+
'post_type' => 'page',
65+
'post_parent' => $parent_page_id,
66+
)
67+
);
68+
69+
$this->assertSame( $parent_page_id, get_post( $page_id )->post_parent );
70+
71+
$this->assertInstanceOf( WP_Post::class, wp_delete_post( $parent_page_id, true ) );
72+
$this->assertSame( $grandparent_page_id, get_post( $page_id )->post_parent );
73+
74+
$this->assertInstanceOf( WP_Post::class, wp_delete_post( $grandparent_page_id, true ) );
75+
$this->assertSame( 0, get_post( $page_id )->post_parent );
76+
}
77+
78+
/**
79+
* Tests: "When I delete a future post using wp_delete_post( $post->ID ) it does not update the cron correctly."
80+
*
81+
* @ticket 5364
82+
*/
83+
public function test_delete_future_post_cron() {
84+
$future_date = strtotime( '+1 day' );
85+
86+
$data = array(
87+
'post_status' => 'publish',
88+
'post_content' => 'content',
89+
'post_title' => 'title',
90+
'post_date' => date_format( date_create( "@{$future_date}" ), 'Y-m-d H:i:s' ),
91+
);
92+
93+
// Insert a post and make sure the ID is OK.
94+
$post_id = wp_insert_post( $data );
95+
96+
// Check that there's a publish_future_post job scheduled at the right time.
97+
$this->assertSame( $future_date, $this->next_schedule_for_post( 'publish_future_post', $post_id ) );
98+
99+
// Now delete the post and make sure the cron entry is removed.
100+
$this->assertInstanceOf( WP_Post::class, wp_delete_post( $post_id ) );
101+
102+
$this->assertFalse( $this->next_schedule_for_post( 'publish_future_post', $post_id ) );
103+
}
104+
105+
/**
106+
* Helper function: return the timestamp(s) of cron jobs for the specified hook and post.
107+
*/
108+
private function next_schedule_for_post( $hook, int $post_id ) {
109+
return wp_next_scheduled( $hook, array( 0 => $post_id ) );
110+
}
111+
112+
/**
113+
* Tests that if the post_id is 0, wp_delete_post should return false.
114+
*
115+
* @ticket 63975
116+
*/
117+
public function test_wp_delete_post_short_circuit_on_post_id_zero() {
118+
$this->setExpectedIncorrectUsage( 'wp_delete_post' );
119+
$this->assertFalse( wp_delete_post( 0, true ) );
120+
}
121+
122+
/**
123+
* Tests wp_delete_post() when the post for the post_id has been already deleted.
124+
*/
125+
public function test_wp_delete_post_returns_false_for_invalid_post() {
126+
$post_id = self::factory()->post->create();
127+
$deleted_post = wp_delete_post( $post_id, true );
128+
$this->assertInstanceOf( WP_Post::class, $deleted_post );
129+
$this->assertSame( $post_id, $deleted_post->ID );
130+
131+
$this->assertNull( wp_delete_post( $post_id, true ) );
132+
}
133+
134+
/**
135+
* Tests actions triggered when deleting a post, even when a string ID is supplied.
136+
*
137+
* @ticket 63975
138+
*/
139+
public function test_wp_delete_post_actions() {
140+
$actions = array(
141+
'before_delete_post',
142+
'delete_post_post',
143+
'delete_post',
144+
'deleted_post_post',
145+
'deleted_post',
146+
'after_delete_post',
147+
);
148+
$captured_action_args = array();
149+
$initial_action_counts = array();
150+
foreach ( $actions as $action ) {
151+
$initial_action_counts[ $action ] = did_action( $action );
152+
add_action(
153+
$action,
154+
static function () use ( $action, &$captured_action_args ) {
155+
$captured_action_args[ $action ] = func_get_args();
156+
},
157+
10,
158+
PHP_INT_MAX
159+
);
160+
}
161+
162+
$post_id = self::factory()->post->create();
163+
$deleted_post = wp_delete_post( (string) $post_id, true );
164+
$this->assertInstanceOf( WP_Post::class, $deleted_post );
165+
166+
foreach ( array( 'before_delete_post', 'delete_post_post', 'delete_post', 'deleted_post_post', 'deleted_post', 'after_delete_post' ) as $action ) {
167+
$this->assertSame( $initial_action_counts[ $action ] + 1, did_action( $action ), "Expected $action action count to increment by 1." );
168+
$this->assertCount( 2, $captured_action_args[ $action ], "Expected count for $action action" );
169+
$this->assertSame( $post_id, $captured_action_args[ $action ][0], "Expected post ID for $action action" );
170+
$this->assertInstanceOf( WP_Post::class, $captured_action_args[ $action ][1], "Expected class for $action action" );
171+
$this->assertSame( $post_id, $captured_action_args[ $action ][1]->ID, "Expected post ID for $action action" );
172+
}
173+
}
174+
175+
/**
176+
* Tests short-circuiting wp_delete_post() with pre_delete_post filter.
177+
*
178+
* @ticket @63975
179+
*/
180+
public function test_wp_delete_post_can_be_short_circuited() {
181+
$post_id = self::factory()->post->create();
182+
$filter = function ( $check, WP_Post $post, bool $force_delete ) use ( $post_id ) {
183+
$this->assertNull( $check );
184+
$this->assertSame( $post_id, $post->ID );
185+
$this->assertTrue( $force_delete );
186+
return false;
187+
};
188+
189+
add_filter( 'pre_delete_post', $filter, 10, 3 );
190+
$this->assertFalse( wp_delete_post( $post_id, true ) );
191+
$post = get_post( $post_id );
192+
$this->assertInstanceOf( WP_Post::class, $post );
193+
$this->assertSame( $post_id, $post->ID );
194+
}
195+
196+
/**
197+
* Tests that wp_delete_post() deletes associated comments.
198+
*/
199+
public function test_wp_delete_post_deletes_associated_comments() {
200+
$post_id = self::factory()->post->create();
201+
$comment_id = self::factory()->comment->create(
202+
array(
203+
'comment_post_ID' => $post_id,
204+
'comment_type' => 'comment',
205+
)
206+
);
207+
208+
$this->assertInstanceOf( WP_Post::class, wp_delete_post( $post_id, true ) );
209+
210+
$this->assertNull( get_comment( $comment_id ) );
211+
}
212+
213+
/**
214+
* Tests that upon deletion of a post, attachments should be reattached to the parent post.
215+
*/
216+
public function test_wp_delete_post_reassigns_attachments_to_parent() {
217+
$parent_post_id = self::factory()->post->create(
218+
array(
219+
'post_type' => 'page',
220+
)
221+
);
222+
$post_id = self::factory()->post->create(
223+
array(
224+
'post_parent' => $parent_post_id,
225+
'post_type' => 'page',
226+
)
227+
);
228+
229+
$attachment_id = self::factory()->attachment->create(
230+
array(
231+
'post_parent' => $post_id,
232+
'post_type' => 'attachment',
233+
)
234+
);
235+
236+
$this->assertInstanceOf( WP_Post::class, wp_delete_post( $post_id, true ) );
237+
clean_post_cache( $attachment_id );
238+
239+
$this->assertSame( $parent_post_id, get_post( $attachment_id )->post_parent );
240+
}
241+
}

‎tests/phpunit/tests/post/wpInsertPost.php‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -565,34 +565,6 @@ public function test_wp_insert_post_with_meta_input() {
565565
$this->assertSame( 'bar', get_post_meta( $post_id, 'foo', true ) );
566566
}
567567

568-
/**
569-
* "When I delete a future post using wp_delete_post( $post->ID ) it does not update the cron correctly."
570-
*
571-
* @ticket 5364
572-
* @covers ::wp_delete_post
573-
*/
574-
public function test_delete_future_post_cron() {
575-
$future_date = strtotime( '+1 day' );
576-
577-
$data = array(
578-
'post_status' => 'publish',
579-
'post_content' => 'content',
580-
'post_title' => 'title',
581-
'post_date' => date_format( date_create( "@{$future_date}" ), 'Y-m-d H:i:s' ),
582-
);
583-
584-
// Insert a post and make sure the ID is OK.
585-
$post_id = wp_insert_post( $data );
586-
587-
// Check that there's a publish_future_post job scheduled at the right time.
588-
$this->assertSame( $future_date, $this->next_schedule_for_post( 'publish_future_post', $post_id ) );
589-
590-
// Now delete the post and make sure the cron entry is removed.
591-
wp_delete_post( $post_id );
592-
593-
$this->assertFalse( $this->next_schedule_for_post( 'publish_future_post', $post_id ) );
594-
}
595-
596568
/**
597569
* Bug: permalink doesn't work if post title is empty.
598570
*

0 commit comments

Comments
 (0)