less strict string matching in magento-vars.php

[joeshelton-wagento]

The isHttpHost() function has very strict string matching. This requires a new block of code utilizing the function to be created for each new environment.

For example:

if (isHttpHost("www.examplea.com")) {
    $_SERVER["MAGE_RUN_CODE"] = "default";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("mcstaging.examplea.com")) {
    $_SERVER["MAGE_RUN_CODE"] = "default";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("examplea.branchname-zrgukpa-kolnt6awfmkyd.us-3.magentosite.cloud")) {
    $_SERVER["MAGE_RUN_CODE"] = "default";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("examplea.test")) {
    $_SERVER["MAGE_RUN_CODE"] = "default";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("www.exampleb.com")) {
    $_SERVER["MAGE_RUN_CODE"] = "storeb";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("mcstaging.exampleb.com")) {
    $_SERVER["MAGE_RUN_CODE"] = "storeb";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("exampleb.branchname-zrgukpa-kolnt6awfmkyd.us-3.magentosite.cloud")) {
    $_SERVER["MAGE_RUN_CODE"] = "storeb";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("exampleb.test")) {
    $_SERVER["MAGE_RUN_CODE"] = "storeb";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}

Also, the default function does not accommodate domain name differentiators that may occur at the end of the domain name. For example:

• example.com
• example.co.uk

I propose a more flexible string matching that would allow for one block of code to identify a store in all environments. For example:

if (isHttpHost("examplea")) {
    $_SERVER["MAGE_RUN_CODE"] = "default";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}
if (isHttpHost("exampleb")) {
    $_SERVER["MAGE_RUN_CODE"] = "storeb";
    $_SERVER["MAGE_RUN_TYPE"] = "store";
}

This is a better fit for Magento's dynamic system of hostname creation.

Personally, I always change the function to have more flexible string matching in my own projects. But, I notice that many people on Community Engineering Slack mistakenly assume that the function can't be changed. They rely on the function provided and are frustrated by it.

This is my own preference:

function isHttpHost($host)
{
    if (!isset($_SERVER['HTTP_HOST'])) {
        return false;
    }
    return strpos(str_replace('---', '.', $_SERVER['HTTP_HOST']), $host) !== false;
}

[NadiyaS]

Hi @joeshelton-wagento, thank you for letting us know about your experience and usage of this function. I agree that in such cases as you described, flexible string matching is preferred. But there are some cases when need to use strict check (e.g. for example.com and fr.example.com). Also, I am not sure about changing this method as it will be BIC. And adding a new method will be too much (probably). We may extend the description in this file and add examples you described. @YPyltiai what do you think?

[YPyltiai]

If documentation coverage would be enough and we do not want to introduce BIC, I am okay with that too. @joeshelton-wagento , would the documentation update work?

[joeshelton-wagento]

Changes (by Magento) in this file would not effect existing projects. In fact, this function was recently changed. I don't think changes here would be considered BIC.

https://github.com/magento/magento-cloud/commit/d9785ce58bf0ae27011de52226e24402bd5d8c49#diff-a82aa6e9574b747dac9152a71413a3ba

Regardless, I'm in favor of whatever will prevent people from putting environment-specific configuration in repo files. The current design pattern in the file is prompting people to do this. If we could enhance the comments directly in the magento-vars.php file, that would be great.

[NadiyaS]

@joeshelton-wagento you are right that change in this file does not affect the existed projects. But developers may continue to use it as they used to for future projects expecting the same behavior. We changed that function as the result was not changed at all. There is no needs instr_replace('---', '.', $_SERVER['HTTP_HOST']) anymore. We may change the name of this function, but still, I am not sure that it won't be considered as BC. Thoughts?

[joeshelton-wagento]

I see what you mean. I'm not in favor of people using the current function as-is. There's no way to use it without environment-specific domains being committed into a repository file. This is the worse of two evils, even if you consider the change BIC. So, I'm still in favor of totally removing it.

Perhaps the best change would be to not include a "live" code example at all. The main problem is that people are mistakenly believing that the function provided is necessary to use when in fact it is optional. Example string matching functions could be provided in comments. Then people would be more likely to understand that the file can take a form of their choosing. And people who were used to the old version of the file would immediately notice the change.

[joeshelton-wagento]

And, by the way, the behavior of that function did change. It became stricter.

d9785ce#diff-a82aa6e9574b747dac9152a71413a3ba

Previously a substring match starting at position 0 would return true. (eg. "example" would match "examplea.com" and "exampleb.com") After the change, a full match is necessary.

[NadiyaS]

And, by the way, the behavior of that function did change. It became stricter.

Oh. That should not have happened. It is definitely a mistake. Thanks for pointing that out.

[NadiyaS]

And I like your idea about

not include a "live" code example at all

Seems like it solves all problems

[taoufiqaitali]

i changed this file to this format, because i have many subdomains

` function isHttpHost($host) { if (!isset($_SERVER['HTTP_HOST'])) { return false; } return strpos($_SERVER['HTTP_HOST'], $host) > -1; }

if (isHttpHost("examplea")){ $_SERVER["MAGE_RUN_CODE"] = "storea"; $_SERVER["MAGE_RUN_TYPE"] = "store"; } if (isHttpHost("exampleb")){ $_SERVER["MAGE_RUN_CODE"] = "storeb"; $_SERVER["MAGE_RUN_TYPE"] = "store"; }

`