r82015 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r82014‎ \| r82015 \| r82016 >
Date:	14:49, 12 February 2011
Author:	platonides
Status:	ok
Tags:
Comment:	Removed some leaks (internal to the request). Fixed ZTS errors. Removed race condition in multithread. Removed unused static le_wmerrors (unused in both thread modes) php_wmerrors_init_globals was never called Enabled the error page for other sapis other than cli, not just apache. Removed showing the backtrace in the error page. It wasn't properly html escaped, it is a lot of ugly information useless for the user (2519 lines for an error in Parser::parse), and could contain sensitive data. Whether to include backtraces in the log is now configurable. Follow up r81991,r82012
Modified paths:	/trunk/php/wmerrors/php_wmerrors.h (modified) (history) /trunk/php/wmerrors/wmerrors.c (modified) (history)

Diff [purge]

Index: trunk/php/wmerrors/wmerrors.c
—	—	@@ -9,8 +9,10 @@
10	10	#include "php_wmerrors.h"
11	11	#include "php_streams.h" /* for __php_stream_call_depth */
12	12	#include "SAPI.h" /* for sapi_module */
	13	+#include "ext/date/php_date.h" /* for php_format_date */
13	14	#include "ext/standard/php_smart_str.h" /* for smart_str */
14	15	#include "ext/standard/html.h" /* for php_escape_html_entities */
	16	+#include "Zend/zend_builtin_functions.h" /* for zend_fetch_debug_backtrace */
15	17
16	18	void wmerrors_cb(int type, const char error_filename, const uint error_lineno, const char format, va_list args);
17	19	static void wmerrors_show_message(int type, const char error_filename, const uint error_lineno, const char format, va_list args TSRMLS_DC);
—	—	@@ -18,8 +20,6 @@
19	21
20	22	ZEND_DECLARE_MODULE_GLOBALS(wmerrors)
21	23
22		~~-static int le_wmerrors;~~
23		-
24	24	zend_function_entry wmerrors_functions[] = {
25	25	{NULL, NULL, NULL}
26	26	};
—	—	@@ -37,7 +37,7 @@
38	38	PHP_RSHUTDOWN(wmerrors),
39	39	PHP_MINFO(wmerrors),
40	40	#if ZEND_MODULE_API_NO >= 20010901
41		~~- "0.1",~~
	41	+ "0.2",
42	42	#endif
43	43	STANDARD_MODULE_PROPERTIES
44	44	};
—	—	@@ -51,20 +51,23 @@
52	52	STD_PHP_INI_BOOLEAN("wmerrors.enabled", "0", PHP_INI_ALL, OnUpdateBool, enabled, zend_wmerrors_globals, wmerrors_globals )
53	53	STD_PHP_INI_ENTRY("wmerrors.message_file", "", PHP_INI_ALL, OnUpdateString, message_file, zend_wmerrors_globals, wmerrors_globals)
54	54	STD_PHP_INI_ENTRY("wmerrors.logging_file", "", PHP_INI_ALL, OnUpdateString, logging_file, zend_wmerrors_globals, wmerrors_globals)
	55	+ STD_PHP_INI_ENTRY("wmerrors.log_level", "0", PHP_INI_ALL, OnUpdateLong, log_level, zend_wmerrors_globals, wmerrors_globals)
55	56	PHP_INI_END()
56	57
	58	+void (old_error_cb)(int type, const char error_filename, const uint error_lineno, const char *format, va_list args);
	59	+
57	60	static void php_wmerrors_init_globals(zend_wmerrors_globals *wmerrors_globals)
58	61	{
59	62	wmerrors_globals->message_file = NULL;
60	63	wmerrors_globals->logging_file = NULL;
61		~~- wmerrors_globals->logfile_stream = NULL;~~
	64	+ wmerrors_globals->log_level = 0;
62	65	}
63	66
64	67	PHP_MINIT_FUNCTION(wmerrors)
65	68	{
	69	+ ZEND_INIT_MODULE_GLOBALS(wmerrors, php_wmerrors_init_globals, NULL);
66	70	REGISTER_INI_ENTRIES();
67		~~- WMERRORS_G(old_error_cb) = zend_error_cb;~~
68		~~- WMERRORS_G(recursion_guard) = 0;~~
	71	+ old_error_cb = zend_error_cb;
69	72	zend_error_cb = wmerrors_cb;
70	73	return SUCCESS;
71	74	}
—	—	@@ -73,7 +76,7 @@
74	77	PHP_MSHUTDOWN_FUNCTION(wmerrors)
75	78	{
76	79	UNREGISTER_INI_ENTRIES();
77		~~- zend_error_cb = WMERRORS_G(old_error_cb);~~
	80	+ zend_error_cb = old_error_cb;
78	81	return SUCCESS;
79	82	}
80	83
—	—	@@ -81,6 +84,7 @@
82	85
83	86	PHP_RINIT_FUNCTION(wmerrors)
84	87	{
	88	+ WMERRORS_G(recursion_guard) = 0;
85	89	return SUCCESS;
86	90	}
87	91
—	—	@@ -91,7 +95,6 @@
92	96	return SUCCESS;
93	97	}
94	98
95		-
96	99	PHP_MINFO_FUNCTION(wmerrors)
97	100	{
98	101	php_info_print_table_start();
—	—	@@ -107,6 +110,9 @@
108	111	#define WM_ERROR_HANDLING PG(error_handling)
109	112	#endif
110	113
	114	+static const char* error_type_to_string(int type);
	115	+static void wmerrors_log_error(int type, const char error_filename, const uint error_lineno, const char format, va_list args TSRMLS_DC);
	116	+
111	117	void wmerrors_cb(int type, const char error_filename, const uint error_lineno, const char format, va_list args)
112	118	{
113	119	TSRMLS_FETCH();
—	—	@@ -122,33 +128,82 @@
123	129	\|\| (type == E_RECOVERABLE_ERROR && WM_ERROR_HANDLING == EH_THROW && !EG(exception))
124	130	\|\| (type != E_ERROR && type != E_CORE_ERROR && type != E_COMPILE_ERROR
125	131	&& type != E_USER_ERROR && type != E_RECOVERABLE_ERROR)
126		~~- \|\| strncmp(sapi_module.name, "apache", 6)~~
127	132	\|\| WMERRORS_G(recursion_guard))
128	133	{
129		~~- WMERRORS_G(old_error_cb)(type, error_filename, error_lineno, format, args);~~
	134	+ old_error_cb(type, error_filename, error_lineno, format, args);
130	135	return;
131	136	}
132	137	WMERRORS_G(recursion_guard) = 1;
133	138	/* No more OOM errors for now thanks */
134	139	zend_set_memory_limit((size_t)-1);
135	140
136		~~- /* Show the message */~~
137		~~- wmerrors_show_message(type, error_filename, error_lineno, format, args TSRMLS_CC);~~
	141	+ if ( WMERRORS_G(enabled) && strncmp(sapi_module.name, "cli", 3) ) {
	142	+ /* Show the message */
	143	+ wmerrors_show_message(type, error_filename, error_lineno, format, args TSRMLS_CC);
	144	+ }
138	145
139		~~- /* TODO: improved logging */~~
	146	+ if ( WMERRORS_G(enabled) && WMERRORS_G(log_level) ) {
	147	+ /* Log the error */
	148	+ wmerrors_log_error(type, error_filename, error_lineno, format, args TSRMLS_CC);
	149	+ }
140	150
141	151	WMERRORS_G(recursion_guard) = 0;
142	152	zend_set_memory_limit(PG(memory_limit));
143	153
144	154	/* Pass through */
145		~~- WMERRORS_G(old_error_cb)(type, error_filename, error_lineno, format, args);~~
	155	+ old_error_cb(type, error_filename, error_lineno, format, args);
146	156	}
147	157
	158	+/* Callback for zend_print_zval_r_ex()
	159	+ * Gets the file to write from the module global logfile_stream.
	160	+ */
148	161	static int wmerrors_write_trace(const char *str, uint str_length) {
149		~~- php_write((void *)str, str_length TSRMLS_CC);~~
150		~~- php_stream_write(WMERRORS_G(logfile_stream), str, str_length TSRMLS_CC);~~
	162	+ TSRMLS_FETCH();
	163	+ return php_stream_write(WMERRORS_G(logfile_stream), str, str_length);
151	164	}
152	165
	166	+static void wmerrors_log_error(int type, const char error_filename, const uint error_lineno, const char format, va_list args TSRMLS_DC) {
	167	+ char tmp1; zval trace; char *error_time_str;
	168	+ int tmp1_len; va_list my_args;
	169	+
	170	+ if ( !WMERRORS_G(enabled) \|\| !WMERRORS_G(log_level) ) {
	171	+ /* Redundant with the caller */
	172	+ return;
	173	+ }
	174	+
	175	+ if ( !WMERRORS_G(logging_file) \|\| *WMERRORS_G(logging_file) == '\0') {
	176	+ /* No log file configured */
	177	+ return;
	178	+ }
	179	+
	180	+ /* Try opening the logging file */
	181	+ WMERRORS_G(logfile_stream) = php_stream_open_wrapper(WMERRORS_G(logging_file), "ab", ENFORCE_SAFE_MODE \| REPORT_ERRORS, NULL);
	182	+ if ( !WMERRORS_G(logfile_stream) ) {
	183	+ return;
	184	+ }
	185	+
	186	+ /* Don't destroy the caller's va_list */
	187	+ va_copy(my_args, args);
	188	+ tmp1_len = vspprintf(&tmp1, 0, format, my_args);
	189	+ va_end(my_args);
	190	+
	191	+ /* Log the error (log_level >= 1) */
	192	+ error_time_str = php_format_date("d-M-Y H:i:s", 11, time(NULL), 0 TSRMLS_CC);
	193	+ php_stream_printf(WMERRORS_G(logfile_stream) TSRMLS_CC, "[%s UTC] %s: %.*s at %s on line %u%s", error_time_str, error_type_to_string(type), tmp1_len, tmp1, error_filename, error_lineno, PHP_EOL);
	194	+ efree(error_time_str);
	195	+ efree(tmp1);
	196	+
	197	+ /* Write a backtrace */
	198	+ if ( WMERRORS_G(log_level) >= 2 ) {
	199	+ ALLOC_INIT_ZVAL(trace);
	200	+ zend_fetch_debug_backtrace(trace, 0, 0 TSRMLS_CC);
	201	+ zend_print_zval_r_ex(wmerrors_write_trace, trace, 4 TSRMLS_CC);
	202	+ FREE_ZVAL(trace);
	203	+ }
	204	+
	205	+ php_stream_close( WMERRORS_G(logfile_stream) );
	206	+}
	207	+
153	208	static void wmerrors_show_message(int type, const char error_filename, const uint error_lineno, const char format, va_list args TSRMLS_DC)
154	209	{
155	210	php_stream *stream;
—	—	@@ -159,7 +214,6 @@
160	215	int tmp1_len, tmp2_len;
161	216	smart_str expanded = {0};
162	217	va_list my_args;
163		~~- zval *trace;~~
164	218
165	219	/* Is there a sane message_file? */
166	220	if (!WMERRORS_G(message_file) \|\| *WMERRORS_G(message_file) == '\0') {
—	—	@@ -172,14 +226,6 @@
173	227	if (!stream) {
174	228	return;
175	229	}
176		-
177		~~- /* Try opening the logging file */~~
178		~~- WMERRORS_G(logfile_stream) = NULL;~~
179		~~- if (WMERRORS_G(logging_file) && *WMERRORS_G(logging_file) != '\0')~~
180		~~- {~~
181		~~- WMERRORS_G(logfile_stream) = php_stream_open_wrapper(WMERRORS_G(logging_file), "ab",~~
182		~~- ENFORCE_SAFE_MODE \| REPORT_ERRORS, NULL);~~
183		~~- }~~
184	230
185	231	/* Don't destroy the caller's va_list */
186	232	va_copy(my_args, args);
—	—	@@ -230,10 +276,6 @@
231	277
232	278	/* Write the message out */
233	279	if (expanded.c) {
234		~~- /php_write(expanded.c, expanded.len TSRMLS_CC);/~~
235		~~- if (WMERRORS_G(logfile_stream)) {~~
236		~~- php_stream_write(WMERRORS_G(logfile_stream), expanded.c, expanded.len TSRMLS_CC);~~
237		~~- }~~
238	280	php_write(expanded.c, expanded.len TSRMLS_CC);
239	281	}
240	282
—	—	@@ -241,13 +283,33 @@
242	284	smart_str_free(&expanded);
243	285	efree(message);
244	286	va_end(my_args);
	287	+}
	288	+
	289	+static const char* error_type_to_string(int type) {
	290	+ int i;
	291	+ #define ErrorType(x) {x, #x}
	292	+ static struct { int type; const char* name; } error_names[] = {
	293	+ ErrorType(E_ERROR),
	294	+ ErrorType(E_CORE_ERROR),
	295	+ ErrorType(E_COMPILE_ERROR),
	296	+ ErrorType(E_USER_ERROR),
	297	+ ErrorType(E_RECOVERABLE_ERROR),
	298	+ ErrorType(E_WARNING),
	299	+ ErrorType(E_CORE_WARNING),
	300	+ ErrorType(E_COMPILE_WARNING),
	301	+ ErrorType(E_USER_WARNING),
	302	+ ErrorType(E_PARSE),
	303	+ ErrorType(E_NOTICE),
	304	+ ErrorType(E_USER_NOTICE),
	305	+ ErrorType(E_STRICT),
	306	+ ErrorType(E_DEPRECATED),
	307	+ ErrorType(E_USER_DEPRECATED)
	308	+ };
245	309
246		~~- /* Write a backtrace */~~
247		~~- ALLOC_ZVAL(trace);~~
248		~~- Z_UNSET_ISREF_P(trace);~~
249		~~- Z_SET_REFCOUNT_P(trace, 0);~~
250		~~- zend_fetch_debug_backtrace(trace, 0, 0 TSRMLS_CC);~~
251		~~- zend_print_zval_r_ex(wmerrors_write_trace, trace, 4);~~
	310	+ for (i=0; i < sizeof(error_names)/sizeof(error_names[0]); i++) {
	311	+ if (type == error_names[i].type) {
	312	+ return error_names[i].name;
	313	+ }
	314	+ }
	315	+ return "Unknown error";
252	316	}
253		-
254		-
Index: trunk/php/wmerrors/php_wmerrors.h
—	—	@@ -22,11 +22,11 @@
23	23	PHP_MINFO_FUNCTION(wmerrors);
24	24
25	25	ZEND_BEGIN_MODULE_GLOBALS(wmerrors)
26		~~- void (old_error_cb)(int type, const char error_filename, const uint error_lineno, const char *format, va_list args);~~
27	26	char * message_file;
28	27	char * logging_file;
29	28	int recursion_guard;
30	29	int enabled;
	30	+ long int log_level;
31	31	php_stream *logfile_stream;
32	32	ZEND_END_MODULE_GLOBALS(wmerrors)
33	33

Follow-up revisions

Revision	Commit summary	Author	Date
r82019	Follow up r82015. Add support for logging by tcp....	platonides	16:22, 12 February 2011

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r81991	wmerrors: Add capability for logging to a file. Implementing UDP logging woul...	catrope	20:25, 11 February 2011
r82012	wmerrors: Add basic support for outputting backtraces. Because PHP is very un...	catrope	12:34, 12 February 2011

Status & tagging log

11:32, 13 February 2011 Catrope (talk | contribs) changed the status of r82015 [removed: new added: ok]