Skip to content

feat: add $allowedHostnames for multiple domain support #6785

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

Merged
merged 21 commits into from
Nov 11, 2022

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Oct 29, 2022

Description
Supersedes #6759
Fixes #5807
Related #6746

  • add Config\App::$allowedHostnames
  • add URI::$baseURL, URI::setBaseURL() and URI::getBaseURL()
  • IncomingRequest::__construct() sets URI::$baseURL
    • use HTTP_HOST when it is an allowed hostname
  • refactor

Changes:

  • current_url(), base_url(), site_url() will return the URL with allowed HTTP_HOST
  • previous_url() will return the site URL with allowed HTTP_HOST when cannot get it from session nor HTTP_REFERER

The following functions/methods are affected (There may be others):

  • Using site_url()
    • form_open()
    • audio()
    • embed()
    • img()
    • link_tag()
    • object()
    • script_tag()
    • source()
    • video()
    • RedirectResponse::route()
    • RedirectResponse::to()
    • Toolbar::prepare()
    • Toolbar::respond()
    • anchor()
    • anchor_popup()
    • base_url()
    • previous_url()
    • url_to()
  • Using base_url()
    • RedirectException in CodeIgnter.php L377
    • UserAgent::isReferral()
  • Using current_url()
    • ChromeLoggerHandler::__construct()
    • CodeIgniter::storePreviousURL()
    • Pager::ensureGroup()
    • Toolbar::run()

Ref #4651

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 enhancement PRs that improve existing functionalities 4.4 labels Oct 29, 2022
@kenjis kenjis marked this pull request as draft October 29, 2022 23:16
@kenjis kenjis marked this pull request as ready for review October 29, 2022 23:28
@kenjis kenjis added the docs needed Pull requests needing documentation write-ups and/or revisions. label Oct 29, 2022
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

I think this is a better solution. Thank you for enumerating the impacted methods and components: that would be a helpful addition to the changelogs.

One thing I would like to note: for a while now I have been a proponent of a separate class for "internal URIs". Using one class for internal logic and hostname resolution and normal URL parsing & passing... it seems to be multiple responsibilities and prone to problems. Past conversations have concluded that maintaining multiple URI classes is overkill and confusing. This PR mixes concerns even more so I would like to bring this conversation back up again for v5.

@kenjis kenjis added 4.3 and removed 4.4 labels Nov 9, 2022
@kenjis kenjis force-pushed the feat-add-allowedHostnames branch from 0909d02 to 69d35ef Compare November 9, 2022 23:55
@kenjis kenjis removed the docs needed Pull requests needing documentation write-ups and/or revisions. label Nov 9, 2022
@kenjis
Copy link
Member Author

kenjis commented Nov 9, 2022

Added the docs.

@kenjis kenjis requested review from michalsn and MGatner November 9, 2022 23:56
@kenjis kenjis force-pushed the feat-add-allowedHostnames branch from 69d35ef to aaadc40 Compare November 10, 2022 00:09
Copy link
Contributor

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

Excellent, this feature always felt empty.

@kenjis kenjis merged commit 16f5c4b into codeigniter4:4.3 Nov 11, 2022
@kenjis kenjis deleted the feat-add-allowedHostnames branch November 11, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs that improve existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants