Skip to content

Commit e43c21e

Browse files
bjoribjori
committed
Fixed issues when streams were closed before curl read/write from them, or cleaning
Closing a original handle after copying it now no longer cleans up all resources (fixes missing CURLOPT_POSTFIELDS values among others)
1 parent 8ed1dc2 commit e43c21e

9 files changed

+124
-57
lines changed

‎ext/curl/interface.c

+80-26
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ static struct gcry_thread_cbs php_curl_gnutls_tsl = {
150150
static void _php_curl_close_ex(php_curl *ch TSRMLS_DC);
151151
static void _php_curl_close(zend_rsrc_list_entry *rsrc TSRMLS_DC);
152152

153+
153154
#define SAVE_CURL_ERROR(__handle, __err) (__handle)->err.no = (int) __err;
154155

155156
#define CAAL(s, v) add_assoc_long_ex(return_value, s, sizeof(s), (long) v);
@@ -204,6 +205,72 @@ static int php_curl_option_url(php_curl *ch, const char *url, const int len) /*
204205
}
205206
/* }}} */
206207

208+
int _php_curl_verify_handlers(php_curl *ch, int reporterror TSRMLS_DC) /* {{{ */
209+
{
210+
php_stream *stream;
211+
if (!ch || !ch->handlers) {
212+
return 0;
213+
}
214+
215+
if (ch->handlers->std_err) {
216+
stream = (php_stream *) zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
217+
if (stream == NULL) {
218+
if (reporterror) {
219+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_STDERR resource has gone away, resetting to stderr");
220+
}
221+
zval_ptr_dtor(&ch->handlers->std_err);
222+
ch->handlers->std_err = NULL;
223+
224+
curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr);
225+
}
226+
}
227+
if (ch->handlers->read && ch->handlers->read->stream) {
228+
stream = (php_stream *) zend_fetch_resource(&ch->handlers->read->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
229+
if (stream == NULL) {
230+
if (reporterror) {
231+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_INFILE resource has gone away, resetting to default");
232+
}
233+
zval_ptr_dtor(&ch->handlers->read->stream);
234+
ch->handlers->read->fd = 0;
235+
ch->handlers->read->fp = 0;
236+
ch->handlers->read->stream = NULL;
237+
238+
curl_easy_setopt(ch->cp, CURLOPT_INFILE, (void *) ch);
239+
}
240+
}
241+
if (ch->handlers->write_header && ch->handlers->write_header->stream) {
242+
stream = (php_stream *) zend_fetch_resource(&ch->handlers->write_header->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
243+
if (stream == NULL) {
244+
if (reporterror) {
245+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_WRITEHEADER resource has gone away, resetting to default");
246+
}
247+
zval_ptr_dtor(&ch->handlers->write_header->stream);
248+
ch->handlers->write_header->fp = 0;
249+
ch->handlers->write_header->stream = NULL;
250+
251+
ch->handlers->write_header->method = PHP_CURL_IGNORE;
252+
curl_easy_setopt(ch->cp, CURLOPT_WRITEHEADER, (void *) ch);
253+
}
254+
}
255+
if (ch->handlers->write && ch->handlers->write->stream) {
256+
stream = (php_stream *) zend_fetch_resource(&ch->handlers->write->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
257+
if (stream == NULL) {
258+
if (reporterror) {
259+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_FILE resource has gone away, resetting to default");
260+
}
261+
zval_ptr_dtor(&ch->handlers->write->stream);
262+
ch->handlers->write->fp = 0;
263+
ch->handlers->write->stream = NULL;
264+
265+
ch->handlers->write->method = PHP_CURL_STDOUT;
266+
ch->handlers->write->type = PHP_CURL_ASCII;
267+
curl_easy_setopt(ch->cp, CURLOPT_FILE, (void *) ch);
268+
}
269+
}
270+
return 1;
271+
}
272+
/* }}} */
273+
207274
/* {{{ arginfo */
208275
ZEND_BEGIN_ARG_INFO_EX(arginfo_curl_version, 0, 0, 0)
209276
ZEND_ARG_INFO(0, version)
@@ -337,7 +404,6 @@ PHP_INI_BEGIN()
337404
PHP_INI_END()
338405
/* }}} */
339406

340-
/* }}} */
341407
/* {{{ PHP_MINFO_FUNCTION
342408
*/
343409
PHP_MINFO_FUNCTION(curl)
@@ -1394,6 +1460,7 @@ static void split_certinfo(char *string, zval *hash)
13941460
efree(org);
13951461
}
13961462
}
1463+
/* }}} */
13971464

13981465
/* {{{ create_certinfo
13991466
*/
@@ -1537,6 +1604,7 @@ PHP_FUNCTION(curl_copy_handle)
15371604

15381605
dupch->cp = cp;
15391606
dupch->uses = 0;
1607+
ch->uses++;
15401608
if (ch->handlers->write->stream) {
15411609
Z_ADDREF_P(dupch->handlers->write->stream);
15421610
dupch->handlers->write->stream = ch->handlers->write->stream;
@@ -2211,28 +2279,10 @@ PHP_FUNCTION(curl_exec)
22112279

22122280
ZEND_FETCH_RESOURCE(ch, php_curl *, &zid, -1, le_curl_name, le_curl);
22132281

2282+
_php_curl_verify_handlers(ch, 1 TSRMLS_CC);
2283+
22142284
_php_curl_cleanup_handle(ch);
22152285

2216-
if (ch->handlers->std_err) {
2217-
php_stream *stream;
2218-
stream = (php_stream*)zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
2219-
if (stream == NULL) {
2220-
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_STDERR resource has gone away, resetting to stderr");
2221-
zval_ptr_dtor(&ch->handlers->std_err);
2222-
curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr);
2223-
}
2224-
}
2225-
if (ch->handlers->read && ch->handlers->read->stream) {
2226-
php_stream *stream;
2227-
stream = (php_stream*)zend_fetch_resource(&ch->handlers->read->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
2228-
if (stream == NULL) {
2229-
php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_INFILE resource has gone away, resetting to default");
2230-
zval_ptr_dtor(&ch->handlers->read->stream);
2231-
ch->handlers->read->fd = 0;
2232-
ch->handlers->read->fp = 0;
2233-
curl_easy_setopt(ch->cp, CURLOPT_INFILE, (void *) ch);
2234-
}
2235-
}
22362286
error = curl_easy_perform(ch->cp);
22372287
SAVE_CURL_ERROR(ch, error);
22382288
/* CURLE_PARTIAL_FILE is returned by HEAD requests */
@@ -2243,6 +2293,14 @@ PHP_FUNCTION(curl_exec)
22432293
RETURN_FALSE;
22442294
}
22452295

2296+
if (ch->handlers->std_err) {
2297+
php_stream *stream;
2298+
stream = (php_stream*)zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream());
2299+
if (stream) {
2300+
php_stream_flush(stream);
2301+
}
2302+
}
2303+
22462304
if (ch->handlers->write->method == PHP_CURL_RETURN && ch->handlers->write->buf.len > 0) {
22472305
smart_str_0(&ch->handlers->write->buf);
22482306
RETURN_STRINGL(ch->handlers->write->buf.c, ch->handlers->write->buf.len, 1);
@@ -2520,11 +2578,7 @@ static void _php_curl_close_ex(php_curl *ch TSRMLS_DC)
25202578
fprintf(stderr, "DTOR CALLED, ch = %x\n", ch);
25212579
#endif
25222580

2523-
/* Prevent crash inside cURL if passed file has already been closed */
2524-
if (ch->handlers->std_err && Z_REFCOUNT_P(ch->handlers->std_err) <= 0) {
2525-
curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr);
2526-
}
2527-
2581+
_php_curl_verify_handlers(ch, 0 TSRMLS_CC);
25282582
curl_easy_cleanup(ch->cp);
25292583
zend_llist_clean(&ch->to_free.str);
25302584

‎ext/curl/multi.c

+24
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,19 @@ PHP_FUNCTION(curl_multi_exec)
211211

212212
ZEND_FETCH_RESOURCE(mh, php_curlm *, &z_mh, -1, le_curl_multi_handle_name, le_curl_multi_handle);
213213

214+
{
215+
zend_llist_position pos;
216+
php_curl *ch;
217+
zval *pz_ch;
218+
219+
for(pz_ch = (zval *)zend_llist_get_first_ex(&mh->easyh, &pos); pz_ch;
220+
pz_ch = (zval *)zend_llist_get_next_ex(&mh->easyh, &pos)) {
221+
222+
ZEND_FETCH_RESOURCE(ch, php_curl *, &pz_ch, -1, le_curl_name, le_curl);
223+
_php_curl_verify_handlers(ch, 1 TSRMLS_CC);
224+
}
225+
}
226+
214227
convert_to_long_ex(&z_still_running);
215228
still_running = Z_LVAL_P(z_still_running);
216229
result = curl_multi_perform(mh->multi, &still_running);
@@ -324,6 +337,17 @@ void _php_curl_multi_close(zend_rsrc_list_entry *rsrc TSRMLS_DC) /* {{{ */
324337
{
325338
php_curlm *mh = (php_curlm *) rsrc->ptr;
326339
if (mh) {
340+
zend_llist_position pos;
341+
php_curl *ch;
342+
zval *pz_ch;
343+
344+
for(pz_ch = (zval *)zend_llist_get_first_ex(&mh->easyh, &pos); pz_ch;
345+
pz_ch = (zval *)zend_llist_get_next_ex(&mh->easyh, &pos)) {
346+
347+
ch = (php_curl *) zend_fetch_resource(&pz_ch TSRMLS_CC, -1, le_curl_name, NULL, 1, le_curl);
348+
_php_curl_verify_handlers(ch, 0 TSRMLS_CC);
349+
}
350+
327351
curl_multi_cleanup(mh->multi);
328352
zend_llist_clean(&mh->easyh);
329353
efree(mh);

‎ext/curl/php_curl.h

+1
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ typedef struct {
150150

151151
void _php_curl_cleanup_handle(php_curl *);
152152
void _php_curl_multi_cleanup_list(void *data);
153+
int _php_curl_verify_handlers(php_curl *ch, int reporterror TSRMLS_DC);
153154

154155
/* streams support */
155156

‎ext/curl/tests/bug48203_multi.phpt

+11-27
Original file line numberDiff line numberDiff line change
@@ -67,40 +67,24 @@ foreach($options_to_check as $option) {
6767
--CLEAN--
6868
<?php @unlink(dirname(__FILE__) . '/bug48203.tmp'); ?>
6969
--EXPECTF--
70-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
70+
Warning: curl_multi_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr in %sbug48203_multi.php on line 36
7171

72-
Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d
73-
74-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
75-
76-
Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d
77-
%a
72+
Warning: curl_multi_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr in %sbug48203_multi.php on line 36
73+
%A
7874
Ok for CURLOPT_STDERR
7975

80-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
81-
82-
Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on line %d
83-
84-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
76+
Warning: curl_multi_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug48203_multi.php on line 36
8577

86-
Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on line %d
87-
%AOk for CURLOPT_WRITEHEADER
78+
Warning: curl_multi_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug48203_multi.php on line 36
79+
Ok for CURLOPT_WRITEHEADER
8880

89-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
81+
Warning: curl_multi_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36
9082

91-
Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d
92-
93-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
94-
95-
Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d
96-
%a
83+
Warning: curl_multi_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36
84+
%A
9785
Ok for CURLOPT_FILE
9886

99-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
100-
101-
Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d
102-
103-
Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d
87+
Warning: curl_multi_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36
10488

105-
Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d
89+
Warning: curl_multi_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36
10690
Ok for CURLOPT_INFILE

‎ext/curl/tests/bug54798.phpt

+5
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,12 @@ Warning: curl_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr
5959
* About to connect() %a
6060
* Closing connection #%d
6161
Ok for CURLOPT_STDERR
62+
63+
Warning: curl_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug54798.php on line 24
6264
Ok for CURLOPT_WRITEHEADER
65+
66+
Warning: curl_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug54798.php on line 24
67+
%a
6368
Ok for CURLOPT_FILE
6469

6570
Warning: curl_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug54798.php on line %d

‎ext/curl/tests/curl_copy_handle_basic_002.phpt

-2
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ Rick Buitenman <rick@meritos.nl>
2929
var_dump( $curl_content );
3030
?>
3131
===DONE===
32-
--XFAIL--
33-
This test fails, the copy seems to be missing the CURLOPT_POSTFIELDS after the original is closed
3432
--EXPECTF--
3533
*** Testing curl copy handle with simple POST ***
3634
string(163) "array(1) {

‎ext/curl/tests/curl_copy_handle_basic_005.phpt

-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ Rick Buitenman <rick@meritos.nl>
3232
var_dump( $curl_content_copy );
3333
?>
3434
===DONE===
35-
--XFAIL--
36-
This test fails, the output of the copy seems to be corrupted if the original is closed after exec()
3735
--EXPECTF--
3836
*** Test curl_copy_handle() after exec() with POST ***
3937
string(163) "array(1) {

‎ext/curl/tests/curl_file_deleted_before_curl_close.phpt

+1
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@ echo "Closed correctly\n";
3434
unlink(dirname(__FILE__) . '/curl_file_deleted_before_curl_close.tmp');
3535
?>
3636
--EXPECTF--
37+
* Closing connection #%d
3738
Closed correctly

‎ext/curl/tests/curl_setopt_basic002.phpt

+2
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,5 @@ curl_close($ch);
4848
*** Testing curl_setopt with CURLOPT_STDERR
4949
string(%d) "%S"
5050
string(%d) "%S"
51+
* Closing connection #%d
52+

0 commit comments

Comments
 (0)