WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#23920 closed defect (bug) (duplicate)

Revisions: clean up wp_ajax_revisions_data()

Reported by: azaozz Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Revisions Keywords:
Focuses: Cc:

Description

See #23497. There are inconsistencies in wp_ajax_revisions_data(), mostly when getting/sanitizing the $_GET values.

We would probably need to pass the main post's ID every time and do current_user_can( 'edit_post' ID ). The code an the moment would show all revisions data to any logged in user that has 'view_post' capability. That cap is fine for the main post but not for revisions.

Attachments (2)

23920.patch (4.6 KB) - added by azaozz 2 years ago.
23920.2.patch (7.1 KB) - added by ocean90 2 years ago.

Download all attachments as: .zip

Change History (10)

@azaozz2 years ago

comment:1 @azaozz2 years ago

Also seeing two compare_to=... set in the $_GET when getting the revisions diffs (all requests to admin-ajax.php after the first one).

@ocean902 years ago

comment:2 follow-up: @ocean902 years ago

23920.2.patch: Did a small clean up while working on #23901. Merged them with 23920.patch​.


Also seeing two compare_to=... set in the $_GET when getting the revisions diffs (all requests to admin-ajax.php after the first one).

Interesting…

comment:3 in reply to: ↑ 2 @adamsilverstein2 years ago

Replying to ocean90:

23920.2.patch: Did a small clean up while working on #23901. Merged them with 23920.patch​.


Also seeing two compare_to=... set in the $_GET when getting the revisions diffs (all requests to admin-ajax.php after the first one).

Interesting…

there should only be one compare_to value in the ajax calls! needs investigating.

comment:4 follow-up: @SergeyBiryukov2 years ago

While we're at it, could we replace $linesadded/$linesdeleted with $lines_added/$lines_deleted (in wp_text_diff_with_count() too), to be more readable and in line with the surrounding variable names?

Same for $alltherevisions vs. $all_the_revisions.

comment:5 in reply to: ↑ 4 ; follow-up: @ocean902 years ago

Replying to SergeyBiryukov:

Sure, done in the upcoming patch.

comment:6 in reply to: ↑ 5 @azaozz2 years ago

Replying to ocean90:

Sure, done in the upcoming patch.

Also see the (int) sanitizing of $right_handle_at and $left_handle_at is not in 23920.2.patch. As far as I see these can only be integers?

comment:7 @ocean902 years ago

  • Milestone 3.6 deleted
  • Resolution set to duplicate
  • Status changed from new to closed
  • Version trunk deleted

Merged into #23901.

comment:8 @SergeyBiryukov2 years ago

  • Version set to trunk
Note: See TracTickets for help on using tickets.