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

[4.4] Rework redirect exception #7610

Merged
merged 8 commits into from
Jun 29, 2023

Conversation

iRedds
Copy link
Collaborator

@iRedds iRedds commented Jun 23, 2023

Description
I would like to propose the following changes to the handling and use of RedirectException.

  • RedirectException now implements ResponsableInterface

  • ResponsableInterface allows developers to add custom exceptions that will prepare the Response object themselves.

  • The logic for generating a Response object for a RedirectException has been moved from the Codeigniter class to RedirectException, and the RedirectException catch has been replaced with the ResponsableInterface.

  • The RedirectException constructor now also accepts a ResponseInterface as its first argument. This allows the developer to set headers, cookies, for the Response object.

  • The CodeIgniter::forceSecureAccess() method has been moved from the run() method to the handleRequest() method, and the force_https() function has been redesigned so that it no longer terminates the script, but throws a RedirectException, passing the exception to the constructor Response object. This approach will gracefully terminate the application.

  • The force_https() function sets the HTTP status code 307, which allows the HTTP request method to be preserved after the redirect.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the 4.4 label Jun 24, 2023
@iRedds
Copy link
Collaborator Author

iRedds commented Jun 25, 2023

Added documentation, tests and changes.

@iRedds iRedds marked this pull request as ready for review June 25, 2023 00:52
@kenjis kenjis added the enhancement PRs that improve existing functionalities label Jun 28, 2023
@kenjis
Copy link
Member

kenjis commented Jun 28, 2023

The last comment.

The force_https() function sets the HTTP status code 307, which allows the HTTP request method to be preserved after the redirect.

This change is not documented. How about like this?

--- a/user_guide_src/source/changelogs/v4.4.0.rst
+++ b/user_guide_src/source/changelogs/v4.4.0.rst
@@ -145,6 +145,10 @@ Changes
   So if you installed CodeIgniter under the folder that contains the special
   characters like ``(``, ``)``, etc., CodeIgniter didn't work. Since v4.4.0,
   this restriction has been removed.
+- **HSTS:** Now :php:func:`force_https()` or
+  ``Config\App::$forceGlobalSecureRequests = true`` sets the HTTP status code 307,
+  which allows the HTTP request method to be preserved after the redirect.
+  In previous versions, it was 302.
 
 Deprecations
 ************

@iRedds
Copy link
Collaborator Author

iRedds commented Jun 28, 2023

@kenjis Thanks for the help.

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@iRedds iRedds force-pushed the rework-redirect-exception branch from 96b6dba to abb921a Compare June 29, 2023 00:47
@kenjis kenjis merged commit aab7037 into codeigniter4:4.4 Jun 29, 2023
@kenjis
Copy link
Member

kenjis commented Jun 29, 2023

@iRedds Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.4 enhancement PRs that improve existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants