On Wed, Apr 1, 2020 at 2:39 PM Joe Orton <[email protected]> wrote: > > On Wed, Apr 01, 2020 at 02:04:21PM +0200, Ruediger Pluem wrote: > > On 3/30/20 3:18 PM, [email protected] wrote: > > > > > > - rv = apr_bucket_split(e, COALESCE_BYTES - (buffered + bytes)); > > > + /* If the read above made the bucket morph, it may now fit > > > + * entirely within the buffer. Otherwise, split it so it does > > > + * fit. */ > > > + if (e->length < COALESCE_BYTES > > > + && e->length + buffered + bytes < COALESCE_BYTES) { > > > + rv = APR_SUCCESS; > > > > Hmm. If we had e->length == -1 above and the bucket read failed, e might > > still be the morphing bucket and hence e->length == -1. > > I think all the code below assumes e->length >= 0 things can get off the > > rails. > > Thanks a lot for the review... I tried to keep that as simple as > possible but there are too many cases to cover. Yep, you're right. > > > How about the following patch (minus whitespace changes) to fix this: > > +1 that looks correct to me, please commit (or I can...)
+1, a morphing bucket that reads to another morphing bucket is way more terrible than one returning EOF :) Speaking of the latter, r1876000 ;)
