httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: stop the press!
Date Thu, 08 Oct 2015 11:26:53 GMT
On Thu, Oct 8, 2015 at 1:05 PM, Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
> Yann, care to apply your eyeballs to this one? Old one is reverted. Thanks!
>
> Index: modules/http2/h2_util.c
> ===================================================================
> --- modules/http2/h2_util.c     (revision 1707479)
> +++ modules/http2/h2_util.c     (working copy)
> @@ -539,14 +539,36 @@
>                                apr_size_t *plen, int *peos)
>  {
>      apr_status_t status;
> +    apr_off_t blen = 0;
> +
>      /* test read to determine available length */
> -    apr_off_t blen = 0;
> -    status = apr_brigade_length(bb, 0, &blen);
> -    if (blen < (apr_off_t)*plen) {
> -        *plen = blen;
> +    status = apr_brigade_length(bb, 1, &blen);
> +    if (status != APR_SUCCESS) {
> +        return status;
>      }
> -    *peos = h2_util_has_eos(bb, *plen);
> -    return status;
> +    else if (blen == 0) {
> +        /* empty brigade, does it have an EOS bucket somwhere? */
> +        *plen = 0;
> +        *peos = h2_util_has_eos(bb, 0);
> +    }
> +    else if (blen > 0) {
> +        /* data in the brigade, limit the length returned. Check for EOS
> +         * bucket only if we indicate data. This is required since plen == 0
> +         * means "the whole brigade" for h2_util_hash_eos()
> +         */
> +        if (blen < (apr_off_t)*plen) {
> +            *plen = blen;
> +        }
> +        *peos = (*plen > 0)? h2_util_has_eos(bb, *plen) : 0;

Maybe last_not_included() could "eat" leading metadata buckets when
called with maxlen == 0, that is remove the leading "if (maxlen > 0)"
in there.
Hence we could call h2_util_has_eos(bb, *plen) unconditionally here
and still still detect EOS.

Can't tell about the other uses of ast_not_included() though...

> +    }
> +    else if (blen < 0) {
> +        /* famous SHOULD NOT HAPPEN, sinc we told apr_brigade_length to readall
> +         */
> +        *plen = 0;
> +        *peos = h2_util_has_eos(bb, 0);
> +        return APR_EINVAL;
> +    }

This seems not needed, maybe change (blen == 0) to (blen <= 0) above
to protect from stray photons?


> +    return APR_SUCCESS;
>  }

Mime
View raw message