Make WordPress Core

Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#59722 closed defect (bug) (fixed)

Regression: class-wp-rest-server.php

Reported by: perrelet's profile perrelet Owned by: jorbin's profile jorbin
Milestone: 6.4 Priority: high
Severity: major Version: 6.4
Component: REST API Keywords: has-patch dev-reviewed
Focuses: rest-api Cc:

Description

In https://github.com/WordPress/WordPress/commit/3f1e6a6a5031b319fd5ebfaaf6598fec3fe78fb1

The rest_send_nocache_headers block is moved after rest_pre_serve_request.

However this renders rest_pre_serve_request useless because rest_send_nocache_headers sends / removes http headers. As a result, using rest_pre_serve_request for it's intended purpose (to manually send the request) results in Cannot modify header information - headers already sent.

Suggest moving the rest_send_nocache_headers block before the rest_pre_serve_request filter.

Change History (13)

This ticket was mentioned in PR #5564 on WordPress/wordpress-develop by perrelet.


10 months ago
#1

  • Keywords has-patch added

The rest_send_nocache_headers block was moved after rest_pre_serve_request in https://github.com/WordPress/WordPress/commit/3f1e6a6a5031b319fd5ebfaaf6598fec3fe78fb1.

This renders rest_pre_serve_request useless because rest_send_nocache_headers sends / removes http headers. As a result, using rest_pre_serve_request for it's intended purpose (to manually send the request) results in:

Cannot modify header information - headers already sent.

Suggest moving the rest_send_nocache_headers block before the rest_pre_serve_request filter.

Trac ticket: https://core.trac.wordpress.org/ticket/59722#ticket

#2 @SergeyBiryukov
10 months ago

  • Component changed from General to REST API
  • Milestone changed from Awaiting Review to 6.4

Hi there, welcome back to WordPress Trac! Thanks for the report.

Introduced in [56834], moving to the milestone to get more eyes on this.

#3 @hellofromTonya
10 months ago

  • Priority changed from normal to high
  • Summary changed from regression: class-wp-rest-server.php to Regression: class-wp-rest-server.php

Moving up the priority for tracking and awareness.

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


10 months ago

#5 @perrelet
10 months ago

Thank you @SergeyBiryukov & @hellofromTonya 🙏

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


10 months ago

#7 @jorbin
9 months ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 57012:

REST API: Move rest_pre_serve_request filter to after no cache headers are sent.

[56834] adjusted the order of activity inside the rest server responses. This lead to the rest_pre_serve_request filter potentially blocking the sending of the no cache headers. This moves that action back to being after the sending of no cache headers has finished to restore the pre 6.3.2 order of these two actions.

Props perrelet, SergeyBiryukov, peterwilsoncc.
Fixes #59722.

#8 @jorbin
9 months ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Thank you @perrelet for identifying this.

Reopening for 2nd committer sign-off to backport to both the 6.4 and 6.3 branches.

#9 @hellofromTonya
9 months ago

  • Keywords dev-reviewed added; dev-feedback removed

[57012] LGTM for backport to 6.4

#10 @hellofromTonya
9 months ago

Whoops, sorry Jorbin, I missed that it's also for 6.3. +1 for that backport too.

#11 @jorbin
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57014:

REST API: Move rest_pre_serve_request filter to after no cache headers are sent.

Merges [57012] to 6.4 branch.

[56834] adjusted the order of activity inside the rest server responses. This lead to the rest_pre_serve_request filter potentially blocking the sending of the no cache headers. This moves that action back to being after the sending of no cache headers has finished to restore the pre 6.3.2 order of these two actions.

Props perrelet, SergeyBiryukov, peterwilsoncc, hellofromTonya.
Fixes #59722.

#12 @jorbin
9 months ago

In 57015:

REST API: Move rest_pre_serve_request filter to after no cache headers are sent.

Merges [57012] to 6.3 branch.

[56834] adjusted the order of activity inside the rest server responses. This lead to the rest_pre_serve_request filter potentially blocking the sending of the no cache headers. This moves that action back to being after the sending of no cache headers has finished to restore the pre 6.3.2 order of these two actions.

Props perrelet, SergeyBiryukov, peterwilsoncc, hellofromTonya.
Fixes #59722.

#13 @jorbin
9 months ago

Thanks everyone. As 6.3 is still being maintained, I wanted to include the fix there as well if/when a 6.3.3 is released but most likely this fix will come in 6.4.

Note: See TracTickets for help on using tickets.