Re: svn commit: r692567 - /httpd/httpd/trunk/modules/filters/mod_charset_lite.c

[Ruediger Pluem]

On 09/06/2008 12:21 AM, [EMAIL PROTECTED] wrote:

Author: gregames
Date: Fri Sep  5 15:21:36 2008
New Revision: 692567

URL: http://svn.apache.org/viewvc?rev=692567&view=rev
Log:
PR 45687: Detect and pass along error buckets

Submitted by: Dan Poirier <poirier pobox.org> Reviewed by: trawick

Modified:
    httpd/httpd/trunk/modules/filters/mod_charset_lite.c

Modified: httpd/httpd/trunk/modules/filters/mod_charset_lite.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_charset_lite.c?rev=692567&r1=692566&r2=692567&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_charset_lite.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_charset_lite.c Fri Sep  5 15:21:36 
2008
@@ -414,6 +414,23 @@
     return rv;
 }

+static apr_status_t send_bucket_downstream(ap_filter_t *f, apr_bucket *b)

+{
+    request_rec *r = f->r;
+    conn_rec *c = r->connection;
+    apr_bucket_brigade *bb;
+    charset_filter_ctx_t *ctx = f->ctx;
+    apr_status_t rv;
+
+    bb = apr_brigade_create(r->pool, c->bucket_alloc);

This is bad. Please store the brigade in the filter context and reuse it,
by cleaning it. See also 
http://httpd.apache.org/docs/trunk/en/developer/output-filters.html#filtering
The same error is in send_downstream and IMHO they can share the same brigade.

+    APR_BRIGADE_INSERT_TAIL(bb, b);
+    rv = ap_pass_brigade(f->next, bb);
+    if (rv != APR_SUCCESS) {
+        ctx->ees = EES_DOWNSTREAM;
+    }
+    return rv;
+}
+
 static apr_status_t set_aside_partial_char(charset_filter_ctx_t *ctx,
                                            const char *partial,
                                            apr_size_t partial_len)

@@ -673,7 +690,7 @@
             }
             b = APR_BRIGADE_FIRST(bb);
             if (b == APR_BRIGADE_SENTINEL(bb) ||
-                APR_BUCKET_IS_EOS(b)) {
+                APR_BUCKET_IS_METADATA(b)) {
                 break;
             }
             rv = apr_bucket_read(b, &bucket, &bytes_in_bucket, APR_BLOCK_READ);
@@ -892,6 +909,17 @@
                 }
                 break;
             }
+            if (APR_BUCKET_IS_METADATA(dptr)) {

Don't we need to handle flush buckets separately and flush our buffer downstream
as far as we can in this case?

+                apr_bucket *metadata_bucket;
+                metadata_bucket = dptr;
+                dptr = APR_BUCKET_NEXT(dptr);
+                APR_BUCKET_REMOVE(metadata_bucket);
+                rv = send_bucket_downstream(f, metadata_bucket);
+                if (rv != APR_SUCCESS) {
+                    done = 1;
+                }
+                continue;
+            }
             rv = apr_bucket_read(dptr, &cur_str, &cur_len, APR_BLOCK_READ);
             if (rv != APR_SUCCESS) {
                 done = 1;

Regards

Rüdiger