Skip to content

Support of error pages behind a load balancer that serves HTTPS #18333

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 4 commits into from
Oct 25, 2018

Conversation

stkams
Copy link
Contributor

@stkams stkams commented Oct 1, 2018

Support of error pages behind a load balancer that serves HTTPS but accesses the webserver on HTTP. Without this css is loaded on HTTP from a document loaded over HTTPS

Description

If you run magento on webservers serving on HTTP while the loadbalancer serves on HTTPS then base is set to a URL using HTTP which the page is HTTPS. The css is then blocked by browsers.

Fixed Issues (if relevant)

  1. magento/magento2#<issue_number>: Issue title
  2. ...

Manual testing scenarios

  1. ...
  2. ...

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

…ccesses the webserver on HTTP. Without this css is loaded on HTTP from a document loaded over HTTPS
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 1, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ihor-sviziev
❌ stefank


stefank seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@@ -265,7 +265,8 @@ public function getHostUrl()
$host = 'localhost';
}

$isSecure = (!empty($_SERVER['HTTPS'])) && ($_SERVER['HTTPS'] != 'off');
// HTTP_X_FORWARDED_PROTO to check whether a webserver using HTTP is behind a load balancer serving HTTPS
Copy link
Contributor

Choose a reason for hiding this comment

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

I think comment is not needed there, it's pretty obvious for what this header is needed.

@@ -265,7 +265,8 @@ public function getHostUrl()
$host = 'localhost';
}

$isSecure = (!empty($_SERVER['HTTPS'])) && ($_SERVER['HTTPS'] != 'off');
// HTTP_X_FORWARDED_PROTO to check whether a webserver using HTTP is behind a load balancer serving HTTPS
$isSecure = (!empty($_SERVER['HTTPS'])) && ($_SERVER['HTTPS'] != 'off') || $_SERVER['HTTP_X_FORWARDED_PROTO'] === 'https';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add checking that $_SERVER['HTTP_X_FORWARDED_PROTO'] exists, otherwise we'll have notice there

@ihor-sviziev
Copy link
Contributor

Hi @stkams,

Could you sign CLA? In case if you already signed CLA, but commit was done from your another email - please add this email to your github account and sign CLI.

Unfortunately we can't accept PRs from people who didn't signed CLA.

stkams added 2 commits October 3, 2018 17:04
…ccesses the webserver on HTTP. Without this css is loaded on HTTP from a document loaded over HTTPS
…ccesses the webserver on HTTP. Without this css is loaded on HTTP from a document loaded over HTTPS
@stkams
Copy link
Contributor Author

stkams commented Oct 3, 2018

Thanks for the review. All done

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Oct 4, 2018

Hi @stkams,
Thank you for you contribution!
Did you signed CLA? Looks like you've signed from another email/ account because CLA check still pending for sign. Please double check that.
Also looks like this changed by you file failing in static tests, could you fix it?

@stkams
Copy link
Contributor Author

stkams commented Oct 4, 2018

Yes, I had signed the CLA using my hotmail email address but there seem to be several ways so I did it again following above link

@ihor-sviziev
Copy link
Contributor

Hi @stkams your commits contains email no in Hotmail. Please add that email as secondary in your github account

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Oct 12, 2018
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-3170 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@stkams thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team
Copy link
Contributor

Hi @stkams. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants