Skip to content

Commit cd75300

Browse files
authored
Audit INI functions and macros, and replace them with better alternatives (#21146)
Audit zend_ini_string(), INI_STR(), and related functions and macros, and all other INI_* macros. The primary motivation is that these APIs should return a `const char*` because the `char*` is owned by a zend_string, and thus should never be released. Moreover, the INI_STR() API doesn't follow our usual naming scheme as it returns a char* rather than a zend_string*. In this PR new zend_ini_{bool|long|double|str|string}_literal() macros are introduced replacing the INI_{BOOL|INT|FLT|STR} macros which follow our more modern naming convention. Moreover, the INI_BOOL() macro didn't produce correct values if the INI string is a word like "true" or "on". The INI_ORIG_* APIs are removed because a Sourcegraph search shows 0 results, other than the one case used in ext/tidy that we fix using the typical API. Add some additional checks for the value of an INI string to ensure it doesn't contain a nul byte when describing a path.
1 parent c3a1214 commit cd75300

File tree

27 files changed

+103
-102
lines changed

27 files changed

+103
-102
lines changed

UPGRADING.INTERNALS

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ PHP 8.6 INTERNALS UPGRADE NOTES
7575
. Added a C23_ENUM() helper macro to define forward-compatible fixed-size
7676
enums.
7777
. Extended php_stream_filter_ops with seek method.
78+
. The INI_STR(), INI_INT(), INI_FLT(), and INI_BOOL() macros have been
79+
removed. Instead new zend_ini_{bool|long|double|str|string}_literal()
80+
macros have been added. This fixes an internal naming inconsistency as
81+
"str" usually means zend_string*, and "string" means char*.
82+
However INI_STR() returned a char*
83+
. The INI_ORIG_{INT|STR|FLT|BOOL}() macros have been removed as they are
84+
unused. If this behaviour is required fall back to the zend_ini_*
85+
functions.
7886

7987
========================
8088
2. Build system changes

Zend/zend_fibers.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,9 @@ static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer)
572572
zend_fiber *fiber = EG(active_fiber);
573573

574574
/* Determine the current error_reporting ini setting. */
575-
zend_long error_reporting = INI_INT("error_reporting");
576-
/* If error_reporting is 0 and not explicitly set to 0, INI_STR returns a null pointer. */
577-
if (!error_reporting && !INI_STR("error_reporting")) {
575+
zend_long error_reporting = zend_ini_long_literal("error_reporting");
576+
/* If error_reporting is 0 and not explicitly set to 0, zend_ini_str returns a null pointer. */
577+
if (!error_reporting && !zend_ini_str_literal("error_reporting")) {
578578
error_reporting = E_ALL;
579579
}
580580

Zend/zend_highlight.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ ZEND_API void zend_highlight(zend_syntax_highlighter_ini *syntax_highlighter_ini
7979
{
8080
zval token;
8181
int token_type;
82-
char *last_color = syntax_highlighter_ini->highlight_html;
83-
char *next_color;
82+
const char *last_color = syntax_highlighter_ini->highlight_html;
83+
const char *next_color;
8484

8585
zend_printf("<pre><code style=\"color: %s\">", last_color);
8686
/* highlight stuff coming back from zendlex() */

Zend/zend_highlight.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@
3030

3131

3232
typedef struct _zend_syntax_highlighter_ini {
33-
char *highlight_html;
34-
char *highlight_comment;
35-
char *highlight_default;
36-
char *highlight_string;
37-
char *highlight_keyword;
33+
const char *highlight_html;
34+
const char *highlight_comment;
35+
const char *highlight_default;
36+
const char *highlight_string;
37+
const char *highlight_keyword;
3838
} zend_syntax_highlighter_ini;
3939

4040

Zend/zend_ini.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -491,15 +491,15 @@ ZEND_API double zend_ini_double(const char *name, size_t name_length, bool orig)
491491
}
492492
/* }}} */
493493

494-
ZEND_API char *zend_ini_string_ex(const char *name, size_t name_length, bool orig, bool *exists) /* {{{ */
494+
ZEND_API const char *zend_ini_string_ex(const char *name, size_t name_length, bool orig, bool *exists) /* {{{ */
495495
{
496496
zend_string *str = zend_ini_str_ex(name, name_length, orig, exists);
497497

498498
return str ? ZSTR_VAL(str) : NULL;
499499
}
500500
/* }}} */
501501

502-
ZEND_API char *zend_ini_string(const char *name, size_t name_length, bool orig) /* {{{ */
502+
ZEND_API const char *zend_ini_string(const char *name, size_t name_length, bool orig) /* {{{ */
503503
{
504504
zend_string *str = zend_ini_str(name, name_length, orig);
505505

Zend/zend_ini.h

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,19 @@ ZEND_API void display_ini_entries(zend_module_entry *module);
8888

8989
ZEND_API zend_long zend_ini_long(const char *name, size_t name_length, bool orig);
9090
ZEND_API double zend_ini_double(const char *name, size_t name_length, bool orig);
91-
ZEND_API char *zend_ini_string(const char *name, size_t name_length, bool orig);
92-
ZEND_API char *zend_ini_string_ex(const char *name, size_t name_length, bool orig, bool *exists);
91+
ZEND_API const char *zend_ini_string(const char *name, size_t name_length, bool orig);
92+
ZEND_API const char *zend_ini_string_ex(const char *name, size_t name_length, bool orig, bool *exists);
9393
ZEND_API zend_string *zend_ini_str(const char *name, size_t name_length, bool orig);
9494
ZEND_API zend_string *zend_ini_str_ex(const char *name, size_t name_length, bool orig, bool *exists);
9595
ZEND_API zend_string *zend_ini_get_value(zend_string *name);
9696
ZEND_API bool zend_ini_parse_bool(const zend_string *str);
9797

98+
#define zend_ini_bool_literal(name) zend_ini_parse_bool(zend_ini_str((name), sizeof("" name) - 1, false))
99+
#define zend_ini_long_literal(name) zend_ini_long((name), sizeof("" name) - 1, false)
100+
#define zend_ini_double_literal(name) zend_ini_double((name), sizeof("" name) - 1, false)
101+
#define zend_ini_str_literal(name) zend_ini_str((name), sizeof("" name) - 1, false)
102+
#define zend_ini_string_literal(name) zend_ini_string((name), sizeof("" name) - 1, false)
103+
98104
/**
99105
* Parses an ini quantity
100106
*
@@ -191,16 +197,6 @@ END_EXTERN_C()
191197
ZEND_INI_ENTRY3_EX(name, default_value, modifiable, on_modify, (void *) XtOffsetOf(struct_type, property_name), (void *) &struct_ptr, NULL, zend_ini_boolean_displayer_cb)
192198
#endif
193199

194-
#define INI_INT(name) zend_ini_long((name), strlen(name), 0)
195-
#define INI_FLT(name) zend_ini_double((name), strlen(name), 0)
196-
#define INI_STR(name) zend_ini_string_ex((name), strlen(name), 0, NULL)
197-
#define INI_BOOL(name) ((bool) INI_INT(name))
198-
199-
#define INI_ORIG_INT(name) zend_ini_long((name), strlen(name), 1)
200-
#define INI_ORIG_FLT(name) zend_ini_double((name), strlen(name), 1)
201-
#define INI_ORIG_STR(name) zend_ini_string((name), strlen(name), 1)
202-
#define INI_ORIG_BOOL(name) ((bool) INI_ORIG_INT(name))
203-
204200
#define REGISTER_INI_ENTRIES() zend_register_ini_entries_ex(ini_entries, module_number, type)
205201
#define UNREGISTER_INI_ENTRIES() zend_unregister_ini_entries_ex(module_number, type)
206202
#define DISPLAY_INI_ENTRIES() display_ini_entries(zend_module)

Zend/zend_multibyte.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ ZEND_API zend_result zend_multibyte_set_functions(const zend_multibyte_functions
114114
* populated, we need to reinitialize script_encoding here.
115115
*/
116116
{
117-
const char *value = zend_ini_string("zend.script_encoding", sizeof("zend.script_encoding") - 1, 0);
118-
zend_multibyte_set_script_encoding_by_string(value, strlen(value));
117+
const zend_string *value = zend_ini_str_literal("zend.script_encoding");
118+
zend_multibyte_set_script_encoding_by_string(ZSTR_VAL(value), ZSTR_LEN(value));
119119
}
120120
return SUCCESS;
121121
}

ext/com_dotnet/com_dotnet.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ static HRESULT dotnet_bind_runtime(LPVOID FAR *ppv)
127127
typedef HRESULT (STDAPICALLTYPE *cbtr_t)(LPCWSTR pwszVersion, LPCWSTR pwszBuildFlavor, REFCLSID rclsid, REFIID riid, LPVOID FAR *ppv);
128128
cbtr_t CorBindToRuntime;
129129
OLECHAR *oleversion;
130-
char *version;
131130

132131
mscoree = LoadLibraryA("mscoree.dll");
133132
if (mscoree == NULL) {
@@ -140,11 +139,11 @@ static HRESULT dotnet_bind_runtime(LPVOID FAR *ppv)
140139
return S_FALSE;
141140
}
142141

143-
version = INI_STR("com.dotnet_version");
144-
if (version == NULL || *version == '\0') {
142+
const zend_string *version = zend_ini_str_literal("com.dotnet_version");
143+
if (version == NULL || ZSTR_LEN(version) == 0) {
145144
oleversion = NULL;
146145
} else {
147-
oleversion = php_com_string_to_olestring(version, strlen(version), COMG(code_page));
146+
oleversion = php_com_string_to_olestring(ZSTR_VAL(version), ZSTR_LEN(version), COMG(code_page));
148147
}
149148

150149
hr = CorBindToRuntime(oleversion, NULL, &CLSID_CorRuntimeHost, &IID_ICorRuntimeHost, ppv);

ext/curl/interface.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,8 +1100,6 @@ static void create_certinfo(struct curl_certinfo *ci, zval *listcode)
11001100
Set default options for a handle */
11011101
static void _php_curl_set_default_options(php_curl *ch)
11021102
{
1103-
char *cainfo;
1104-
11051103
curl_easy_setopt(ch->cp, CURLOPT_NOPROGRESS, 1L);
11061104
curl_easy_setopt(ch->cp, CURLOPT_VERBOSE, 0L);
11071105
curl_easy_setopt(ch->cp, CURLOPT_ERRORBUFFER, ch->err.str);
@@ -1114,9 +1112,9 @@ static void _php_curl_set_default_options(php_curl *ch)
11141112
curl_easy_setopt(ch->cp, CURLOPT_DNS_CACHE_TIMEOUT, 120L);
11151113
curl_easy_setopt(ch->cp, CURLOPT_MAXREDIRS, 20L); /* prevent infinite redirects */
11161114

1117-
cainfo = INI_STR("openssl.cafile");
1115+
const char *cainfo = zend_ini_string_literal("openssl.cafile");
11181116
if (!(cainfo && cainfo[0] != '\0')) {
1119-
cainfo = INI_STR("curl.cainfo");
1117+
cainfo = zend_ini_string_literal("curl.cainfo");
11201118
}
11211119
if (cainfo && cainfo[0] != '\0') {
11221120
curl_easy_setopt(ch->cp, CURLOPT_CAINFO, cainfo);

ext/date/php_date.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5478,18 +5478,18 @@ static void php_do_date_sunrise_sunset(INTERNAL_FUNCTION_PARAMETERS, bool calc_s
54785478
ZEND_PARSE_PARAMETERS_END();
54795479

54805480
if (latitude_is_null) {
5481-
latitude = INI_FLT("date.default_latitude");
5481+
latitude = zend_ini_double_literal("date.default_latitude");
54825482
}
54835483

54845484
if (longitude_is_null) {
5485-
longitude = INI_FLT("date.default_longitude");
5485+
longitude = zend_ini_double_literal("date.default_longitude");
54865486
}
54875487

54885488
if (zenith_is_null) {
54895489
if (calc_sunset) {
5490-
zenith = INI_FLT("date.sunset_zenith");
5490+
zenith = zend_ini_double_literal("date.sunset_zenith");
54915491
} else {
5492-
zenith = INI_FLT("date.sunrise_zenith");
5492+
zenith = zend_ini_double_literal("date.sunrise_zenith");
54935493
}
54945494
}
54955495

0 commit comments

Comments
 (0)