centralnicgroup-opensource / rtldev-middleware-whmcs

CentralNic's WHMCS Software Bundle
https://centralnicreseller.com
Other
35 stars 15 forks source link

ibs_getCountryCodeByName bugged - IT transfers not occurring #273

Closed Kian987 closed 9 months ago

Kian987 commented 9 months ago

Describe the bug IBS module. Every transfer of .it domains always fails due to ibs_getCountryCodeByName() function being broken. Keep in mind that this function is used in 7 places therefore there could be issues also in other features.

Here is what ibs_getCountryCodeByName() does. Please notice that I kept only 2 countries in the array to avoid posting a wall of text with all countries of the world.

function ibs_getCountryCodeByName($countryName)
{
    $country = [ /* ... cut ... */ "ITALY" => "IT", "IVORY COAST" => "CI" /* ... cut ... */ ];
    return $country[$countryName];
}

The function converts country names into ISO 3166-1 alpha-2 (eg . "ITALY" becomes "IT"). The issue is that most of the times - if not all of the times - $countryName (the parameter) already comes in ISO 3166-1 alpha-2 format due to the fact that WHMCS stores nationality as IT, DE, ES and not as Italy, Germany, Spain.

In other words the feature is almost completely useless and always returns a null value leading to errors due to missing nationality. I solved the issue by replacing this:

$data["registrant_dotitnationality"] = ibs_getCountryCodeByName($data["registrant_dotitnationality"]);

With:

$data["registrant_dotitnationality"] = $data["registrant_dotitnationality"];

To Reproduce

  1. Begin the transfer an .it domain for an European-based customer
  2. The module informs you that Required parameter "registrant_dotitnationality" missing!
KaiSchwarz-cnic commented 9 months ago

hmm... not reproducible... You're basically right, but we patched that function already a while ago.

function ibs_getCountryCodeByName($countryName)
{
    static $country = null;
    if (is_null($country)) {
        $country = [
            "AFGHANISTAN" => "AF",
            // ...
            "ZIMBABWE" => "ZW"
        ];
    }
    return $country[strtoupper($countryName)] ?? $countryName;
}

This function broke in the past in case the input parameter was about a value not covered by the mapping (as you mentioned). That's why we introduced the coalescence operator to automatically fallback to the input parameter itself. In addition, a potential input parameter could also be country name but provided in lower case, camel case or whatever. That's why strtoupper is at least the next improvement to increase the chance for a match.

My honest opinion on that mapping: Hardcoding a country to ISO 3166-1 alpha-2 mapping is a not reliable way and leading to recurring manual maintenance effort. No idea why it had been covered that way in the past. A professional registrar system could make such a list accessible (so either providing the entire list or a function for doing that ISO Code or Country Name Lookup - depends on what is actually needed. Alternatively, using WHMCS built-in functionality (if available). One of the systems should offer that...

In lack of time, we just patched this - the final solution is of course a different one.

Hope that helps!