Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controller response of type redirect does not call 'exit' #38

Open
lllopo opened this issue Nov 29, 2022 · 0 comments
Open

Controller response of type redirect does not call 'exit' #38

lllopo opened this issue Nov 29, 2022 · 0 comments

Comments

@lllopo
Copy link

lllopo commented Nov 29, 2022

Version

Please add the exact versions used for each of the following:

  • WP Emerge: 0.17.0
  • WordPress: 6.1.1
  • PHP: 8.0.15

Expected behavior

When returning a response of type App::redirect()->to( ... ) as controller action result the redirect should happen immediately and stop any further rendering and processing after the redirect headers are returned.

Actual behavior

The processing continues and the page that is called upon gets fully rendered although unnecessary. All actions and filters fire.

Steps to reproduce (in case of a bug)

  1. Create a controller action that returns App::redirect()->to( ... )
  2. Make it fire on an existing admin page like users.php
  3. Check if all of users.php actions a filters fire.

Comments

I know it is a somewhat edge case, but here my setup - I am extending the admin users.php functions with additional actions. I added custom route conditions to catch these. Example : /wp-admin/users.php?dplt-action=mark_as_deleted is caught by my controller and processed properly. The problem is that I redirect back to users.php?role=whatever is set. At the redirect I don't want to have all of the users.php processing firing after my controller is done anyway. Unfortunately, since there is no exit or die it does. This mainly messes up my approach to flashing messages as admin notices, but also slows down the page without any need. So, I would like to suggest the following fix, if it won't cause other issues : add in ResponseService change sendHeaders like this :

public function sendHeaders( ResponseInterface $response ) {
		// Status
		header( sprintf(
			'HTTP/%s %s %s',
			$response->getProtocolVersion(),
			$response->getStatusCode(),
			$response->getReasonPhrase()
		) );

		// Headers
		foreach ( $response->getHeaders() as $name => $values ) {
			foreach ( $values as $i => $value ) {
				header( sprintf( '%s: %s', $name, $value ), $i === 0 );
			}
		}

		if($response->getStatusCode() >= 300 && $response->getStatusCode() < 400) {
			exit;
		}
	}

I tried it and it works, but again - I don't know if it is fully safe and won't cause other issues for WPEmerge inernally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant