Closed ChrissiQ closed 5 years ago
We may be able to refactor this to use an injectable HTTPRequest, and also build in a silent return if no request is set. Something like this:
- $currentHost = $_SERVER['HTTP_HOST'];
+ if (!Injector::inst()->has(HTTPRequest::class)) {
+ return '';
+ }
+ /** @var HTTPRequest $request */
+ $request = Injector::inst()->get(HTTPRequest::class);
+ $currentHost = $request->getHost();
Untested
I've put this as impact/medium because I've never experienced this before, so it's probably harder to come across (do you have steps to reproduce?).
I'll note that some other server variables are set by HTTPRequestBuilder during the app startup, but HTTP_HOST doesn't appear to be one of them. I think we should replace its use in this code to avoid it the null pointer exception.
As to steps to recreate, I'm going to hazard a guess that this is because an error page has been created for a subsite, which I believe isn't automatically created when adding a subsite which is likely why we've not come across it. These write a static cache to disk on flush, which means the page must render - with correct links. I get similar errors all the time because my webserver setup runs a different user to my login, although mine are permission errors, it seems the trace to get to that point is familiar to me.
requireDefaultRecords
or a BuildTask
Are you able to confirm please @ChrissiQ ?
I also get the same issue. SS4 CWP 2.3
Would one of you mind providing steps to reproduce on a clean install please?
"require": {
"php": ">=5.6.0",
"silverstripe/recipe-plugin": "^1.2",
"silverstripe/recipe-cms": "4.4.x-dev",
"silverstripe-themes/simple": "~3.2.0",
"silverstripe/subsites": "2.3.x-dev"
},
I have set up a subsite. By default, it has either no pages or a single page (this admittedly was an existing install for addressing another subsites issue - but I've reset to heads and composer update
before proceeding)
I set up an error page. I have chosen 400
, please ignore that this is inconsistent with my choice of Title
(although this shouldn't matter; neither the code nor the title).
dev/build
has completed successfully.
BUT
This has uncovered another bug, in that the error page has generated with the wrong domain.
subfix.localhost
sfix.localhost
(note the filename in the breadcrumbs).Interestingly, when I do remove the domain from my Subsite definition (in the CMS), no page is generated at all. This is OK though.
So, how is my environment configured?
[vagrant@archlinux subfix]$ grep URL ../.env
SS_BASE_URL="http://localhost:8080/"
Note though that the domain used in the static page generation is not matching this environment definition 🤔
Oh... whoops!
[vagrant@archlinux subfix]$ php -i | grep error
display_errors => Off => Off
Ah, but
[vagrant@archlinux subfix]$ grep -rin error_reporting vendor/silverstripe/framework/
vendor/silverstripe/framework/src/Core/CoreKernel.php:455: error_reporting(E_ALL & ~(E_DEPRECATED | E_STRICT | E_NOTICE));
vendor/silverstripe/framework/src/Core/CoreKernel.php:458: error_reporting(E_ALL | E_STRICT);
The second line is for sites not in live
mode, and my environment is set to dev
:)
[vagrant@archlinux subfix]$ grep -i environment_type ../.env
SS_ENVIRONMENT_TYPE="dev"
So, a breakpoint in getSubstituteDomain
shows that when building, this code is never called.
http://localhost/
.
I have created a project subfix
, with a Subsite
with the SubsiteDomain
set to sfix.localhost
- these work as expected. I have created an error page on that subsite, and published it.dev/build
works for me. Static error page caches are created, but they use the wrong domain... and getSubstituteDomain
is never called.Got it. Thanks @Leapfrognz for providing more context (via Slack)
A subsite where Default site
is true
This is the only criterion.
Reproduction:
Default site
PR at #440
After Director::host(); back, http_host issue is coming back. However, robbie's fix isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; is working .. Can you please help why this is happened?
@NZTA can you please clarify what you mean? Is the fix working or not working for you?
@NZTA can you please clarify what you mean? Is the fix working or not working for you?
This is not working after changing to Director::host();.
Can you clarify the problem you’re having? Are you seeing undefined index errors, does your dev/build break, does the subsite functionality break..?
Hi @robbieaverill
The fix in #440 merged into version 2.0 but does not exist from version 2.1
2.0: https://github.com/silverstripe/silverstripe-subsites/blob/2.0/src/Model/SubsiteDomain.php#L188 2.1: https://github.com/silverstripe/silverstripe-subsites/blob/2.1/src/Model/SubsiteDomain.php#L188
@satrun77 thanks for letting me know. I've merged all the branches up. You can use a dev branch for now until a new version is tagged in each release line, e.g. 2.1.x-dev
or 2.2.x-dev
in your composer constraint for the subsites module.
Thanks, @satrun77. After running dev/build, it showed that undefined index errors under the subsite domain.
@robbyahn please confirm:
"silverstripe/subsites": "^2"
) to a dev version (e.g. "silverstripe/subsites": "2.2.x-dev"
)composer update
silverstripe/subsites
to 38b356c
@robbyahn please confirm:
- You have updated the composer dependency from a constraint (e.g.
"silverstripe/subsites": "^2"
) to a dev version (e.g."silverstripe/subsites": "2.2.x-dev"
)- You have run a
composer update
- Composer has updated
silverstripe/subsites
to38b356c
- 👀 You are still seeing the same error
It was happened in cwp 2.3. Thanks, after merging, it is working good.
Thanks for reporting back :) These adjustments will be released into CWP with the next quarterly release.
Because
$_SERVER
variables are not set in the CLI, thedev/build
command thows a notice: