feat: add $allowedHostnames for multiple domain support by kenjis · Pull Request #6785 · codeigniter4/CodeIgniter4

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis · 2022-10-29T23:16:48Z

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

MGatner

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.

The URI object is updated in IncomingRequest constructor.

IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

kenjis · 2022-11-09T23:55:42Z

Added the docs.

datamweb

Excellent, this feature always felt empty.

kenjis added enhancement PRs that improve existing functionalities 4.4 labels Oct 29, 2022

kenjis marked this pull request as draft October 29, 2022 23:16

kenjis mentioned this pull request Oct 29, 2022
feat: improve multiple domain support #6759
Closed
5 tasks

kenjis marked this pull request as ready for review October 29, 2022 23:28

kenjis added the docs needed Pull requests needing documentation write-ups and/or revisions. label Oct 29, 2022

MGatner approved these changes Oct 30, 2022
View reviewed changes

michalsn approved these changes Nov 8, 2022
View reviewed changes

kenjis added 4.3 and removed 4.4 labels Nov 9, 2022

kenjis added 19 commits November 10, 2022 08:21

docs: update PHPDoc
192ee2f

docs: fix typo
c73b420

docs: add note
9d1ac4a

feat: add property $baseURL in URI
c41ec09

style: add ()
ad4dd52

docs: fix@throws
04ad08f

test: fix global state before test
560f953

fix: remove uneeded URI object
c36d787
The URI object is updated in IncomingRequest constructor.

feat: set baseURL in URI
2a3021e

refactor: _get_url() uses $request->getUri()->getBaseURL()
0f76766

refactor: do not use function call
e44ac50

test: fix failed test
7ee80a5

fix: enable detectURI()
6b80e3d
IncomingRequest::detectURI() updates URI ojbect state, so we need to run this.

test: update failed tests
87e4a54

test: add tests for Config\App::$AllowedHostname
6f945f6

test: add tests for base_url() and site_url() with $allowedHostnames
f8ae113

feat: add Config\App::$allowedHostnames and use it for URI::$baseURL
761289a

refactor: make detectURI() parameters deprecated
32fdad7

docs: update comments
c140ee4

kenjis force-pushed the feat-add-allowedHostnames branch from 0909d02 to 69d35efCompare November 9, 2022 23:55

kenjis removed the docs needed Pull requests needing documentation write-ups and/or revisions. label Nov 9, 2022

kenjis requested review from MGatner and michalsn November 9, 2022 23:56

docs: add user guide
aaadc40

kenjis force-pushed the feat-add-allowedHostnames branch from 69d35ef to aaadc40Compare November 10, 2022 00:09

datamweb approved these changes Nov 10, 2022
View reviewed changes

refactor: change $baseURL to private and add@internal
1f62c1c

michalsn approved these changes Nov 10, 2022
View reviewed changes

MGatner approved these changes Nov 11, 2022
View reviewed changes

kenjis merged commit 16f5c4b into codeigniter4:4.3Nov 11, 2022

kenjis deleted the feat-add-allowedHostnames branch November 11, 2022 08:19

kenjis mentioned this pull request Nov 11, 2022
Bug: Cache does not respect subdomain #6746
Closed