WordPress.org

Make WordPress Core

#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 11 months ago.

Download all attachments as: .zip

Change History (12)

@jnylen0
11 months ago

#1 @jnylen0
11 months ago

  • Component changed from General to REST API

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


11 months ago

#3 @rachelbaker
11 months ago

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

#4 @jnylen0
11 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
11 months ago

  • Milestone changed from 4.8 to 4.7.1

#6 @rachelbaker
11 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
11 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
11 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 11 months ago by jnylen0 (previous) (diff)

#9 @jnylen0
11 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.


11 months ago

#11 @dd32
11 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.