WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

#39010 closed enhancement (fixed)

REST API: Treat null and other falsy values like `false` in 'rest_allow_anonymous_comments'

Reported by: jnylen0 Owned by: jnylen0
Milestone: 4.7.1 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch has-unit-tests commit fixed-major
Focuses: Cc:

Description

Currently the rest_allow_anonymous_comments filter checks for an explicit false value to disable anonymous comments. We should extend this check to any falsy value instead.

As noted in the tests, the specific case I'm envisioning is that a plugin developer forgets to include a return value for some code path in their callback for this filter, leading to a value of null which is currently treated like true. We should fix this before it has a chance to show up in the wild.

Props @joehoyle for originally spotting this at ticket:38855#comment:6, but it wasn't addressed in the commit there.

Attachments (1)

39010.diff (2.3 KB) - added by jnylen0 9 months ago.

Download all attachments as: .zip

Change History (12)

@jnylen0
9 months ago

#1 @jnylen0
9 months ago

  • Component changed from General to REST API

This ticket was mentioned in Slack in #core-restapi by jnylen. View the logs.


9 months ago

#3 @rachelbaker
9 months ago

  • Milestone changed from 4.7 to 4.8
  • Type changed from defect (bug) to enhancement

#4 @jnylen0
9 months ago

@rachelbaker if we don't fix this in 4.7, is it something that we can still go back and fix later?

#5 @rachelbaker
9 months ago

  • Milestone changed from 4.8 to 4.7.1

#6 @rachelbaker
9 months ago

  • Keywords commit added
  • Owner set to jnylen0
  • Status changed from new to assigned

Patch 39010.diff looks good. @jnylen0 you want to commit when ready?

#7 @jnylen0
9 months ago

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

In 39487:

REST API: Treat any falsy value as false in 'rest_allow_anonymous_comments'.

Extend the check in 'rest_allow_anonymous_comments' to accept any falsy value
(previously this was an explicit check for false).

One possible failure case is that a plugin developer forgets to include a
return value for some code path in their callback for this filter, leading to a
value of null which is currently treated like true.

Props joehoyle, jnylen0.

Fixes #39010.

#8 @jnylen0
9 months ago

  • Keywords fixed-minor added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Needs merge to 4.7.1 after the 4.7 release.

Last edited 9 months ago by jnylen0 (previous) (diff)

#9 @jnylen0
9 months ago

  • Keywords fixed-major added; fixed-minor removed

Making this ticket great again.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


8 months ago

#11 @dd32
8 months ago

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

In 39566:

REST API: Treat any falsy value as false in 'rest_allow_anonymous_comments'.

Extend the check in 'rest_allow_anonymous_comments' to accept any falsy value (previously this was an explicit check for false).

One possible failure case is that a plugin developer forgets to include a return value for some code path in their callback for this filter, leading to a value of null which is currently treated like true.

Props joehoyle, jnylen0.
Merges [39487] to the 4.7 branch.
Fixes #39010.

Note: See TracTickets for help on using tickets.