-
Notifications
You must be signed in to change notification settings - Fork 105
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
Running dev/build via CLI throws [Notice]: Undefined index: HTTP_HOST #437
Comments
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.
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? |
Let's go on a journey
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 I set up an error page. I have chosen
BUT This has uncovered another bug, in that the error page has generated with the wrong domain.
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?
Note though that the domain used in the static page generation is not matching this environment definition 🤔 Oh... whoops!
Ah, but
The second line is for sites not in
So, a breakpoint in So, where are we so far?
|
Got it. Thanks @Leapfrognz for providing more context (via Slack) A subsite where This is the only criterion. Reproduction:
|
PR at #440 |
#440 is merged. |
After Director::host(); back, http_host issue is coming back. However, robbie's fix |
@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..? |
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 |
@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. |
Thanks, @satrun77. |
@robbyahn please confirm:
|
It was happened in cwp 2.3. |
Thanks for reporting back :) |
Because
$_SERVER
variables are not set in the CLI, thedev/build
command thows a notice:The text was updated successfully, but these errors were encountered: