Em Fri, 20 May 2011 07:59:51 +0100, Alexey Shein <con...@gmail.com>
escreveu:
* I think a better strategy would be to just dup the file descriptor
gotten after the cast in curl_setopt, store it (instead of storing the
zval) and close it on curl handle destruction. This way we wouldn't
have to worry
about zval refcounts or whether the file descriptor obtained is still
valid.
Could you see if this other strategy works? (otherwise I can do it
later this week)
Ok, I made it to work (thanks for consultation to Pierre and
Johannes). You were right, the bug remains with curl_multi_exec too.
BTW it happens not only with CURL_STDERR but also with all other
options that take fp as a parameter (CURLOPT_FILE,
CURLOPT_WRITEHEADER, CURLOPT_INFILE) so I added them to the test and
made separate test for curl_multi_exec (see the patch).
After some thoughts I think this is the case when user really wants to
shoot into his leg - probably from user pov we should generate a
warning that we can't write into the supplied pointer (but actually we
can :)) like my previous patch did, but it raised a couple of
problems:
1) curl_multi_exec is called directly without interception from php
and create a wrapper just for this check seems like an overkill to me
2) we need to add 3 more checks to restore default values for all fp
options (see above) in two places: curl_exec and curl_multi_exec -
again overkill.
So I decided to go with this patch.
What do you think?
First some considerations about your patch:
* You seem to be leaking the FILE* variable you created (you just removed
the zval_ptr_dtor(&ch->handlers->std_err); did not add any fclose call).
* You say the problem also occurs with CURLOPT_WRITEHEADER,
CURLOPT_WRITEHEADER and CURLOPT_INFILE. But again, you don't consider the
curl extension doesn't close the stdio pointers.
* No error checking in the calls to fileno, dup or fdopen.
* php_stream.mode cannot be used directly; see
php_stream_mode_sanitize_fdopen_fopencookie.
Now, I'm starting to think something in the line of your original patch is
better. Of course, curl_multi_exec and the other options would have to be
considered too.
The reason is I hadn't considered the curl extensions casts into a FILE*,
not an fd. Sure, sometimes you can still dup the fd and create another
stdio pointer, but this is not always the case (see fopencookie). So this
represents a removal of some functionality.
Another solution would be to force the stream to preserve its handle upon
closing. This is possible with PHP_STREAM_FLAG_NO_CLOSE, but I don't think
throwing the actual file closing to the script shutdown is a good idea.
--
Gustavo Lopes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
