You are right. The continue catches that case. Nice work!

> Am 11.01.2018 um 11:18 schrieb Yann Ylavic <ylavic....@gmail.com>:
> 
> On Thu, Jan 11, 2018 at 10:55 AM, ste...@eissing.org <ste...@eissing.org> 
> wrote:
>> Hmm...lines 952+: if the first data bucket has length 0, will the
>> parser not still be opened on potentially uninitialized buf memory?
> 
> I tried to make so that zero length buckets (actually any leading
> bucket until "ctxt->rmin" is reached) are caught by the "continue" at
> 945. Do you see an overlook on my side?
> 
> See below comments...
> 
>> 
>>> Am 03.01.2018 um 14:37 schrieb yla...@apache.org:
>>> 
>>> Modified: httpd/httpd/trunk/modules/filters/mod_proxy_html.c
> []
>>> static apr_status_t proxy_html_filter(ap_filter_t *f, apr_bucket_brigade 
>>> *bb)
>>> {
>>>    apr_bucket* b;
> []
>>> @@ -918,9 +936,18 @@ static apr_status_t proxy_html_filter(ap
>>>                 * HTML rewriting. The URL schema (i.e. 'http') needs four 
>>> bytes alone.
>>>                 * And the HTML parser needs at least four bytes to 
>>> initialise correctly.
>>>                 */
>>> -                if ((bytes < 4) && APR_BUCKET_IS_EOS(APR_BUCKET_NEXT(b))) {
>>> -                    ap_remove_output_filter(f);
>>> -                    return ap_pass_brigade(f->next, bb);
>>> +                ctxt->rmin += bytes;
>>> +                if (ctxt->rmin < sizeof(ctxt->rbuf)) {
>>> +                    memcpy(ctxt->rbuf + ctxt->rlen, buf, bytes);
>>> +                    ctxt->rlen += bytes;
>>> +                    continue;
> 
> Here, supposedly, leading empty buckets should be ignored.
> 
> 
>>> +                }
>>> +                if (ctxt->rlen && ctxt->rlen < sizeof(ctxt->rbuf)) {
>>> +                    apr_size_t rem = sizeof(ctxt->rbuf) - ctxt->rlen;
>>> +                    memcpy(ctxt->rbuf + ctxt->rlen, buf, rem);
>>> +                    ctxt->rlen += rem;
>>> +                    buf += rem;
>>> +                    bytes -= rem;
>>>                }
> 
> So from here we should either have the bucket's buf >= 4 bytes or
> "ctxt->rbuf" filled in.
> 
>>> 
>>>                if (!xml2enc_charset ||
>>> @@ -949,15 +976,25 @@ static apr_status_t proxy_html_filter(ap
>>>                }
>>> 
>>>                ap_fputs(f->next, ctxt->bb, ctxt->cfg->doctype);
>>> -                ctxt->parser = htmlCreatePushParserCtxt(&sax, ctxt, buf,
>>> -                                                        4, 0, enc);
>>> -                buf += 4;
>>> -                bytes -= 4;
> 
> The two cases being handled below:
>>> +
>>> +                if (ctxt->rlen) {
>>> +                    ctxt->parser = htmlCreatePushParserCtxt(&sax, ctxt,
>>> +                                                            ctxt->rbuf,
>>> +                                                            ctxt->rlen,
>>> +                                                            NULL, enc);
>>> +                }
>>> +                else {
>>> +                    ctxt->parser = htmlCreatePushParserCtxt(&sax, ctxt, 
>>> buf, 4,
>>> +                                                            NULL, enc);
>>> +                    buf += 4;
>>> +                    bytes -= 4;
>>> +                }
> 
> 
> Thanks for the review!

Reply via email to