On Wed, Dec 20, 2023 at 4:56 PM <jor...@apache.org> wrote:
>
> Author: jorton
> Date: Wed Dec 20 15:56:15 2023
> New Revision: 1914804
>
> URL: http://svn.apache.org/viewvc?rev=1914804&view=rev
> Log:
> * modules/http/chunk_filter.c (ap_http_chunk_filter): For a brigade
>   containing [FLUSH EOS], insert the last-chunk terminator before the
>   FLUSH rather than between the FLUSH and the EOS.
>
> Github: closes #400

It seems that we should set the last-chunk before the FLUSH only if
the latter precedes an EOS?

So below:

> --- httpd/httpd/trunk/modules/http/chunk_filter.c (original)
> +++ httpd/httpd/trunk/modules/http/chunk_filter.c Wed Dec 20 15:56:15 2023
> @@ -64,7 +64,7 @@ apr_status_t ap_http_chunk_filter(ap_fil
>
>      for (more = tmp = NULL; b; b = more, more = NULL) {
>          apr_off_t bytes = 0;
> -        apr_bucket *eos = NULL;
> +        apr_bucket *eos = NULL; /* EOS bucket, or FLUSH preceding EOS */
>          apr_bucket *flush = NULL;
>          /* XXX: chunk_hdr must remain at this scope since it is used in a
>           *      transient bucket.
> @@ -99,8 +99,20 @@ apr_status_t ap_http_chunk_filter(ap_fil
>                  }
>                  if (APR_BUCKET_IS_FLUSH(e)) {
>                      flush = e;

Don't set "flush" above.

> +
> +                    /* Special case to catch common brigade ending of
> +                     * [FLUSH] [EOS] - insert the last_chunk before
> +                     * the FLUSH rather than between the FLUSH and the
> +                     * EOS. */
>                      if (e != APR_BRIGADE_LAST(b)) {
> -                        more = apr_brigade_split_ex(b, APR_BUCKET_NEXT(e), 
> tmp);
> +                        if (APR_BUCKET_IS_EOS(APR_BUCKET_NEXT(e))) {

But here only?

> +                            eos = e;
> +                            /* anything after EOS is dropped, no need
> +                             * to split. */
> +                        }
> +                        else {
> +                            more = apr_brigade_split_ex(b, 
> APR_BUCKET_NEXT(e), tmp);
> +                        }
>                      }
>                      break;
>                  }
> @@ -173,12 +185,12 @@ apr_status_t ap_http_chunk_filter(ap_fil
>               * FLUSH bucket, or appended to the brigade
>               */
>              e = apr_bucket_immortal_create(CRLF_ASCII, 2, c->bucket_alloc);
> -            if (eos != NULL) {
> -                APR_BUCKET_INSERT_BEFORE(eos, e);
> -            }
> -            else if (flush != NULL) {
> +            if (flush != NULL) {
>                  APR_BUCKET_INSERT_BEFORE(flush, e);
>              }
> +            else if (eos != NULL) {
> +                APR_BUCKET_INSERT_BEFORE(eos, e);
> +            }
>              else {
>                  APR_BRIGADE_INSERT_TAIL(b, e);
>              }


Regards;
Yann.

Reply via email to