- disallow detaching requests while executing progress callbacks

- hopefully the final fix for nested exceptions

diff --git a/http_api.c b/http_api.c
index 372265a04052ce482568da9f8f44ff862b1a8b7e..a2a7f6ce71cfb0601392e46574e85dfdb74cc334 100644 (file)
--- a/http_api.c
+++ b/http_api.c
@@ -96,15 +96,15 @@ void _http_error_ex(long type TSRMLS_DC, long code, const char *format, ...)
        
        va_start(args, format);
 #ifdef ZEND_ENGINE_2
-               if ((type == E_THROW) || (PG(error_handling) == EH_THROW)) {
-                       char *message;
-                       zend_class_entry *ce = http_exception_get_for_code(code);
-                       
-                       http_try {
-                               vspprintf(&message, 0, format, args);
-                               zend_throw_exception(ce, message, code TSRMLS_CC);
-                               efree(message);
-                       } http_catch(ce);
+       if ((type == E_THROW) || (PG(error_handling) == EH_THROW)) {
+               char *message;
+               zend_class_entry *ce = http_exception_get_for_code(code);
+               
+               http_try {
+                       vspprintf(&message, 0, format, args);
+                       zend_throw_exception(ce, message, code TSRMLS_CC);
+                       efree(message);
+               } http_catch(PG(exception_class) ? PG(exception_class) : HTTP_EX_DEF_CE);
        } else
 #endif
        php_verror(NULL, "", type, format, args TSRMLS_CC);

diff --git a/http_exception_object.c b/http_exception_object.c
index 4562922bad76cd273187fba6a14b9b6f0b494e61..80e995277edea44a54872e0408b79e915112f3c2 100644 (file)
--- a/http_exception_object.c
+++ b/http_exception_object.c
@@ -16,7 +16,12 @@
 
 #ifdef ZEND_ENGINE_2
 
+#ifndef HTTP_DBG_EXCEPTIONS
+#      define HTTP_DBG_EXCEPTIONS 0
+#endif
+
 #include "zend_interfaces.h"
+#include "zend_exceptions.h"
 #include "php_http_exception_object.h"
 
 zend_class_entry *http_exception_object_ce;
@@ -34,18 +39,29 @@ zend_class_entry *HTTP_EX_CE(response);
 zend_class_entry *HTTP_EX_CE(url);
 zend_class_entry *HTTP_EX_CE(querystring);
 
-#define HTTP_EMPTY_ARGS(method)                                HTTP_EMPTY_ARGS_EX(HttpException, method, 0)
+#define HTTP_EMPTY_ARGS(method)                                        HTTP_EMPTY_ARGS_EX(HttpException, method, 0)
 #define HTTP_EXCEPTION_ME(method, visibility)  PHP_ME(HttpException, method, HTTP_ARGS(HttpException, method), visibility)
 
 HTTP_EMPTY_ARGS(__toString);
 
-#define OBJ_PROP_CE http_deflatestream_object_ce
 zend_function_entry http_exception_object_fe[] = {
        HTTP_EXCEPTION_ME(__toString, ZEND_ACC_PUBLIC)
        
        EMPTY_FUNCTION_ENTRY
 };
 
+#if HTTP_DBG_EXCEPTIONS
+static void http_exception_hook(zval *ex TSRMLS_DC)
+{
+       if (ex) {
+               zval *m = zend_read_property(Z_OBJCE_P(ex), ex, "message", lenof("message"), 0 TSRMLS_CC);
+               fprintf(stderr, "*** Threw exception '%s'\n", Z_STRVAL_P(m));
+       } else {
+               fprintf(stderr, "*** Threw NULL exception\n");
+       }
+}
+#endif
+
 PHP_MINIT_FUNCTION(http_exception_object)
 {
        HTTP_REGISTER_CLASS(HttpException, http_exception_object, ZEND_EXCEPTION_GET_DEFAULT(), 0);
@@ -80,6 +96,10 @@ PHP_MINIT_FUNCTION(http_exception_object)
        HTTP_LONG_CONSTANT("HTTP_E_URL", HTTP_E_URL);
        HTTP_LONG_CONSTANT("HTTP_E_QUERYSTRING", HTTP_E_QUERYSTRING);
        
+#if HTTP_DBG_EXCEPTIONS
+       zend_throw_exception_hook=http_exception_hook;
+#endif
+       
        return SUCCESS;
 }
 
@@ -122,15 +142,15 @@ PHP_METHOD(HttpException, __toString)
        do {
                ce = Z_OBJCE_P(zobj);
                
-               if (zobj != getThis()) {
-                       phpstr_appends(&full_str, " inner ");
-               }
-               
                m = zend_read_property(ce, zobj, "message", lenof("message"), 0 TSRMLS_CC);
                f = zend_read_property(ce, zobj, "file", lenof("file"), 0 TSRMLS_CC);
                l = zend_read_property(ce, zobj, "line", lenof("line"), 0 TSRMLS_CC);
                
                if (m && f && l && Z_TYPE_P(m) == IS_STRING && Z_TYPE_P(f) == IS_STRING && Z_TYPE_P(l) == IS_LONG) {
+                       if (zobj != getThis()) {
+                               phpstr_appends(&full_str, " inner ");
+                       }
+               
                        phpstr_appendf(&full_str, "exception '%.*s' with message '%.*s' in %.*s:%ld" PHP_EOL,
                                ce->name_length, ce->name, Z_STRLEN_P(m), Z_STRVAL_P(m), Z_STRLEN_P(f), Z_STRVAL_P(f), Z_LVAL_P(l)
                        );
@@ -149,6 +169,7 @@ PHP_METHOD(HttpException, __toString)
        
        RETURN_PHPSTR_VAL(&full_str);
 }
+
 #endif
 
 /*

diff --git a/http_request_api.c b/http_request_api.c
index b6d7b1e76e4cb602e0ef8e9a13626b4160ced6c5..b253a97caae73cca8b99dcf883dd39889b81e52c 100644 (file)
--- a/http_request_api.c
+++ b/http_request_api.c
@@ -899,7 +899,9 @@ static int http_curl_progress_callback(void *ctx, double dltotal, double dlnow,
        add_assoc_double(param, "ulnow", ulnow);
        
        with_error_handling(EH_NORMAL, NULL) {
+               request->_in_progress_cb = 1;
                call_user_function(EG(function_table), NULL, request->_progress_callback, &retval, 1, &param TSRMLS_CC);
+               request->_in_progress_cb = 0;
        } end_error_handling();
        
        zval_ptr_dtor(&param);

diff --git a/http_request_pool_api.c b/http_request_pool_api.c
index ae125259302939a74a3882147f23b185bd0ceaa1..5b04472566e05111f51d04e56216dabcc6407561 100644 (file)
--- a/http_request_pool_api.c
+++ b/http_request_pool_api.c
@@ -80,12 +80,12 @@ PHP_HTTP_API STATUS _http_request_pool_attach(http_request_pool *pool, zval *req
        if (req->pool) {
                http_error_ex(HE_WARNING, HTTP_E_INVALID_PARAM, "HttpRequest object(#%d) is already member of %s HttpRequestPool", Z_OBJ_HANDLE_P(request), req->pool == pool ? "this" : "another");
        } else if (SUCCESS != http_request_object_requesthandler(req, request)) {
-               http_error_ex(HE_WARNING, HTTP_E_REQUEST, "Could not initialize HttpRequest object for attaching to the HttpRequestPool");
+               http_error_ex(HE_WARNING, HTTP_E_REQUEST, "Could not initialize HttpRequest object(#%d) for attaching to the HttpRequestPool", Z_OBJ_HANDLE_P(request));
        } else {
                CURLMcode code = curl_multi_add_handle(pool->ch, req->request->ch);
 
                if ((CURLM_OK != code) && (CURLM_CALL_MULTI_PERFORM != code)) {
-                       http_error_ex(HE_WARNING, HTTP_E_REQUEST_POOL, "Could not attach HttpRequest object to the HttpRequestPool: %s", curl_multi_strerror(code));
+                       http_error_ex(HE_WARNING, HTTP_E_REQUEST_POOL, "Could not attach HttpRequest object(#%d) to the HttpRequestPool: %s", Z_OBJ_HANDLE_P(request), curl_multi_strerror(code));
                } else {
                        req->pool = pool;
 
@@ -119,8 +119,10 @@ PHP_HTTP_API STATUS _http_request_pool_detach(http_request_pool *pool, zval *req
 #endif
        } else if (req->pool != pool) {
                http_error_ex(HE_WARNING, HTTP_E_INVALID_PARAM, "HttpRequest object(#%d) is not attached to this HttpRequestPool", Z_OBJ_HANDLE_P(request));
+       } else if (req->request->_in_progress_cb) {
+               http_error_ex(HE_WARNING, HTTP_E_REQUEST_POOL, "HttpRequest object(#%d) cannot be detached from the HttpRequestPool while executing the progress callback", Z_OBJ_HANDLE_P(request));
        } else if (CURLM_OK != (code = curl_multi_remove_handle(pool->ch, req->request->ch))) {
-               http_error_ex(HE_WARNING, HTTP_E_REQUEST_POOL, "Could not detach HttpRequest object from the HttpRequestPool: %s", curl_multi_strerror(code));
+               http_error_ex(HE_WARNING, HTTP_E_REQUEST_POOL, "Could not detach HttpRequest object(#%d) from the HttpRequestPool: %s", Z_OBJ_HANDLE_P(request), curl_multi_strerror(code));
        } else {
                req->pool = NULL;
                zend_llist_del_element(&pool->finished, request, http_request_pool_compare_handles);
@@ -213,7 +215,7 @@ PHP_HTTP_API STATUS _http_request_pool_send(http_request_pool *pool TSRMLS_DC)
        fprintf(stderr, "Attempt to send %d requests of pool %p\n", zend_llist_count(&pool->handles), pool);
 #endif
        
-       while (http_request_pool_perform(pool, 0)) {
+       while (http_request_pool_perform(pool)) {
                if (SUCCESS != http_request_pool_select(pool)) {
 #ifdef PHP_WIN32
                        /* see http://msdn.microsoft.com/library/en-us/winsock/winsock/windows_sockets_error_codes_2.asp */
@@ -274,7 +276,7 @@ PHP_HTTP_API STATUS _http_request_pool_select(http_request_pool *pool)
 /* }}} */
 
 /* {{{ int http_request_pool_perform(http_request_pool *) */
-PHP_HTTP_API int _http_request_pool_perform(http_request_pool *pool, int once TSRMLS_DC)
+PHP_HTTP_API int _http_request_pool_perform(http_request_pool *pool TSRMLS_DC)
 {
        CURLMsg *msg;
        int remaining = 0;
@@ -284,20 +286,13 @@ PHP_HTTP_API int _http_request_pool_perform(http_request_pool *pool, int once TS
        while ((msg = curl_multi_info_read(pool->ch, &remaining))) {
                if (CURLMSG_DONE == msg->msg) {
                                if (CURLE_OK != msg->data.result) {
-                                       http_request_pool_try {
-                                               http_request *r = NULL;
-                                               curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE, &r);
-                                               http_error_ex(HE_WARNING, HTTP_E_REQUEST, "%s; %s (%s)", curl_easy_strerror(msg->data.result), r?r->_error:"", r?r->url:"");
-                                       } http_request_pool_catch();
+                                       http_request *r = NULL;
+                                       curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE, &r);
+                                       http_error_ex(HE_WARNING, HTTP_E_REQUEST, "%s; %s (%s)", curl_easy_strerror(msg->data.result), r?r->_error:"", r?r->url:"");
                                }
-                               http_request_pool_try {
-                                       http_request_pool_apply_with_arg(pool, _http_request_pool_responsehandler, msg->easy_handle);
-                               } http_request_pool_catch();
+                               http_request_pool_apply_with_arg(pool, _http_request_pool_responsehandler, msg->easy_handle);
                }
        }
-       if (once || !pool->unfinished) {
-               http_request_pool_final();
-       }
        
        return pool->unfinished;
 }

diff --git a/http_requestpool_object.c b/http_requestpool_object.c
index 665a65ccbe27e2036b6ba89defd3ab39dfd7fef9..69c3f9616a0ade832514f9e38e153d90d61c2655 100644 (file)
--- a/http_requestpool_object.c
+++ b/http_requestpool_object.c
@@ -194,15 +194,13 @@ PHP_METHOD(HttpRequestPool, __construct)
 
                for (i = 0; i < argc; ++i) {
                        if (Z_TYPE_PP(argv[i]) == IS_OBJECT && instanceof_function(Z_OBJCE_PP(argv[i]), http_request_object_ce TSRMLS_CC)) {
-                               http_request_pool_try {
-                                       http_request_pool_attach(&obj->pool, *(argv[i]));
-                               } http_request_pool_catch();
+                               http_request_pool_attach(&obj->pool, *(argv[i]));
                        }
                }
-               http_request_pool_final();
        }
        efree(argv);
        SET_EH_NORMAL();
+       http_final(HTTP_EX_CE(request_pool));
 }
 /* }}} */
 
@@ -311,6 +309,9 @@ PHP_METHOD(HttpRequestPool, send)
        SET_EH_THROW_HTTP();
        status = http_request_pool_send(&obj->pool);
        SET_EH_NORMAL();
+       
+       /* rethrow as HttpRequestPoolException */
+       http_final(HTTP_EX_CE(request_pool));
 
        RETURN_SUCCESS(status);
 }
@@ -353,7 +354,7 @@ PHP_METHOD(HttpRequestPool, socketPerform)
 
        NO_ARGS;
 
-       if (0 < http_request_pool_perform(&obj->pool, 1)) {
+       if (0 < http_request_pool_perform(&obj->pool)) {
                RETURN_TRUE;
        } else {
                RETURN_FALSE;

diff --git a/package2.xml b/package2.xml
index fe7c74995af0962c1f6eef8613c5fb3e45cbe7b7..75e5556e99ef3f90e837595be50169add9ac7707 100644 (file)
--- a/package2.xml
+++ b/package2.xml
@@ -40,6 +40,7 @@ support. Parallel requests are available for PHP 5 and greater.
  <license>BSD, revised</license>
  <notes><![CDATA[
 + Added HttpException::__toString() which takes care about any inner exceptions
+- Disallowed detaching HttpRequest objects from HttpRequestPool while executing progress callback
 * Fixed issues with inheritance and cloning of HttpDeflateStream, HttpInflateStream and HttpMessage
   (Extending classes lose default properties on instantiation; Complex members ignored during cloning)
 * Fixed suppression of nested exceptions in e.g. HttpRequestPool::send() (fixes bug #8535)

diff --git a/php_http_api.h b/php_http_api.h
index 325378c4d0ec3d2e167d6efaafa8c36ccc196ca4..9e5c389da97df25377b60c4849acc2f098ab0ed0 100644 (file)
--- a/php_http_api.h
+++ b/php_http_api.h
@@ -54,9 +54,7 @@ extern zval *_http_exception_wrap(zval *old_exception, zval *new_exception, zend
 }
 #define http_final(ex_ce) \
        if (EG(exception)) { \
-               zval *exception = http_exception_wrap(EG(exception), NULL, ex_ce); \
-               EG(exception) = NULL; \
-               zend_throw_exception_object(exception TSRMLS_CC); \
+               EG(exception) = http_exception_wrap(EG(exception), NULL, ex_ce); \
        }
 #endif /* ZEND_ENGINE_2 */
 

diff --git a/php_http_request_api.h b/php_http_request_api.h
index 286792dae8a563c1ecfdcd5feebe16572d3bc4f7..bc6e2461cce0dae1363a6b96fcf9394c4c4ad202 100644 (file)
--- a/php_http_request_api.h
+++ b/php_http_request_api.h
@@ -48,6 +48,8 @@ typedef struct _http_request_t {
        void ***tsrm_ls;
 #endif
 
+       uint _in_progress_cb:1;
+
 } http_request;
 
 #define http_curl_init(r) http_curl_init_ex(NULL, (r))

diff --git a/php_http_request_pool_api.h b/php_http_request_pool_api.h
index 492a797393763cce8e285c01250d8aafdd5ff5f6..aff9c1eebcf2d81465052f796be5d3c78c3df6e1 100644 (file)
--- a/php_http_request_pool_api.h
+++ b/php_http_request_pool_api.h
@@ -29,10 +29,6 @@ typedef int (*http_request_pool_apply_with_arg_func)(http_request_pool *pool, zv
 #define http_request_pool_responsehandler(p, r, c) _http_request_pool_responsehandler((p), (r), (c) TSRMLS_CC)
 extern int _http_request_pool_responsehandler(http_request_pool *pool, zval *req, void *ch TSRMLS_DC);
 
-#define http_request_pool_try http_try
-#define http_request_pool_catch() http_catch(HTTP_EX_CE(request_pool))
-#define http_request_pool_final() http_final(HTTP_EX_CE(request_pool))
-
 #define http_request_pool_init(p) _http_request_pool_init((p) TSRMLS_CC)
 PHP_HTTP_API http_request_pool *_http_request_pool_init(http_request_pool *pool TSRMLS_DC);
 
@@ -57,8 +53,8 @@ PHP_HTTP_API STATUS _http_request_pool_send(http_request_pool *pool TSRMLS_DC);
 #define http_request_pool_select _http_request_pool_select
 PHP_HTTP_API STATUS _http_request_pool_select(http_request_pool *pool);
 
-#define http_request_pool_perform(p, o) _http_request_pool_perform((p), (o) TSRMLS_CC)
-PHP_HTTP_API int _http_request_pool_perform(http_request_pool *pool, int once TSRMLS_DC);
+#define http_request_pool_perform(p) _http_request_pool_perform((p) TSRMLS_CC)
+PHP_HTTP_API int _http_request_pool_perform(http_request_pool *pool TSRMLS_DC);
 
 #define http_request_pool_dtor(p) _http_request_pool_dtor((p) TSRMLS_CC)
 PHP_HTTP_API void _http_request_pool_dtor(http_request_pool *pool TSRMLS_DC);

diff --git a/tests/HttpRequestPool_006.phpt b/tests/HttpRequestPool_006.phpt
index 9d788ad2ada97592ac8a323eb2834c969b8ac8b9..1b5cb9f14269c6c0e2017a79b9de0bb720be483a 100644 (file)
--- a/tests/HttpRequestPool_006.phpt
+++ b/tests/HttpRequestPool_006.phpt
@@ -10,23 +10,41 @@ checkurl("de.php.net");
 --FILE--
 <?php
 echo "-TEST\n";
-class r1 extends HttpRequest {
+class r extends HttpRequest {
        function onProgress() {
-               $GLOBALS['p']->detach($this);
+               static $i = array();
+               if (empty($i[$this->getUrl()])) {
+                       $i[$this->getUrl()] = true;
+                       try {
+                               $GLOBALS['p']->detach($this);
+                       } catch (Exception $ex) {
+                               echo $ex, "\n";
+                       }
+               }
        }
-}
-class r2 extends HttpRequest {
        function onFinish() {
                $GLOBALS['p']->detach($this);
        }
 }
-$p = new HttpRequestPool(new r1("at.php.net"), new r2("de.php.net"));
+$p = new HttpRequestPool(new r("http://at.php.net"), new r("http://de.php.net"));
 $p->send();
 var_dump($p->getAttachedRequests());
 echo "Done\n";
 ?>
 --EXPECTF--
 %sTEST
+exception 'HttpRequestPoolException' with message 'HttpRequest object(#%d) cannot be detached from the HttpRequestPool while executing the progress callback' in %sHttpRequestPool_006.php:%d
+Stack trace:
+#0 %sHttpRequestPool_006.php(%d): HttpRequestPool->detach(Object(r))
+#1 [internal function]: r->onProgress(Array)
+#2 %sHttpRequestPool_006.php(%d): HttpRequestPool->send()
+#3 {main}
+exception 'HttpRequestPoolException' with message 'HttpRequest object(#%d) cannot be detached from the HttpRequestPool while executing the progress callback' in %sHttpRequestPool_006.php:%d
+Stack trace:
+#0 %sHttpRequestPool_006.php(%d): HttpRequestPool->detach(Object(r))
+#1 [internal function]: r->onProgress(Array)
+#2 %sHttpRequestPool_006.php(%d): HttpRequestPool->send()
+#3 {main}
 array(0) {
 }
 Done