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.
-aaron
p.s. this patch applies to the php4_2_0RC2 branch, and includes some
changes from HEAD that weren't yet merged into that branch, namly
the changes to use the new bucket allocator.
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
