WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 10 days ago

#50075 new defect (bug)

Trigger _doing_it_wrong for dangerous REST API endpoint option

Reported by: rmccue Owned by:
Milestone: 5.5 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

REST API endpoints can be registered with a permission_callback option. It's really easy to accidentally write permissions_callback instead, and tough to catch during peer review, making this a potential footgun.

We should trigger a _doing_it_wrong() for this.

Change History (7)

#2 in reply to: ↑ 1 @rmccue
3 weeks ago

Replying to TimothyBlynJacobs:

One of the things we discussed is whether we should allow permission_callback => true as a shortcut for __return_true.

IMO no, as that's a potential footgun too; if you accidentally cast to true or something crazy. I think it also complicates the API surface for no real gain, given __return_true already exists.

Note that I'm only suggesting the misspelling here, not the lack of permission_callback; while that would be OK, it's a bit of an API breakage, and would cause a lot of warnings.

#3 @sorenbronsted
2 weeks ago

I will work on this

This ticket was mentioned in PR #265 on WordPress/wordpress-develop by sorenbronsted.


2 weeks ago

Added a notice when permission_callback is misspelled

Trac ticket: https://core.trac.wordpress.org/ticket/50075

#5 @sorenbronsted
2 weeks ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

#6 @TimothyBlynJacobs
10 days ago

IMO no, as that's a potential footgun too; if you accidentally cast to true or something crazy. I think it also complicates the API surface for no real gain, given __return_true already exists.

Works for me.

Note that I'm only suggesting the misspelling here, not the lack of permission_callback; while that would be OK, it's a bit of an API breakage, and would cause a lot of warnings.

Ah, I misunderstood. Yeah I'm conflicted about it, I kinda hoped that the issue last year would be a "one and done" type of deal, but it has cropped up again. I'm not sure if it is worth the noise.

#7 @prbot
10 days ago

TimothyBJacobs commented on PR #265:

Thanks for working on a patch @sorenbronsted! Left a couple of comments.

Note: See TracTickets for help on using tickets.