On Thu, Oct 8, 2015 at 1:05 PM, Stefan Eissing
<stefan.eiss...@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;
> }
