Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#39010 closed enhancement (fixed)

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

Reported by: jnylen0's profile jnylen0 Owned by: jnylen0's profile 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 7 years ago.

Download all attachments as: .zip

Change History (12)

@jnylen0
7 years ago

#1 @jnylen0
7 years ago

  • Component changed from General to REST API

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


7 years ago

#3 @rachelbaker
7 years ago

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

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

  • Milestone changed from 4.8 to 4.7.1

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

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


7 years ago

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