Skip to content
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

processor.php getHostUrl() does not detect the server port correctly #12969

Closed
boldhedgehog opened this issue Jan 3, 2018 · 3 comments
Closed
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@boldhedgehog
Copy link

When Magento runs using the sandwich hosting set up (nginx-Varnish-nginx), and the backend nginx run on a not standard port (80 nor 443), the base URL is detected wrong in pub/errors/processor.hp

Preconditions

  1. Magento 2.1.7
  2. nginx as the SSL offloader on 80, 443 ports
  3. Varnish used for FPC
  4. nginx as the backend for Varnish listening to not standard port (for example 8081)
  5. PHP-FPM runs Magento
  6. Production mode

Steps to reproduce

  1. Open any media resource that does not exist and is not allowed in var/resource_config.json

Expected result

  1. Magento's 404 page should be displayed in the browser with a link to configured skin CSS.
  2. Base in the HTML code should be generated based on the requested host, for example:
<base href="https://domain.com/errors/default/" />

Actual result

  1. Magento's 404 page should be displayed in the browser with a link to configured skin CSS.
  2. Base in the HTML code is generated based on the requested host AND $_SERVER['SERVER_PORT'] , for example:
<base href="https://domain.com:8081/errors/default/" />
  1. No static assets with relative path can be loaded due to the faulty base

The logic in processor.php should be similar to one in //vendor/zendframework/zend-http/src/PhpEnvironment/Request.php:

  • Try to use HTTP_HOST header
  • If the above is not available, the host is $_SERVER['SERVER_NAME']:$_SERVER['SERVER_PORT']

At this moment also in 2.3-develop branch the server port is always concatenated:

public function getHostUrl()
    {
        /**
         * Define server http host
         */
        if (!empty($_SERVER['HTTP_HOST'])) {
            $host = $_SERVER['HTTP_HOST'];
        } elseif (!empty($_SERVER['SERVER_NAME'])) {
            $host = $_SERVER['SERVER_NAME'];
        } else {
            $host = 'localhost';
        }

        $isSecure = (!empty($_SERVER['HTTPS'])) && ($_SERVER['HTTPS'] != 'off');
        $url = ($isSecure ? 'https://' : 'http://') . $host;

// ------> should not be the case if the $host is assigned 
// ------> from  $_SERVER['HTTP_HOST']
       if (!empty($_SERVER['SERVER_PORT']) && !in_array($_SERVER['SERVER_PORT'], [80, 433])
            && !preg_match('/.*?\:[0-9]+$/', $url)
        ) {
            $url .= ':' . $_SERVER['SERVER_PORT'];
        }
        return  $url;
    }
@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jan 3, 2018
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Mar 14, 2018
@magento-engcom-team
Copy link
Contributor

@boldhedgehog, thank you for your report.
We've acknowledged the issue and added to our backlog.

@magento-engcom-team magento-engcom-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Mar 15, 2018
@sidolov sidolov added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Oct 16, 2018
@sidolov
Copy link
Contributor

sidolov commented Oct 16, 2018

Hi @boldhedgehog. Thank you for your report.
The issue has been fixed in #18393 by @loganstellway in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.1 release.

magento-engcom-team added a commit that referenced this issue Oct 16, 2018
 - Merge Pull Request #18393 from loganstellway/magento2:fix-for-issue-12969
 - Merged commits:
   1. 422d244
   2. 1e28a8e
   3. 7d921b6
gelanivishal pushed a commit to gelanivishal/magento2 that referenced this issue Oct 16, 2018
Updates `getHostUrl()` method to reference `HTTP_HOST` rather than `SERVER_PORT`.
@magento-engcom-team
Copy link
Contributor

Hi @boldhedgehog. Thank you for your report.
The issue has been fixed in #18659 by @gelanivishal in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.8 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Oct 17, 2018
magento-engcom-team added a commit that referenced this issue Oct 17, 2018
…rors #18659

 - Merge Pull Request #18659 from gelanivishal/magento2:2.2-develop-PR-port-18393
 - Merged commits:
   1. ef426a7
   2. 774a8a2
   3. 763be49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

3 participants