Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#55176 closed defect (bug) (fixed)

if $stickies in function stick_post is not array, it should be empty array

Reported by: denishua's profile denishua Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.7
Component: Posts, Post Types Keywords: has-patch has-unit-tests needs-testing
Focuses: Cc:

Description

<?php
if ( ! is_array( $stickies ) ) {
                $stickies = array( $post_id );
        } 


should change to

<?php
if ( ! is_array( $stickies ) ) {
                $stickies = array();
        }

Because of some reason, if we delete the sticky_posts option in wp_options, then we can never sticky a post, the following code always false:

<?php
if ( ! in_array( $post_id, $stickies, true ) ) {
                $stickies[] = $post_id;
                $updated    = update_option( 'sticky_posts', array_values( $stickies ) );
        }

so it will never execute update_option. so we can't stick post.


Attachments (5)

#55176.2.patch (408 bytes) - added by kajalgohel 2 years ago.
55176.3.diff (1.0 KB) - added by azouamauriac 2 years ago.
The previous patch LGTM, I've juste made some optimisations here.
55176.4.diff (576 bytes) - added by azouamauriac 2 years ago.
55176.3.diff contains undesirable changes. this last is good.
55176.5.diff (1.1 KB) - added by azouamauriac 2 years ago.
unit test
55176.6.diff (1.6 KB) - added by azouamauriac 2 years ago.
add "* @ticket 52007" to test docblock and fix typo

Download all attachments as: .zip

Change History (18)

@kajalgohel
2 years ago

#1 @kajalgohel
2 years ago

  • Keywords has-patch added

Adding blank array for $stickies

#2 @azouamauriac
2 years ago

  • Component changed from General to Posts, Post Types

#3 @azouamauriac
2 years ago

  • Version set to trunk

Hello there, thanks for the report, I'm able to reproduce the issue, the patch looks good too I think.

The code was introduced here: [50380] to fix duplicated post's id in stick_post option.

pinging @peterwilsoncc as he was spearhead this part of code.

@azouamauriac
2 years ago

The previous patch LGTM, I've juste made some optimisations here.

#4 @azouamauriac
2 years ago

@SergeyBiryukov if you confirm the ticket, I think we can move it for 6.0 milestone.

@azouamauriac
2 years ago

55176.3.diff contains undesirable changes. this last is good.

#5 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.0

#6 @peterwilsoncc
2 years ago

  • Version changed from trunk to 5.7

Thanks for reporting this @denishua and providing the patches @azouamauriac.

It would be good to get some additional tests in to account for this situation as it was something I missed in #52007.

#7 @azouamauriac
2 years ago

  • Keywords needs-testing added

@azouamauriac
2 years ago

unit test

#8 @azouamauriac
2 years ago

  • Keywords has-unit-tests added

@peterwilsoncc I've added some units tests. let me know if you need something more.

@azouamauriac
2 years ago

add "* @ticket 52007" to test docblock and fix typo

#10 @peterwilsoncc
2 years ago

  • Owner set to peterwilsoncc
  • Status changed from new to assigned

Thanks for the tests @azouamauriac.

So I could ensure the tests were passing, I've created a pull request on GitHub and linked it to this ticket. This mainly to run the test suite before committing.

It combines 55176.4.diff and the tests in 55176.6.diff with some minor modifications:

  • data provider removed for deleting the option, it's not needed
  • added tests for various other incorrectly formed options
  • Reduced the changes in stick_post() to a single line -- the if ( $stickies ) { line didn't make sense so I had to guess your intent here
  • fixed a couple of minor PHP coding standards issues

peterwilsoncc commented on PR #2353:


2 years ago
#11

@costdev Would you be able to take a look at this PR, for some reason I can not assign you as a reviewer.

#12 @peterwilsoncc
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53238:

Posts, Post Types: Fix option validation in stick_post().

Normalize an invalid sticky_posts option to an empty array within stick_post(). This fixes a bug in which an unexpected option value would prevent new posts from being made sticky.

Follow up to [50380].

Props azouamauriac, denishua, kajalgohel, sergeybiryukov, costdev.
Fixes #55176.

Note: See TracTickets for help on using tickets.