Detect binary prefix in intval() with base = 0 #2182
Conversation
|
I don't really feel very happy about the code for sending the negative value to |
|
Made the prefix check a little less terrible, I think. |
|
Uff, looks like |
|
When you do, be sure to check whitespace in the same manner it does. I think it uses the same rules as |
|
@lt bump ... |
|
@krakjoe I've added the whitespace checks, and a check for a unary '+' symbol. |
ext/standard/type.c
Outdated
| zval *num; | ||
| zend_long base = 10; | ||
|
|
||
| zend_long strlen; |
ext/standard/type.c
Outdated
|
|
||
| if (tmpval[0] == '0' && tmpval[1] == 'b') { | ||
| /* This effectively causes the string to have two leading zeros */ | ||
| tmpval[1] = '0'; |
There was a problem hiding this comment.
Unfortunately doing this is not possible, as it may cause SHM corruption.
There was a problem hiding this comment.
Well that sucks, I was trying to avoid copying the string.
ext/standard/type.c
Outdated
| tmpval++; | ||
| } | ||
|
|
||
| if (tmpval[0] == '0' && tmpval[1] == 'b') { |
There was a problem hiding this comment.
Do we accept 0X next to 0x in intval? If so, 0B should be supported here as well.
There was a problem hiding this comment.
A quick test shows 0X is accepted.
//TODO
Nikita informs me that the previous code could cause potential SHM corruption.
ext/standard/type.c
Outdated
| offset = 1; | ||
| } | ||
|
|
||
| if (strval[offset] == '0' && (strval[offset + 1] == 'b' || strval[offset + 1] == 'B')) { |
There was a problem hiding this comment.
I think we're missing a base==0 check somewhere around here.
| --EXPECTF-- | ||
| --- Good Inputs --- | ||
| string(34) "0b11111111111111111111111111111111" | ||
| int(4294967295) |
There was a problem hiding this comment.
This is presumably going to have different output on 32 bit and 64 bit. We either need to fork the test or, more easily, drop one bit :)
|
@krakjoe This fine for 7.1? |
|
👍 |
|
Merged via 4f7b498, thanks! |
https://bugs.php.net/bug.php?id=73374
strtolldoesn't detect0bas a prefix so we have to fake it.