It seems my last letter didn't came to the list, resending it.
So this is new version of this patch - with curl_multi_exec involved
and accordingly changed tests. Let me know what you think about it.
Additionally, I found another test case (actually two) not handled yet
(see attached test case in second file):
1) If fp of non-existant file is setup to stderr and is closed before
calling curl_close() it causes glibc memory error
2) if I additionally unlink file before curl_close() - it causes
segfault (fie not found in io_write.c or smth like that)
--
Regards,
Shein Alexey
--
Regards,
Shein Alexey
Index: trunk/ext/curl/php_curl.h
===================================================================
--- trunk/ext/curl/php_curl.h (revision 306938)
+++ trunk/ext/curl/php_curl.h (revision )
@@ -150,6 +150,7 @@
void _php_curl_cleanup_handle(php_curl *);
void _php_curl_multi_cleanup_list(void *data);
+int _php_curl_file_pointers_correct(php_curl *ch TSRMLS_DC);
/* streams support */
Index: trunk/ext/curl/multi.c
===================================================================
--- trunk/ext/curl/multi.c (revision 306938)
+++ trunk/ext/curl/multi.c (revision )
@@ -130,6 +130,16 @@
}
/* }}} */
+static long _php_curl_multi_remove_handle(php_curlm *mh, php_curl *ch)
+{
+ --ch->uses;
+
+ zend_llist_del_element( &mh->easyh, &ch,
+ (int (*)(void *, void
*)) curl_compare_resources );
+
+ return (long) curl_multi_remove_handle(mh->multi, ch->cp);
+}
+
/* {{{ proto int curl_multi_remove_handle(resource mh, resource ch)
Remove a multi handle from a set of cURL handles */
PHP_FUNCTION(curl_multi_remove_handle)
@@ -146,12 +156,7 @@
ZEND_FETCH_RESOURCE(mh, php_curlm *, &z_mh, -1,
le_curl_multi_handle_name, le_curl_multi_handle);
ZEND_FETCH_RESOURCE(ch, php_curl *, &z_ch, -1, le_curl_name, le_curl);
- --ch->uses;
-
- zend_llist_del_element( &mh->easyh, &z_ch,
- (int (*)(void *, void
*)) curl_compare_resources );
-
- RETURN_LONG((long) curl_multi_remove_handle(mh->multi, ch->cp));
+ RETURN_LONG(_php_curl_multi_remove_handle(mh, ch));
}
/* }}} */
@@ -211,6 +216,22 @@
ZEND_FETCH_RESOURCE(mh, php_curlm *, &z_mh, -1,
le_curl_multi_handle_name, le_curl_multi_handle);
+ {
+ zend_llist_position pos;
+ php_curl *ch;
+ zval *ch_zval;
+
+ /* search the list of easy handles hanging off the multi-handle
*/
+ for(ch_zval = (zval *)zend_llist_get_first_ex(&mh->easyh,
&pos); ch_zval;
+ ch_zval = (zval *)zend_llist_get_next_ex(&mh->easyh,
&pos)) {
+ ZEND_FETCH_RESOURCE(ch, php_curl *, &ch_zval, -1,
le_curl_name, le_curl);
+
+ if (!_php_curl_file_pointers_correct(ch TSRMLS_CC)) {
+ _php_curl_multi_remove_handle(mh, ch);
+ }
+ }
+ }
+
convert_to_long_ex(&z_still_running);
still_running = Z_LVAL_P(z_still_running);
result = curl_multi_perform(mh->multi, &still_running);
Index: trunk/ext/curl/tests/bug48203.phpt
===================================================================
--- trunk/ext/curl/tests/bug48203.phpt (revision 281989)
+++ trunk/ext/curl/tests/bug48203.phpt (revision )
@@ -1,5 +1,5 @@
--TEST--
-Bug #48203 (Crash when CURLOPT_STDERR is set to regular file)
+Bug #48203 (Crash when file pointers passed to curl are closed before calling
curl_exec)
--SKIPIF--
<?php
if (!extension_loaded("curl")) {
@@ -12,22 +12,68 @@
--FILE--
<?php
-$fp = fopen(dirname(__FILE__) . '/bug48203.tmp', 'w');
+function checkForClosedFilePointer($curl_option, $description) {
+ $fp = fopen(dirname(__FILE__) . '/bug48203.tmp', 'w+');
-$ch = curl_init();
+ $ch = curl_init();
+ // we also need CURLOPT_VERBOSE to be set to test CURLOPT_STDERR
properly
+ if (CURLOPT_STDERR == $curl_option) {
-curl_setopt($ch, CURLOPT_VERBOSE, 1);
+ curl_setopt($ch, CURLOPT_VERBOSE, 1);
-curl_setopt($ch, CURLOPT_STDERR, $fp);
-curl_setopt($ch, CURLOPT_URL, "");
+ }
+ if (CURLOPT_INFILE == $curl_option) {
+ curl_setopt($ch, CURLOPT_UPLOAD, 1);
+ }
+
+ curl_setopt($ch, $curl_option, $fp);
+
+ curl_setopt($ch, CURLOPT_URL, 'localhost');
+ curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+
-fclose($fp); // <-- premature close of $fp caused a crash!
+ fclose($fp); // <-- premature close of $fp caused a crash!
-curl_exec($ch);
+ curl_exec($ch);
-echo "Ok\n";
+ curl_close($ch);
+ echo "Ok for $description\n";
+}
+
+$options_to_check = array(
+ "CURLOPT_STDERR",
+ "CURLOPT_WRITEHEADER",
+ "CURLOPT_FILE",
+ "CURLOPT_INFILE"
+);
+
+foreach($options_to_check as $option) {
+ checkForClosedFilePointer(constant($option), $option);
+}
+
?>
--CLEAN--
<?php @unlink(dirname(__FILE__) . '/bug48203.tmp'); ?>
---EXPECT--
-Ok
+--EXPECTF--
+Warning: curl_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d
+* About to connect() %a
+* Closing connection #%d
+Ok for CURLOPT_STDERR
+
+Warning: curl_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on line %d
+Ok for CURLOPT_WRITEHEADER
+
+Warning: curl_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_exec(): CURLOPT_FILE handle is incorrect in %s on line %d
+%a
+Ok for CURLOPT_FILE
+
+Warning: curl_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d
+Ok for CURLOPT_INFILE
Index: trunk/ext/curl/interface.c
===================================================================
--- trunk/ext/curl/interface.c (revision 309881)
+++ trunk/ext/curl/interface.c (revision )
@@ -2190,6 +2190,41 @@
}
/* }}} */
+int _php_curl_check_file_pointer(zval *zv_stream, const char* filehandle_type
TSRMLS_DC)
+{
+ php_stream *stream;
+ if (zv_stream) {
+ if (!(php_stream_from_zval_no_verify(stream, &zv_stream))) {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s handle
is incorrect", filehandle_type);
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+int _php_curl_file_pointers_correct(php_curl *ch TSRMLS_DC)
+{
+ if (!_php_curl_check_file_pointer(ch->handlers->std_err,
"CURLOPT_STDERR" TSRMLS_CC)) {
+ curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr);
+ }
+
+ if (!_php_curl_check_file_pointer(ch->handlers->write->stream,
"CURLOPT_FILE" TSRMLS_CC)) {
+ ch->handlers->write->method = PHP_CURL_STDOUT;
+ }
+
+ if (!_php_curl_check_file_pointer(ch->handlers->write_header->stream,
"CURLOPT_WRITEHEADER" TSRMLS_CC)) {
+ ch->handlers->write_header->method = PHP_CURL_IGNORE;
+ }
+
+ // this is critical since there's no point in upload with corrupted file
+ if (!_php_curl_check_file_pointer(ch->handlers->read->stream,
"CURLOPT_INFILE" TSRMLS_CC)) {
+ return 0;
+ }
+
+ return 1;
+}
+
/* {{{ proto bool curl_exec(resource ch)
Perform a cURL session */
PHP_FUNCTION(curl_exec)
@@ -2204,6 +2239,10 @@
ZEND_FETCH_RESOURCE(ch, php_curl *, &zid, -1, le_curl_name, le_curl);
+ if (!_php_curl_file_pointers_correct(ch TSRMLS_CC)) {
+ RETURN_FALSE;
+ }
+
_php_curl_cleanup_handle(ch);
error = curl_easy_perform(ch->cp);
Index: trunk/ext/curl/tests/bug48203_multi.phpt
===================================================================
--- trunk/ext/curl/tests/bug48203_multi.phpt (revision )
+++ trunk/ext/curl/tests/bug48203_multi.phpt (revision )
@@ -0,0 +1,106 @@
+--TEST--
+Variation of bug #48203 with curl_multi_exec (Crash when file pointers passed
to curl are closed before calling curl_multi_exec)
+--SKIPIF--
+<?php
+if (!extension_loaded("curl")) {
+ exit("skip curl extension not loaded");
+}
+if (false === getenv('PHP_CURL_HTTP_REMOTE_SERVER')) {
+ exit("skip PHP_CURL_HTTP_REMOTE_SERVER env variable is not defined");
+}
+?>
+--FILE--
+<?php
+
+function checkForClosedFilePointer($curl_option, $description) {
+ $fp = fopen(dirname(__FILE__) . '/bug48203.tmp', 'w');
+
+ $ch1 = curl_init();
+ $ch2 = curl_init();
+
+ $options = array(
+ CURLOPT_RETURNTRANSFER => 1,
+ $curl_option => $fp,
+ CURLOPT_URL => getenv("PHP_CURL_HTTP_REMOTE_SERVER")
+ );
+
+ // we also need to set CURLOPT_VERBOSE to test CURLOPT_STDERR properly
+ if (CURLOPT_STDERR == $curl_option) {
+ $options[CURLOPT_VERBOSE] = 1;
+ }
+
+ if (CURLOPT_INFILE == $curl_option) {
+ $options[CURLOPT_UPLOAD] = 1;
+ }
+
+ curl_setopt_array($ch1, $options);
+ curl_setopt_array($ch2, $options);
+
+ fclose($fp); // <-- premature close of $fp caused a crash!
+
+ $mh = curl_multi_init();
+
+ curl_multi_add_handle($mh, $ch1);
+ curl_multi_add_handle($mh, $ch2);
+
+ $active = 0;
+ do {
+ curl_multi_exec($mh, $active);
+ } while ($active > 0);
+
+ curl_multi_remove_handle($mh, $ch1);
+ curl_multi_remove_handle($mh, $ch2);
+ curl_multi_close($mh);
+
+ echo "Ok for $description\n";
+}
+
+$options_to_check = array(
+ "CURLOPT_STDERR", "CURLOPT_WRITEHEADER", "CURLOPT_FILE",
"CURLOPT_INFILE"
+);
+
+foreach($options_to_check as $option) {
+ checkForClosedFilePointer(constant($option), $option);
+}
+
+?>
+--CLEAN--
+<?php @unlink(dirname(__FILE__) . '/bug48203.tmp'); ?>
+--EXPECTF--
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d
+%a
+Ok for CURLOPT_STDERR
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on
line %d
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on
line %d
+%AOk for CURLOPT_WRITEHEADER
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d
+%a
+Ok for CURLOPT_FILE
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d
+
+Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
+
+Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d
+Ok for CURLOPT_INFILE
Index: trunk/ext/curl/tests/curl_file_deleted_before_curl_close.phpt
===================================================================
--- trunk/ext/curl/tests/curl_file_deleted_before_curl_close.phpt
(revision )
+++ trunk/ext/curl/tests/curl_file_deleted_before_curl_close.phpt
(revision )
@@ -0,0 +1,37 @@
+--TEST--
+Memory corruption error if fp of just created file is closed before curl_close.
+--CREDITS--
+Alexey Shein <[email protected]>
+--SKIPIF--
+<?php if (!extension_loaded("curl") || false ===
getenv('PHP_CURL_HTTP_REMOTE_SERVER')) print "skip"; ?>
+--FILE--
+<?php
+
+$ch = curl_init(getenv('PHP_CURL_HTTP_REMOTE_SERVER'));
+
+$temp_file = dirname(__FILE__) . '/curl_file_deleted_before_curl_close.tmp';
+if (file_exists($temp_file)) {
+ unlink($temp_file); // file should not exist before test
+}
+
+$handle = fopen($temp_file, 'w');
+
+curl_setopt($ch, CURLOPT_STDERR, $handle);
+curl_setopt($ch, CURLOPT_VERBOSE, 1);
+curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+
+curl_exec($ch);
+
+fclose($handle); // causes glibc memory error
+
+//unlink($temp_file); // uncomment to test segfault (file not found on
iowrite.c)
+
+curl_close($ch);
+echo "Closed correctly\n";
+?>
+--CLEAN--
+<?php
+unlink(dirname(__FILE__) . '/curl_file_deleted_before_curl_close.tmp');
+?>
+--EXPECTF--
+Closed correctly
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
