You are right. The continue catches that case. Nice work!
> Am 11.01.2018 um 11:18 schrieb Yann Ylavic <[email protected]>:
>
> On Thu, Jan 11, 2018 at 10:55 AM, [email protected] <[email protected]>
> 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 [email protected]:
>>>
>>> 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!
