Make WordPress Core

Opened 19 months ago

Last modified 9 months ago

#58763 new defect (bug)

Inconsistent add/get/update/delete_post_meta() functions leads to deleting post metadata.

Reported by: jsmoriss's profile jsmoriss Owned by:
Milestone: Future Release Priority: normal
Severity: major Version:
Component: General Keywords: has-patch needs-testing has-testing-info has-unit-tests early
Focuses: Cc:

Description

The add_post_meta(), delete_post_meta(), and update_post_meta() functions all use wp_is_post_revision() to add/delete/update the metadata of the post, not its revision. That's fine, but the get_post_meta() function does NOT use wp_is_post_revision().

As a consequence, if you use get_post_meta() and update_post_meta() during the saving process for a revision, the post metadata ends up being overwritten/deleted: The get_post_meta() function gets the revision metadata, and then the update_post_meta() function updates the post metadata, NOT the revision metadata.

js.

Change History (15)

#1 @audrasjb
19 months ago

  • Milestone changed from Awaiting Review to 6.4

#2 @oglekler
18 months ago

  • Keywords needs-patch added

This needs patch, and I am also wondering if this ticket needs to be handled early.

This ticket was mentioned in PR #5216 on WordPress/wordpress-develop by @monircoderex.


17 months ago
#3

  • Keywords has-patch added; needs-patch removed

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

## Description
This PR used wp_is_post_revision() function get_post_meta() function.

#4 @oglekler
16 months ago

  • Keywords needs-testing added

Needs testing, I wonder if changes that made for #20564 ticket, are affecting outcome for this issue.

#5 follow-up: @nicolefurlan
16 months ago

  • Keywords needs-testing-info added

This ticket could use some testing info to make things easier for testers. @jsmoriss would you be able to add them?

#6 in reply to: ↑ 5 @jsmoriss
16 months ago

Replying to nicolefurlan:

This ticket could use some testing info to make things easier for testers. @jsmoriss would you be able to add them?

Off the top of my head, here's some code to define a post metadata key for the post (not the revision), then when a revision is restored, the post metadata is erased because get_post_meta() gets the revision metadata (which does not exist) and update_post_meta() updates the post metadata.

add_action( 'save_post', 'test_save_post', 10, 1 );

function test_save_options( $post_id ) {

        $post_type = get_post_type( $post_id );

        if ( 'revision' !== $post_type ) {

                add_post_meta( $post_id, '_test_metadata', 'OK' );      // Adds POST metadata once.

                $val = get_post_meta( $post_id, '_test_metadata', $single = true );     // Value should always be "OK".

                error_log( $post_id . ' ' . $post_type . ' _test_metadata = ' . $val . ' (value should always be "OK" )' );

                if ( empty( $val ) ) {

                        error_log( $post_id . ' ' . $post_type . ' resetting test (by deleting _test_metadata)' );

                        delete_post_meta( $post_id, '_test_metadata' );
                }

        } else {

                $val = get_post_meta( $post_id, '_test_metadata', $single = true ); // Retrieves REVISION metadata (which does not exist).

                update_post_meta( $post_id, '_test_metadata', $val ); // Updates the POST metadata (not the revision metadata).

                error_log( $post_id . ' ' . $post_type . ' _test_metadata = ' . $val . ' (post metadata value now erased)' );
        }
}

The error log output looks something like this:

[09-Oct-2023 19:44:46 UTC] 1086 page _test_metadata = OK (value should always be "OK" )
[09-Oct-2023 19:45:27 UTC] 4624 revision _test_metadata =  (post metadata value now erased)
[09-Oct-2023 19:45:27 UTC] 1086 page _test_metadata =  (value should always be "OK" )
[09-Oct-2023 19:45:27 UTC] 1086 page resetting test (by deleting _test_metadata)

js.

#7 @nicolefurlan
16 months ago

  • Keywords has-testing-info added; needs-testing-info removed

Wonderful. Thank you @jsmoriss!

This ticket was mentioned in Slack in #core-test by nicolefurlan. View the logs.


16 months ago

This ticket was mentioned in PR #5476 on WordPress/wordpress-develop by @felipeelia.


16 months ago
#9

  • Keywords has-unit-tests added

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


16 months ago

#11 @nicolefurlan
16 months ago

  • Milestone changed from 6.4 to 6.5

This ticket was discussed in today's bug scrub.

Per @hellofromTonya:

Because of the change is touching older code, a deep dive into the code's commit history is needed. Why? To understand if it's by design, is it the root cause, when did the bug first start.

It would be great to get this into 6.5 Beta 1 to make sure there are no unknowns or side-effects.

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


12 months ago

#13 @audrasjb
12 months ago

  • Keywords early added
  • Milestone changed from 6.5 to 6.6

The aim was to commit it before beta 1. Looks like we let it slip to beta 2…

By the way, as unit tests are failing, it's probably better to handle this ticket early in the 6.6 cycle.

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


9 months ago

#15 @davidbaumwald
9 months ago

  • Milestone changed from 6.6 to Future Release

With a deeper dive needed and this ticket not really gaining any momentum the last two cycles, moving this to Future Release.

Once a path forward is determined, the milestone can be updated to a specific cycle.

Note: See TracTickets for help on using tickets.