On Thu, Apr 11, 2002 at 01:10:31AM +0200, Edin Kadribasic wrote:
> > I posted this patch earlier, but this has some improvements. First
> > of all, it's tabified, so we have consistent style. It also has some
> > performance improvements suggested by Cliff Woolley, including one that
> > will significantly improve output buffering/copying.
> >
> > The main guts of the patch include a overhaul/rewrite of the
> > php_output_filter. It no longer requires setaside brigades, and can
> > handle exactly one FILE bucket (Which it promptly passes off to PHP for
> > processing). If it finds no FILE bucket, it silently passes the brigade
> > bits down to the next filter. After processing a FILE, it steps out of
> > the way and passes all other brigades down the chain.
> >
> > The SEGVs reported earlier on this list and others are fixed with
> > this patch, as well as some other potential problems.
>
> Great work. Could you please send the patch as an email attachment? You
> should probably apply for the CVS account if you didn't have one already.
Attached, thanks.
-aaron
Index: sapi/apache2filter/sapi_apache2.c
===================================================================
RCS file: /repository/php4/sapi/apache2filter/sapi_apache2.c,v
retrieving revision 1.61.2.1
diff -u -u -r1.61.2.1 sapi_apache2.c
--- sapi/apache2filter/sapi_apache2.c 14 Mar 2002 10:57:00 -0000 1.61.2.1
+++ sapi/apache2filter/sapi_apache2.c 10 Apr 2002 20:36:27 -0000
@@ -48,26 +48,31 @@
{
apr_bucket *b;
apr_bucket_brigade *bb;
+ apr_bucket_alloc_t *ba;
php_struct *ctx;
- uint now;
ctx = SG(server_context);
if (str_length == 0) return 0;
- bb = apr_brigade_create(ctx->f->r->pool);
- while (str_length > 0) {
- now = MIN(str_length, 4096);
- b = apr_bucket_transient_create(str, now);
- APR_BRIGADE_INSERT_TAIL(bb, b);
- str += now;
- str_length -= now;
- }
+ ba = ctx->f->c->bucket_alloc;
+ bb = apr_brigade_create(ctx->f->r->pool, ba);
+
+ b = apr_bucket_transient_create(str, str_length, ba);
+ APR_BRIGADE_INSERT_TAIL(bb, b);
+
+ /* Add a Flush bucket to the end of this brigade, so that
+ * the transient buckets above are more likely to make it out
+ * the end of the filter instead of having to be copied into
+ * someone's setaside. */
+ b = apr_bucket_flush_create(ba);
+ APR_BRIGADE_INSERT_TAIL(bb, b);
+
if (ap_pass_brigade(ctx->f->next, bb) != APR_SUCCESS) {
php_handle_aborted_connection();
}
- return str_length;
+ return 0; /* we wrote everything, we promise! */
}
static int
@@ -161,6 +166,7 @@
{
php_struct *ctx = server_context;
apr_bucket_brigade *bb;
+ apr_bucket_alloc_t *ba;
apr_bucket *b;
if (!server_context)
@@ -171,8 +177,9 @@
* all further flush buckets.
*/
- bb = apr_brigade_create(ctx->f->r->pool);
- b = apr_bucket_flush_create();
+ ba = ctx->f->r->connection->bucket_alloc;
+ bb = apr_brigade_create(ctx->f->r->pool, ba);
+ b = apr_bucket_flush_create(ba);
APR_BRIGADE_INSERT_TAIL(bb, b);
if (ap_pass_brigade(ctx->f->next, bb) != APR_SUCCESS) {
php_handle_aborted_connection();
@@ -230,13 +237,6 @@
AP_MODULE_DECLARE_DATA module php4_module;
-#define INIT_CTX \
- if (ctx == NULL) { \
- /* Initialize filter context */ \
- SG(server_context) = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx)); \
- ctx->bb = apr_brigade_create(f->c->pool); \
- }
-
static int php_input_filter(ap_filter_t *f, apr_bucket_brigade *bb,
ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
{
@@ -254,7 +254,10 @@
ctx = SG(server_context);
- INIT_CTX;
+ if (ctx == NULL) {
+ /* Initialize filter context */
+ SG(server_context) = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
+ }
if ((rv = ap_get_brigade(f->next, bb, mode, block, readbytes)) != APR_SUCCESS)
{
return rv;
@@ -327,79 +330,67 @@
ap_add_common_vars(f->r);
ap_add_cgi_vars(f->r);
- ctx = SG(server_context);
- INIT_CTX;
-
- ctx->f = f;
-
- /* states:
- * 0: request startup
- * 1: collecting data
- * 2: script execution and request shutdown
- */
- if (ctx->state == 0) {
-
- apply_config(conf);
-
- ctx->state++;
-
- php_apache_request_ctor(f, ctx TSRMLS_CC);
+ if (f->ctx == NULL) {
+ /* Initialize filter context */
+ f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
+ ctx->f = f;
}
- /* moves all buckets from bb to ctx->bb */
- ap_save_brigade(f, &ctx->bb, &bb, f->r->pool);
+ if (ctx->request_processed) {
+ return ap_pass_brigade(f->next, bb);
+ }
- /* If we have received all data from the previous filters,
- * we "flatten" the buckets by creating a single string buffer.
- */
- if (ctx->state == 1 && APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(ctx->bb))) {
+ APR_BRIGADE_FOREACH(b, bb) {
zend_file_handle zfd;
- apr_bucket *eos;
- /* We want to execute only one script per request.
- * A bug in Apache or other filters could cause us
- * to reenter php_filter during script execution, so
- * we protect ourselves here.
- */
- ctx->state = 2;
+ if (!ctx->request_processed && APR_BUCKET_IS_FILE(b)) {
+ const char *path;
+ apr_bucket_brigade *prebb = bb;
+
+ /* Split the brigade into two brigades before and after
+ * the file bucket. Leave the "after the FILE" brigade
+ * in the original bb, so it gets passed outside of this
+ * loop. */
+ bb = apr_brigade_split(prebb, b);
+
+ /* Pass the "before the FILE" brigade here
+ * (if it's non-empty). */
+ if (!APR_BRIGADE_EMPTY(prebb)) {
+ apr_status_t rv;
+ rv = ap_pass_brigade(f->next, prebb);
+ /* XXX: destroy the prebb, since we know we're
+ * done with it? */
+ if (rv != APR_SUCCESS) {
+ php_handle_aborted_connection();
+ }
+ }
- /* Handle phpinfo/phpcredits/built-in images */
- if (!php_handle_special_queries(TSRMLS_C)) {
+ SG(server_context) = ctx;
+ apply_config(conf);
+ php_apache_request_ctor(f, ctx TSRMLS_CC);
+
+ apr_file_name_get(&path, ((apr_bucket_file *) b->data)->fd);
+ zfd.type = ZEND_HANDLE_FILENAME;
+ zfd.filename = (char *) path;
+ zfd.free_filename = 0;
+ zfd.opened_path = NULL;
- b = APR_BRIGADE_FIRST(ctx->bb);
+ php_execute_script(&zfd TSRMLS_CC);
+ php_apache_request_dtor(f TSRMLS_CC);
- if (APR_BUCKET_IS_FILE(b)) {
- const char *path;
+ ctx->request_processed = 1;
- apr_file_name_get(&path, ((apr_bucket_file *)
b->data)->fd);
-
- zfd.type = ZEND_HANDLE_FILENAME;
- zfd.filename = (char *) path;
- zfd.free_filename = 0;
- zfd.opened_path = NULL;
-
- php_execute_script(&zfd TSRMLS_CC);
- } else {
-
-#define PHP_NO_DATA "The PHP Filter did not receive suitable input data"
-
- eos = apr_bucket_transient_create(PHP_NO_DATA,
sizeof(PHP_NO_DATA)-1);
- APR_BRIGADE_INSERT_HEAD(bb, eos);
- }
- }
-
- php_apache_request_dtor(f TSRMLS_CC);
+ /* Delete the FILE bucket from the brigade. */
+ apr_bucket_delete(b);
- SG(server_context) = 0;
- /* Pass EOS bucket to next filter to signal end of request */
- eos = apr_bucket_eos_create();
- APR_BRIGADE_INSERT_TAIL(bb, eos);
-
- return ap_pass_brigade(f->next, bb);
- } else
- apr_brigade_destroy(bb);
+ /* We won't handle any more buckets in this brigade, so
+ * it's ok to break out now. */
+ break;
+ }
+ }
- return APR_SUCCESS;
+ /* Pass whatever is left on the brigade. */
+ return ap_pass_brigade(f->next, bb);
}
static apr_status_t
Index: sapi/apache2filter/php_apache.h
===================================================================
RCS file: /repository/php4/sapi/apache2filter/php_apache.h,v
retrieving revision 1.10
diff -u -u -r1.10 php_apache.h
--- sapi/apache2filter/php_apache.h 28 Feb 2002 08:27:20 -0000 1.10
+++ sapi/apache2filter/php_apache.h 10 Apr 2002 20:36:27 -0000
@@ -21,7 +21,6 @@
typedef struct php_struct {
int state;
- apr_bucket_brigade *bb;
ap_filter_t *f;
/* Length of post_data buffer */
int post_len;
@@ -29,6 +28,8 @@
int post_idx;
/* Buffer for request body filter */
char *post_data;
+ /* Whether or not we've processed PHP in the output filters yet. */
+ int request_processed;
} php_struct;
int php_apache_register_module(void);
--
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php
