diff --git a/ext/session/session.c b/ext/session/session.c index dd968d453bda..2a95ab1dc811 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -865,16 +865,44 @@ static PHP_INI_MH(OnUpdateSessionDivisor) return SUCCESS; } +static bool php_session_is_ini_whitespace(char c) +{ + return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\v' || c == '\f'; +} + static PHP_INI_MH(OnUpdateRfc1867Freq) { - int new_freq = ZEND_ATOL(ZSTR_VAL(new_value)); + const char *str = ZSTR_VAL(new_value); + size_t len = ZSTR_LEN(new_value); + + /* Ignore trailing whitespace so it doesn't hide the '%' suffix (leading + * whitespace is already tolerated by is_numeric_string_ex() below). */ + while (len > 0 && php_session_is_ini_whitespace(str[len - 1])) { + len--; + } + + bool is_percentage = len > 0 && str[len - 1] == '%'; + size_t numeric_len = is_percentage ? len - 1 : len; + + /* Whitespace directly before the '%' (e.g. "5 %") is rejected. */ + if (is_percentage && numeric_len > 0 && php_session_is_ini_whitespace(str[numeric_len - 1])) { + php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be of type int"); + return FAILURE; + } + + zend_long new_freq = 0; + uint8_t type = is_numeric_string_ex(str, numeric_len, &new_freq, NULL, false, NULL, NULL); + if (type != IS_LONG) { + php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be of type int"); + return FAILURE; + } if (new_freq < 0) { php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be greater than or equal to 0"); return FAILURE; } - if (ZSTR_LEN(new_value) > 0 && ZSTR_VAL(new_value)[ZSTR_LEN(new_value) - 1] == '%') { + if (is_percentage) { if (new_freq > 100) { php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be less than or equal to 100%%"); return FAILURE; @@ -884,6 +912,12 @@ static PHP_INI_MH(OnUpdateRfc1867Freq) PS(rfc1867_freq) = new_freq; } + /* Store a whitespace-free canonical value so ini_get() doesn't echo back stray whitespace. */ + char buf[MAX_LENGTH_OF_LONG + 2]; + int normalized_len = snprintf(buf, sizeof(buf), is_percentage ? ZEND_LONG_FMT "%%" : ZEND_LONG_FMT, new_freq); + entry->value = zend_string_init(buf, (size_t) normalized_len, true); + GC_MAKE_PERSISTENT_LOCAL(entry->value); + return SUCCESS; } diff --git a/ext/session/tests/rfc1867_freq_leading_whitespace.phpt b/ext/session/tests/rfc1867_freq_leading_whitespace.phpt new file mode 100644 index 000000000000..65396e73bbc7 --- /dev/null +++ b/ext/session/tests/rfc1867_freq_leading_whitespace.phpt @@ -0,0 +1,17 @@ +--TEST-- +session rfc1867 freq: leading whitespace is tolerated and stripped from the stored value +--INI-- +session.upload_progress.freq=" 5%" +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECT-- +string(2) "5%" diff --git a/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt b/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt new file mode 100644 index 000000000000..b12d2b87afac --- /dev/null +++ b/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt @@ -0,0 +1,17 @@ +--TEST-- +session rfc1867 freq: trailing whitespace after the "%" suffix is tolerated +--INI-- +session.upload_progress.freq="5% " +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECT-- +string(2) "5%" diff --git a/ext/session/tests/rfc1867_invalid_settings_3.phpt b/ext/session/tests/rfc1867_invalid_settings_3.phpt new file mode 100644 index 000000000000..f8f998f43c6e --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_3.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 3: non-numeric freq is rejected instead of silently truncated +--INI-- +session.upload_progress.freq=5 apples +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" diff --git a/ext/session/tests/rfc1867_invalid_settings_4.phpt b/ext/session/tests/rfc1867_invalid_settings_4.phpt new file mode 100644 index 000000000000..d51a1538ad8a --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_4.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 4: non-numeric freq in percentage mode is rejected instead of silently truncated +--INI-- +session.upload_progress.freq=5 apples% +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" diff --git a/ext/session/tests/rfc1867_invalid_settings_5.phpt b/ext/session/tests/rfc1867_invalid_settings_5.phpt new file mode 100644 index 000000000000..71e467e37911 --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_5.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 5: whitespace-only freq is rejected +--INI-- +session.upload_progress.freq=" " +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" diff --git a/ext/session/tests/rfc1867_invalid_settings_6.phpt b/ext/session/tests/rfc1867_invalid_settings_6.phpt new file mode 100644 index 000000000000..2b4e7f53262d --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_6.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 6: whitespace between the number and the "%" suffix is rejected +--INI-- +session.upload_progress.freq="5 %" +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%"