WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#32565 assigned defect (bug)

Adding an underscore prefix meta key with a meta value provides an incorrect error message

Reported by: danielpataki Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 4.3
Component: Options, Meta APIs Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

To reproduce this, use the custom fields section in a post to add a custom field. Add a hidden field (prefix the custom field name with an underscore) and type any value. The error returned is "Please provide a custom field value." which is not what the issue is, the issue is that we are trying to add a protected meta key.

I've included a fix which I don't particularly like. Perhaps add_meta should return a WP_Error object with the correct error. This would be easy enough to do, but I'm not familiar enough with add_meta(), I'm not sure what this could break.

I changed this in all three places the issue could occur, but I'm not sure this is needed. During troubleshooting it was obvious that the issue originates from line 1198. The other two occurrences could be line 1193 and 1281.

Hope this helps,

Daniel

Attachments (4)

ajax-actions.diff (1.6 KB) - added by danielpataki 3 years ago.
Diff to output the correct error, but not very elegantly coded
32565.diff (2.0 KB) - added by ericlewis 3 years ago.
32565.2.diff (1.3 KB) - added by ericlewis 3 years ago.
32565.3.diff (1.3 KB) - added by MikeHansenMe 3 years ago.

Download all attachments as: .zip

Change History (20)

@danielpataki
3 years ago

Diff to output the correct error, but not very elegantly coded

#1 @SergeyBiryukov
3 years ago

  • Component changed from General to Options, Meta APIs

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


3 years ago

#3 @jorbin
3 years ago

  • Type changed from defect (bug) to enhancement

The code is working as expected, but I think a better error message could certainly be in order.

@ericlewis
3 years ago

@ericlewis
3 years ago

#4 @ericlewis
3 years ago

In attachment:32565.2.diff, I've extended the work in @danielpataki's previous diff.

First, we can simplify the code here by issuing one call to add_meta() regardless of post status, and thereby one wp_die() statement.

Preferably add_meta() would supply us with extra failure information about why the routine failed, but we cannot modify the return value because of backwards compatibility. So, the work that function does to detect metakey must be duplicated here to decide whether add_meta() is failing because the user isn't allowed to edit it (by the meta being protected or capability checks) or if the value is empty.

Last edited 3 years ago by ericlewis (previous) (diff)

#5 @ericlewis
3 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.4

#6 @danielpataki
3 years ago

Thanks @ericlewis for that :) I assumed something like that was the issue with add_meta, thanks for clarifying!

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


3 years ago

#8 @SergeyBiryukov
3 years ago

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

#9 @DrewAPicture
3 years ago

@SergeyBiryukov What's the status here? Looks like there's some braces missing in 32565.2.diff and the patch still applies.

@MikeHansenMe
3 years ago

#10 @MikeHansenMe
3 years ago

32565.3.diff adds a the missing braces, removes assignment from inside the if, minor clean up, and stricter checks on $metakeyinput and $metakeyselect.

#11 @wonderboymusic
3 years ago

  • Milestone changed from 4.4 to Future Release

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


3 years ago

#13 @wonderboymusic
3 years ago

  • Milestone changed from Future Release to 4.4
  • Type changed from enhancement to defect (bug)

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


3 years ago

#15 @ocean90
3 years ago

  • Keywords needs-unit-tests added

This looks like it can be unit tested.

#16 @ocean90
3 years ago

  • Milestone changed from 4.4 to Future Release
Note: See TracTickets for help on using tickets.