php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #65910 hex2bin returns null rather than false when parameter 1 is not scalar
Submitted: 2013-10-16 05:12 UTC Modified: 2014-12-30 18:45 UTC
From: lucas at threeamdesign dot com Assigned:
Status: Not a bug Package: *General Issues
PHP Version: 5.5Git-2013-10-16 (Git) OS:
Private report: No CVE-ID:
 [2013-10-16 05:12 UTC] lucas at threeamdesign dot com
Description:
------------
The docs say it returns false on failure but it returns null when the first argument is not a string (except objects with __toString() ).

The function should be made consistent. People who are aware of this quirk will likely be using a ($val === false || $val === null) test, and people who aren't will likely have only ($val === false). Making it return false wouldn't be perfectly backwards-compatible, but would fix more problems than it causes because failure goes undetected if only the latter test is used.

If the quirk must remain for compatibility, at least update the docs.

Test script:
---------------
<?php var_dump(hex2bin(array()));

Expected result:
----------------
Warning: hex2bin() expects parameter 1 to be string, array given in foo.php on line 1
bool(false)

Actual result:
--------------
Warning: hex2bin() expects parameter 1 to be string, array given in foo.php on line 1
NULL

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-10-23 01:49 UTC] yohgaki@php.net
-Status: Open +Status: Analyzed
 [2013-10-23 01:49 UTC] yohgaki@php.net
This is because "return;" is called when there is parameter mismatch.
hex2bin() is not the only one, but there are MANY of them.

What should we do with this?
It would be cleaner if we return FALSE for all parameter mismatch.

Any comments? 


/* {{{ proto string hex2bin(string data)
   Converts the hex representation of data to binary */
PHP_FUNCTION(hex2bin)
{
	char *result, *data;
	size_t newlen;
	int datalen;

	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &data, &datalen) == FAILURE) {
		return;
	}
 [2013-10-23 01:51 UTC] yohgaki@php.net
-Type: Documentation Problem +Type: Feature/Change Request -Package: Scripting Engine problem +Package: *General Issues
 [2014-12-30 18:29 UTC] levim@php.net
I think returning null for errors generally makes more sense than false. Read the definition of null in our manual and it hopefully explains why:

> The special NULL value represents a variable with no value.

If there is an error then there probably is no value to return. It makes more sense than false in any case.
 [2014-12-30 18:45 UTC] nikic@php.net
-Status: Analyzed +Status: Not a bug
 [2014-12-30 18:45 UTC] nikic@php.net
The null return value used for zpp failures is NULL by convention (all functions where there are no special BC concerns use it). The docs however leave the return value for this case explicitly undefined, see the note on http://php.net/manual/en/functions.internal.php. This behavior applies to all internal functions and as such is not mentioned on every single function page.
 
PHP Copyright © 2001-2015 The PHP Group
All rights reserved.
Last updated: Mon Jun 15 18:01:54 2015 UTC