jeremykendall / php-domain-parser

Public Suffix List based domain parsing implemented in PHP
MIT License
1.16k stars 128 forks source link

Return value of Pdp\Converter::idnToAscii() must be of the type string, boolean returned #230

Closed spagu closed 5 years ago

spagu commented 5 years ago

Issue summary

does not work

System informations

(In case of a bug report Please complete the table below)

Information Description
Pdp version 5.3.0
PHP version 7.2
OS Platform BSD

Standalone code, or other way to reproduce the problem

$manager = new Manager(new Cache(), new CurlHttpClient()); $rules = $manager->getRules();

Expected result

no error

Actual result

Type: TypeError Message: Return value of Pdp\Converter::idnToAscii() must be of the type string, boolean returned File: /project/vendor/jeremykendall/php-domain-parser/src/IDNAConverterTrait.php Line: 88

nyamsprod commented 5 years ago

@spagu thanks for your issue report but alas I fail to reproduce your error:

Could you please try using the develop branch to see if you still have the same issue thanks in advance. The following snippet should improve detecting where the bug is

use Pdd\Manager;
use Pdd\Rules;
$rules = Rules::createFromPath(Manager::PSL_URL);

If this snippet works then it means that the issue is with how the CurlHttpClient is setup. Some improvement will be landing on the next release to improve the CurlHttpClient adapter default setup but you can override them easily by providing your own curl options.

spagu commented 5 years ago

I tried as you suggested and this did not help:

Type: TypeError
Message: Return value of Pdp\Converter::idnToAscii() must be of the type string, boolean returned
File: /project/vendor/jeremykendall/php-domain-parser/src/IDNAConverterTrait.php
Line: 88
nyamsprod commented 5 years ago

mmmh seems that the downloaded files is corrupted somehow :thinking: Can you download the PSL directly from your browser and feed it to the Converter class like below:

use Pdp\Converter;

$content = file_get_contents('/path/saved/public-suffix-list.tld');
$converter = new Converter();
$res = $converter->convert($content);

If you still get an error then try with the develop branch.

Sorry for asking but I can not reproduce the error with my setup. I've tested on two different OS (Windows and Linux) and I get not error in both setup.

spagu commented 5 years ago
Type: TypeError
Message: Return value of Pdp\Converter::idnToAscii() must be of the type string, boolean returned
File: /project/vendor/jeremykendall/php-domain-parser/src/IDNAConverterTrait.php
Line: 88

this fails only when doing : $res = $converter->convert($content);

Notice: Undefined index: errors in /project/vendor/jeremykendall/php-domain-parser/src/IDNAConverterTrait.php on line 87

nyamsprod commented 5 years ago

Ok made some digging on php.net bug database and it seems you are running into this issue since you are on BSD :( https://bugs.php.net/bug.php?id=74276 which can not be resolve at this lib level I'm afraid :'(

spagu commented 5 years ago

I have added output check for false in IDNAConverterTrait.php / function idnToAscii

        if ( !$arr['errors'] && false !== $output ) {
            return $output;
        }

this somehow does not generate problem and idn names works. But I am not sure on impact.

nyamsprod commented 5 years ago

I thought about adding this check but it defeat completely the use of the $arr array and also it may skip valid public suffix on BSD .... a better alternative would be to create a special Exception for this case IMHO and tell the user that the ext-intl extension is not correctly installed at least not from PHP point of view.

nyamsprod commented 5 years ago

@spagu I choose to throw an UnexpectedValueException from PHP SPL Exception directly and not make the Exception part of the package Exception subnamespace to clearly mark that it is not a package exception per se but rather a PHP/Intl misconfiguration and that resolution should be taken at another level. Is that ok for you ?

nyamsprod commented 5 years ago

version 5.4.0 is released with the bug fix implemented