WsdlToPhp / PackageGenerator

Generates a PHP SDK based on a WSDL, simple and powerful, WSDL to PHP
https://providr.io
MIT License
426 stars 73 forks source link

is_numeric is too permissive when checking for int's #172

Closed ragol closed 5 years ago

ragol commented 5 years ago

The function is_numeric() is too permissive for checking integers. It also allows numbers with decimal parts. Use ctype_digit() instead.

mikaelcom commented 5 years ago

@ragol ctype_digit validates '6' but does not validate 6 which is disapointing as the test should pass for a use integer :cry:.

So I don't know if I were too tired (up late at night) at the moment but there is simply the is_int function which will invalidate '6', '6.5', 6.5.

Pushed on feature/issue-168 branch

ragol commented 5 years ago

is_int function is too strict since it expects an integer as input. If you give it a string that just happens to look like an integer it will return false. ctype_digit seems to do the opposite.

Decimal:

>>> ctype_digit(6)
=> false
>>> ctype_digit('6')
=> true
>>> is_int(6)
=> true
>>> is_int('6')
=> false

Also it depends on whether you have a decimal number or use another base like octal or hexadecimal numbers. Although I would assume that's an unusual case for XML values.

Hexadecimal

>>> ctype_digit('0x6')
=> false
>>> ctype_digit(0x6)
=> false
>>> is_int('0x6')
=> false
>>> is_int(0x6)
=> true

Octal:

>>> ctype_digit(06)
=> false
>>> ctype_digit('06')
=> true
>>> is_int(06)
=> true
>>> is_int('06')
=> false

filter_var() with FILTER_VALIDATE_INT filter checks the data type of the variable. So a string consisting of digits would get rejected.

I assume that all values coming from XML would be strings. Thus you need to use functions that work with the string representation and ignore the actual data type. And you have to avoid PHP's type conversion. That's the reasoning behind my suggestion of using ctype_digit. If it is possible that the validating function gets an actual integer instead of a string that looks like one then you'll have to check both cases. Something like

is_int($var) || ctype_digit($var)
mikaelcom commented 5 years ago

I'm not sure to follow you. If int have to be checked, then it should ensure that the passed value is actually an int.

What is your use case? Do you have a concrete example of WSDL => generated PHP => request contruction parameters? ,Thx

ragol commented 5 years ago

I want to use it on the SOAP server for request validation:

  1. Server receives a SOAP request.
  2. Server instantiates a request object that was generated from WSDL. If incoming request is invalid a request object should not be able to be instantiated.

Since SOAP request is XML and therefore a string then validation using is_int would not be the right tool.

mikaelcom commented 5 years ago

Ok, I do understand now. I must admit this is not the primary goal of the generated classes.

Your case is in my opinion proper to your situation as if you agree with, an int is an int, if the passed value is the representation of an int using a string, then it is not exactly an int. Sorry for my strictness.

For my knowledge, how are the classes used from the SoapServer? Is it you that maps the XML request to the PHP classes or is it automatic such as with the classmap option?

If it is you that maps the XML request data to the generated classes, then you should add a mapper that takes care of validating and casting the value to the right type before instanciating the objects.

mikaelcom commented 5 years ago

After some thought, it would require to replace !is_int($value) by !(is_int($value) || ctype_digit($value)).

I must admit I'm not a fan as my rigorousness makes me think it's not strict enough but as the sent XML request would not make any difference between a pure integer and stringified integer, I'm going to allow that modification

ragol commented 5 years ago

Ah, I've got your use case backwards then. :smile: An int is of course an int.

In my case I map XML to objects myself. I just thought that it would be easier to create and validate objects using classes generated directly from a schema.

mikaelcom commented 5 years ago

In the same spirit, float values should then be validated with is_float($value) || is_numeric($value), do you agree?

ragol commented 5 years ago

Yes, I agree

mikaelcom commented 5 years ago

@ragol it should be ok with the develop banch. Can you try before I release it? Thanks

ragol commented 5 years ago

Looks good to me