amphp / uri

Uri Parser and Resolver.
MIT License
24 stars 7 forks source link

An empty line caused by inet_pton() #8

Closed shtse8 closed 7 years ago

shtse8 commented 7 years ago

I am using amphp\artax as it is using amphp\uri. An empty line is printed when I send every request. After a deep investing, https://github.com/amphp/uri/blob/master/src/Uri.php#L49 produce an empty line. Any ideas?

kelunik commented 7 years ago

Do you use a custom error handler that doesn't correctly respect suppressed errors?

shtse8 commented 7 years ago

Yes, I have figured it out. This is the custom error handler I am using.

/* Error Handler */
    function exception_error_handler($severity, $string, $file, $line, $context) 
    {
        // Determine if this error is one of the enabled ones in php config (php.ini, .htaccess, etc)
        $error_is_enabled = (bool)($severity & ini_get('error_reporting') );

        // -- FATAL ERROR
        // throw an Error Exception, to be handled by whatever Exception handling logic is available in this context
        if( in_array($severity, array(E_USER_ERROR, E_RECOVERABLE_ERROR)) && $error_is_enabled ) {
            throw new ErrorException($errstr, 0, $errno, $errfile, $errline);
        }

        // -- NON-FATAL ERROR/WARNING/NOTICE
        // Log the error if it's enabled, otherwise just ignore it
        else if( $error_is_enabled ) {
            error_log( $message, 0 );
            return false; // Make sure this ends up in $php_errormsg, if appropriate
        }
    }
    set_error_handler("exception_error_handler");
kelunik commented 7 years ago

I'm not sure how that particular code just produces a newline, but for a side-note, you can just use error_reporting() instead of ini_get('error_reporting').

It's not an issue in our code, but rather your error handler.

shtse8 commented 7 years ago

instead of fixing the error handler, do you think possibly adding some codes to check the $this->host if it is valid for inet_pton instead of try and error?

shtse8 commented 7 years ago

because this error handler is got from http://php.net/manual/en/class.errorexception.php and I believe many developers are using it. We should avoid using @ for best practice.

kelunik commented 7 years ago

Checking for the right format would be quite some overhead and it also works that way. There are quite a few APIs in PHP where the @ operator is just the best choice one has, another example where it's used is fwrite(). Given that, you need to fix your error handler anyway. Did you find out where the newline comes from?

shtse8 commented 7 years ago

I didn't try and just disabled the error handler which I want PHP to throw ErrorException instead of just showing error message. By guessing, maybe error_log printing the empty $message which is hidden by @ operator.

kelunik commented 7 years ago

Did you try replacing the ini_get() with error_reporting()?