Skip to content

Commit f807414

Browse files
GirgiasGirgias
committed
Improve error handling for when step = 0
Have a clear error message for this case, and eliminate a bunch of useless conditions as this is now pre-checked
1 parent a02a2f8 commit f807414

File tree

3 files changed

+102
-165
lines changed

3 files changed

+102
-165
lines changed

‎ext/standard/array.c

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2643,8 +2643,8 @@ PHP_FUNCTION(array_fill_keys)
26432643
}
26442644
/* }}} */
26452645

2646-
#define RANGE_CHECK_DOUBLE_INIT_ARRAY(start, end) do { \
2647-
double __calc_size = ((start - end) / step) + 1; \
2646+
#define RANGE_CHECK_DOUBLE_INIT_ARRAY(start, end, _step) do { \
2647+
double __calc_size = ((start - end) / (_step)) + 1; \
26482648
if (__calc_size >= (double)HT_MAX_SIZE) { \
26492649
zend_value_error(\
26502650
"The supplied range exceeds the maximum array size: start=%0.0f end=%0.0f", end, start); \
@@ -2670,32 +2670,39 @@ PHP_FUNCTION(array_fill_keys)
26702670
/* {{{ Create an array containing the range of integers or characters from low to high (inclusive) */
26712671
PHP_FUNCTION(range)
26722672
{
2673-
zval *zlow, *zhigh, *zstep = NULL, tmp;
2674-
int err = 0, is_step_double = 0;
2675-
double step = 1.0;
2673+
zval *zlow, *zhigh, *user_step = NULL, tmp;
2674+
bool err = 0, is_step_double = false;
2675+
double step_double = 1.0;
26762676

26772677
ZEND_PARSE_PARAMETERS_START(2, 3)
26782678
Z_PARAM_NUMBER_OR_STR(zlow)
26792679
Z_PARAM_NUMBER_OR_STR(zhigh)
26802680
Z_PARAM_OPTIONAL
2681-
Z_PARAM_NUMBER(zstep)
2681+
Z_PARAM_NUMBER(user_step)
26822682
ZEND_PARSE_PARAMETERS_END();
26832683

2684-
if (zstep) {
2685-
is_step_double = Z_TYPE_P(zstep) == IS_DOUBLE;
2686-
step = zval_get_double(zstep);
2687-
2684+
if (user_step) {
2685+
if (UNEXPECTED(Z_TYPE_P(user_step) == IS_DOUBLE)) {
2686+
step_double = Z_DVAL_P(user_step);
2687+
is_step_double = true;
2688+
} else {
2689+
step_double = (double) Z_LVAL_P(user_step);
2690+
}
2691+
if (step_double == 0.0) {
2692+
zend_argument_value_error(3, "cannot be 0");
2693+
RETURN_THROWS();
2694+
}
26882695
/* We only want positive step values. */
2689-
if (step < 0.0) {
2690-
step *= -1;
2696+
if (step_double < 0.0) {
2697+
step_double *= -1;
26912698
}
26922699
}
26932700

26942701
/* If the range is given as strings, generate an array of characters. */
26952702
if (Z_TYPE_P(zlow) == IS_STRING && Z_TYPE_P(zhigh) == IS_STRING && Z_STRLEN_P(zlow) >= 1 && Z_STRLEN_P(zhigh) >= 1) {
26962703
int type1, type2;
26972704
unsigned char low, high;
2698-
zend_long lstep = (zend_long) step;
2705+
zend_long lstep = (zend_long) step_double;
26992706

27002707
type1 = is_numeric_string(Z_STRVAL_P(zlow), Z_STRLEN_P(zlow), NULL, NULL, 0);
27012708
type2 = is_numeric_string(Z_STRVAL_P(zhigh), Z_STRLEN_P(zhigh), NULL, NULL, 0);
@@ -2710,7 +2717,7 @@ PHP_FUNCTION(range)
27102717
high = (unsigned char)Z_STRVAL_P(zhigh)[0];
27112718

27122719
if (low > high) { /* Negative Steps */
2713-
if (low - high < lstep || lstep <= 0) {
2720+
if (low - high < lstep) {
27142721
err = 1;
27152722
goto err;
27162723
}
@@ -2727,7 +2734,7 @@ PHP_FUNCTION(range)
27272734
}
27282735
} ZEND_HASH_FILL_END();
27292736
} else if (high > low) { /* Positive Steps */
2730-
if (high - low < lstep || lstep <= 0) {
2737+
if (high - low < lstep) {
27312738
err = 1;
27322739
goto err;
27332740
}
@@ -2760,29 +2767,29 @@ PHP_FUNCTION(range)
27602767
}
27612768

27622769
if (low > high) { /* Negative steps */
2763-
if (low - high < step || step <= 0) {
2770+
if (low - high < step_double) {
27642771
err = 1;
27652772
goto err;
27662773
}
27672774

2768-
RANGE_CHECK_DOUBLE_INIT_ARRAY(low, high);
2775+
RANGE_CHECK_DOUBLE_INIT_ARRAY(low, high, step_double);
27692776

27702777
ZEND_HASH_FILL_PACKED(Z_ARRVAL_P(return_value)) {
2771-
for (i = 0, element = low; i < size && element >= high; ++i, element = low - (i * step)) {
2778+
for (i = 0, element = low; i < size && element >= high; ++i, element = low - (i * step_double)) {
27722779
ZEND_HASH_FILL_SET_DOUBLE(element);
27732780
ZEND_HASH_FILL_NEXT();
27742781
}
27752782
} ZEND_HASH_FILL_END();
27762783
} else if (high > low) { /* Positive steps */
2777-
if (high - low < step || step <= 0) {
2784+
if (high - low < step_double) {
27782785
err = 1;
27792786
goto err;
27802787
}
27812788

2782-
RANGE_CHECK_DOUBLE_INIT_ARRAY(high, low);
2789+
RANGE_CHECK_DOUBLE_INIT_ARRAY(high, low, step_double);
27832790

27842791
ZEND_HASH_FILL_PACKED(Z_ARRVAL_P(return_value)) {
2785-
for (i = 0, element = low; i < size && element <= high; ++i, element = low + (i * step)) {
2792+
for (i = 0, element = low; i < size && element <= high; ++i, element = low + (i * step_double)) {
27862793
ZEND_HASH_FILL_SET_DOUBLE(element);
27872794
ZEND_HASH_FILL_NEXT();
27882795
}
@@ -2801,16 +2808,7 @@ PHP_FUNCTION(range)
28012808
low = zval_get_long(zlow);
28022809
high = zval_get_long(zhigh);
28032810

2804-
if (step <= 0) {
2805-
err = 1;
2806-
goto err;
2807-
}
2808-
2809-
lstep = (zend_ulong)step;
2810-
if (step <= 0) {
2811-
err = 1;
2812-
goto err;
2813-
}
2811+
lstep = (zend_ulong)step_double;
28142812

28152813
if (low > high) { /* Negative steps */
28162814
if ((zend_ulong)low - high < lstep) {

‎ext/standard/tests/array/range/range_errors.phpt

Lines changed: 0 additions & 134 deletions
This file was deleted.

‎ext/standard/tests/array/range/range_step_errors.phpt

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
--TEST--
2+
range() programattic errors for the $step parameter
3+
--INI--
4+
precision=14
5+
--FILE--
6+
<?php
7+
echo "Step cannot be 0\n";
8+
try {
9+
var_dump( range(1.0, 7.0, 0.0) );
10+
} catch (\ValueError $e) {
11+
echo $e->getMessage(), "\n";
12+
}
13+
try {
14+
var_dump( range(1, 7, 0) );
15+
} catch (\ValueError $e) {
16+
echo $e->getMessage(), "\n";
17+
}
18+
try {
19+
var_dump( range('A', 'H', 0) );
20+
} catch (\ValueError $e) {
21+
echo $e->getMessage(), "\n";
22+
}
23+
try {
24+
var_dump( range('A', 'H', 0.0) );
25+
} catch (\ValueError $e) {
26+
echo $e->getMessage(), "\n";
27+
}
28+
29+
echo "Step must be within the range of input parameters\n";
30+
echo "-- Testing ( (low < high) && (high-low < step) ) --\n";
31+
try {
32+
var_dump( range(1.0, 7.0, 6.5) );
33+
} catch (\ValueError $e) {
34+
echo $e->getMessage(), "\n";
35+
}
36+
37+
echo "-- Testing ( (low > high) && (low-high < step) ) --\n";
38+
try {
39+
var_dump( range(7.0, 1.0, 6.5) );
40+
} catch (\ValueError $e) {
41+
echo $e->getMessage(), "\n";
42+
}
43+
44+
echo "-- Testing ( (low < high) && (high-low < step) ) for characters --\n";
45+
try {
46+
var_dump(range('a', 'z', 100));
47+
} catch (\ValueError $e) {
48+
echo $e->getMessage(), "\n";
49+
}
50+
51+
echo "-- Testing ( (low > high) && (low-high < step) ) for characters --\n";
52+
try {
53+
var_dump(range('z', 'a', 100));
54+
} catch (\ValueError $e) {
55+
echo $e->getMessage(), "\n";
56+
}
57+
58+
?>
59+
--EXPECT--
60+
Step cannot be 0
61+
range(): Argument #3 ($step) cannot be 0
62+
range(): Argument #3 ($step) cannot be 0
63+
range(): Argument #3 ($step) cannot be 0
64+
range(): Argument #3 ($step) cannot be 0
65+
Step must be within the range of input parameters
66+
-- Testing ( (low < high) && (high-low < step) ) --
67+
range(): Argument #3 ($step) must not exceed the specified range
68+
-- Testing ( (low > high) && (low-high < step) ) --
69+
range(): Argument #3 ($step) must not exceed the specified range
70+
-- Testing ( (low < high) && (high-low < step) ) for characters --
71+
range(): Argument #3 ($step) must not exceed the specified range
72+
-- Testing ( (low > high) && (low-high < step) ) for characters --
73+
range(): Argument #3 ($step) must not exceed the specified range

0 commit comments

Comments
 (0)