Skip to content

Commit e2b9149

Browse files
committed
Address some comments
1 parent dac7178 commit e2b9149

11 files changed

+130
-31
lines changed

Zend/tests/zend_ini_parse_quantity_error.phpt renamed to Zend/tests/zend_ini/zend_ini_parse_quantity_error.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ $tests = [
1313
'1X', # Unknown multiplier.
1414
'1.0K', # Non integral digits.
1515

16+
'0X', # Valid prefix with no value
17+
'0Z', # Invalid prefix
18+
'0XK', # Valid prefix with no value and multiplier
19+
1620
# Null bytes
1721
" 123\x00K",
1822
"\x00 123K",
@@ -48,6 +52,21 @@ int(1)
4852
Warning: Invalid quantity "1.0K", interpreting as "1K" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
4953
int(1024)
5054

55+
# "0X"
56+
57+
Warning: Invalid quantity: no digits after base prefix, interpreting as "0" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
58+
int(0)
59+
60+
# "0Z"
61+
62+
Warning: Invalid prefix "0Z", interpreting as "0" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
63+
int(0)
64+
65+
# "0XK"
66+
67+
Warning: Invalid quantity "0XK": no valid leading digits, interpreting as "0" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
68+
int(0)
69+
5170
# " 123\000K"
5271

5372
Warning: Invalid quantity " 123\x00K", interpreting as " 123K" for backwards compatibility in %s on line %d
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
--TEST--
2+
Test parsing of valid 0 quantities
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
8+
$tests = [
9+
'0',
10+
'0K',
11+
'0k',
12+
'0M',
13+
'0m',
14+
'0G',
15+
'0g',
16+
'-0',
17+
'-0K',
18+
'-0k',
19+
'-0M',
20+
'-0m',
21+
'-0G',
22+
'-0g',
23+
];
24+
25+
foreach ($tests as $setting) {
26+
printf("# \"%s\"\n", addcslashes($setting, "\0..\37!@\177..\377"));
27+
var_dump(zend_test_zend_ini_parse_quantity($setting));
28+
print "\n";
29+
}
30+
--EXPECT--
31+
# "0"
32+
int(0)
33+
34+
# "0K"
35+
int(0)
36+
37+
# "0k"
38+
int(0)
39+
40+
# "0M"
41+
int(0)
42+
43+
# "0m"
44+
int(0)
45+
46+
# "0G"
47+
int(0)
48+
49+
# "0g"
50+
int(0)
51+
52+
# "-0"
53+
int(0)
54+
55+
# "-0K"
56+
int(0)
57+
58+
# "-0k"
59+
int(0)
60+
61+
# "-0M"
62+
int(0)
63+
64+
# "-0m"
65+
int(0)
66+
67+
# "-0G"
68+
int(0)
69+
70+
# "-0g"
71+
int(0)

Zend/zend_ini.c

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "zend_strtod.h"
2626
#include "zend_modules.h"
2727
#include "zend_smart_str.h"
28+
#include <ctype.h>
2829

2930
static HashTable *registered_zend_ini_directives;
3031

@@ -552,7 +553,6 @@ static zend_ulong zend_ini_parse_quantity_internal(zend_string *value, zend_ini_
552553
char *str = ZSTR_VAL(value);
553554
char *str_end = &str[ZSTR_LEN(value)];
554555
char *digits = str;
555-
size_t digits_len = ZSTR_LEN(value);
556556
bool overflow = false;
557557
zend_ulong factor;
558558
smart_str invalid = {0};
@@ -561,10 +561,10 @@ static zend_ulong zend_ini_parse_quantity_internal(zend_string *value, zend_ini_
561561

562562
/* Ignore leading whitespace. ZEND_STRTOL() also skips leading whitespaces,
563563
* but we need the position of the first non-whitespace later. */
564-
while (digits < str_end && zend_is_whitespace(*digits)) {++digits; --digits_len;}
564+
while (digits < str_end && zend_is_whitespace(*digits)) {++digits;}
565565

566566
/* Ignore trailing whitespace */
567-
while (digits < str_end && zend_is_whitespace(*(str_end-1))) {--str_end; --digits_len;}
567+
while (digits < str_end && zend_is_whitespace(*(str_end-1))) {--str_end;}
568568

569569
if (digits == str_end) {
570570
*errstr = NULL;
@@ -574,69 +574,78 @@ static zend_ulong zend_ini_parse_quantity_internal(zend_string *value, zend_ini_
574574
bool is_negative = false;
575575
if (digits[0] == '+') {
576576
++digits;
577-
--digits_len;
578577
} else if (digits[0] == '-') {
579578
is_negative = true;
580579
++digits;
581-
--digits_len;
582580
}
583581

582+
/* TODO Handle cases such as ++25, --25, or white space after sign symbol?
583+
if (!isdigit(digits[0])) {
584+
585+
}
586+
*/
587+
584588
int base = 0;
585-
if (digits_len >= 2 && digits[0] == '0') {
589+
if (digits[0] == '0' && !isdigit(digits[1])) {
590+
/* Value is just 0 */
591+
if ((digits+1) == str_end) {
592+
*errstr = NULL;
593+
return 0;
594+
}
595+
586596
switch (digits[1]) {
597+
/* Multiplier suffixes */
598+
case 'g':
599+
case 'G':
600+
case 'm':
601+
case 'M':
602+
case 'k':
603+
case 'K':
604+
goto evaluation;
587605
case 'x':
588606
case 'X':
589607
base = 16;
590-
digits += 2;
591-
digits_len -= 2;
592608
break;
593609
case 'o':
594610
case 'O':
595611
base = 8;
596-
digits += 2;
597-
digits_len -= 2;
598612
break;
599613
case 'b':
600614
case 'B':
601615
base = 2;
602-
digits += 2;
603-
digits_len -= 2;
604616
break;
617+
default:
618+
*errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility",
619+
digits[1]);
620+
return 0;
605621
}
606-
/* TODO Error for invalid prefix? */
607-
if (digits == str_end) {
608-
*errstr = NULL;
622+
digits += 2;
623+
if (UNEXPECTED(digits == str_end)) {
624+
*errstr = zend_strpprintf(0, "Invalid quantity: no digits after base prefix, interpreting as \"0\" for backwards compatibility");
609625
return 0;
610626
}
611627
}
628+
evaluation:
612629

613-
zend_ulong retval;
614630
errno = 0;
615-
616-
if (signed_result == ZEND_INI_PARSE_QUANTITY_SIGNED) {
617-
retval = (zend_ulong) ZEND_STRTOL(digits, &digits_end, base);
618-
} else {
619-
retval = ZEND_STRTOUL(digits, &digits_end, base);
620-
}
631+
zend_ulong retval = ZEND_STRTOUL(digits, &digits_end, base);
621632

622633
if (errno == ERANGE) {
623634
overflow = true;
624635
} else if (signed_result == ZEND_INI_PARSE_QUANTITY_UNSIGNED) {
625-
/* ZEND_STRTOUL() does not report a range error when the subject starts
626-
* with a minus sign, so we check this here. Ignore "-1" as it is
627-
* commonly used as max value, for instance in memory_limit=-1. */
628636
if (is_negative) {
629-
if (digits_end - digits == 1 && digits_end == str_end && digits[0] == '1') {
637+
/* Ignore "-1" as it is commonly used as max value, for instance in memory_limit=-1. */
638+
if (retval == 1 && digits_end == str_end) {
630639
retval = -1;
631640
} else {
632641
overflow = true;
633642
}
634643
}
635-
}
636-
637-
if (signed_result == ZEND_INI_PARSE_QUANTITY_SIGNED) {
638-
if (is_negative) {
639-
retval = (zend_ulong) (-1 * (zend_long) retval);
644+
} else if (signed_result == ZEND_INI_PARSE_QUANTITY_SIGNED) {
645+
if ((zend_long) retval < 0) {
646+
overflow = true;
647+
} else if (is_negative) {
648+
retval = 0u - retval;
640649
}
641650
}
642651

0 commit comments

Comments
 (0)