Re: Two mod_include problems

[Paul J. Reder]

The "ctx->bytes_parsed = 0;" *should* be inside the if block.
bytes_parsed should only be reset to 0 if the parsed bytes are
passed on (via ap_pass_brigade()). bytes_parsed is supposed to
indicate the number of bytes in the currently held brigade
which have been parsed. If it exceeds BYTE_COUNT_THRESHOLD then
we forward the initial part of the brigade. This is to keep from
buffering up huge chunks of data.
I concur with the rest of the patch (from a visual perspective,
I haven't tested it).
Paul J. Reder

Cliff Woolley wrote:
yOn Wed, 9 Jul 2003, Ron Park wrote:


This problem revolves around the use of 'if' 'else' and 'elseif'.
Originally, we thought the problem was related to <!--#else -->
but later we found an troublesome <!--#if ... --> that lead to
us finding the real culprit.


Andre and I counter-propose the following patch for this problem.  Can you
please test it on your test cases?
Also note that there's that one line at the bottom that sets
ctx->bytes_parsed=0, and we weren't sure if it should go inside or outside
that if().  Comments?
Thanks,
Cliff


Index: mod_include.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
retrieving revision 1.233
diff -u -d -r1.233 mod_include.c
--- mod_include.c       3 Feb 2003 17:53:01 -0000       1.233
+++ mod_include.c       10 Jul 2003 02:24:25 -0000
@@ -2956,7 +2956,7 @@
             /* If I am inside a conditional (if, elif, else) that is false
              *   then I need to throw away anything contained in it.
              */
-            if ((!(ctx->flags & FLAG_PRINTING)) && (tmp_dptr != NULL) &&
+            if ((!(ctx->flags & FLAG_PRINTING)) &&
                 (dptr != APR_BRIGADE_SENTINEL(*bb))) {
                 while ((dptr != APR_BRIGADE_SENTINEL(*bb)) &&
                        (dptr != tmp_dptr)) {
@@ -3197,25 +3197,16 @@
      *   once the whole tag has been found.
      */
     if (ctx->state == PRE_HEAD) {
-        /* Inside a false conditional (if, elif, else), so toss it all... */
-        if ((dptr != APR_BRIGADE_SENTINEL(*bb)) &&
-            (!(ctx->flags & FLAG_PRINTING))) {
-            apr_bucket *free_bucket;
-            do {
-                free_bucket = dptr;
-                dptr = APR_BUCKET_NEXT (dptr);
-                apr_bucket_delete(free_bucket);
-            } while (dptr != APR_BRIGADE_SENTINEL(*bb));
-        }
-        else {
-            /* Otherwise pass it along...
+        if (!APR_BRIGADE_EMPTY(*bb)) {
+            /* Pass it along...
              * No SSI tags in this brigade... */
             rv = ap_pass_brigade(f->next, *bb);
             if (rv != APR_SUCCESS) {
                 return rv;
             }
-            ctx->bytes_parsed = 0;
         }
+        /* XXX: should this line go inside the if(!empty)? */
+        ctx->bytes_parsed = 0;
     }
     else if (ctx->state == PARSED) {         /* Invalid internal condition... */
         apr_bucket *content_head = NULL, *tmp_bkt;
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein