fix for big int-ranges #934
Conversation
|
Could you add a failing unit test? |
|
@staabm If I try to add a test like this ... $builder = ConstantArrayTypeBuilder::createFromConstantArray(
new ConstantArrayType(
[new ConstantIntegerType(0), new ConstantIntegerType(1)],
[new StringType(), new StringType()],
),
);
$builder->setOffsetValueType(IntegerRangeType::fromInterval(-2147483648, 2147483647), new StringType());... I see I exception from phpunit itself: EDIT: test added and added a missing "-" before min-range |
voku
force-pushed
the
array_builder_fix
branch
from
e283c94
to
ca47adf
Compare
| @@ -104,14 +104,17 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt | |||
| $integerRanges = TypeUtils::getIntegerRanges($offsetType); | |||
| if (count($integerRanges) > 0) { | |||
| foreach ($integerRanges as $integerRange) { | |||
| if ($integerRange->getMin() === null) { | |||
| $minRange = $integerRange->getMin(); | |||
| if ($minRange === null || $minRange <= -self::ARRAY_COUNT_LIMIT) { | |||
There was a problem hiding this comment.
I dont think the fix is correct.
It reads like a range from a value smaller then -256 would no longer be possible?
There was a problem hiding this comment.
It wasn't possible before anyway, see next lines of code ... if (count($scalarTypes) > 0 && count($scalarTypes) < self::ARRAY_COUNT_LIMIT) {...
There was a problem hiding this comment.
I think its not the same thing to have a range of at max 256 elements vs only ranges between -256 and +256
There was a problem hiding this comment.
Other ideas? The problem is, we need to check this before the range( code, otherwise php will crash, Maybe we can avoid range and use a loop here?
There was a problem hiding this comment.
Hmm max-min <= limit could work since stepping is always 1 in this cases?
There was a problem hiding this comment.
Yes, that was the idea. I already tried it, maybe you can take look at it, thanks.
voku
force-pushed
the
array_builder_fix
branch
from
cba6fa7
to
4401bb3
Compare
fix issue #6375
fix issue #6375
voku
force-pushed
the
array_builder_fix
branch
from
4401bb3
to
cd99722
Compare
| do { | ||
| $rangeCount++; | ||
| if ($rangeCount > self::ARRAY_COUNT_LIMIT) { | ||
| $scalarTypes = $integerRangesBackup; |
There was a problem hiding this comment.
I don't understand the logic. The $scalarTypes array should never contain IntegerRangeType, why do you assign it there?
There was a problem hiding this comment.
I am sure that the classes that triggers the issue #6375 contains int-ranges and that the range call triggers the fatal error. So I tried to test it via unit tests.
What type triggers the fatal error from issue #6375? Does the int type somehow contains the int-range? I thought that this is a new type? 🤔
|
I'm sorry, I didn't understand your fix, so I fixed it by myself: d3f968d |
fix issue #6375