Hmm...lines 952+: if the first data bucket has length 0, will the parser not 
still be opened on potentially uninitialized buf memory?

> Am 03.01.2018 um 14:37 schrieb yla...@apache.org:
> 
> Author: ylavic
> Date: Wed Jan  3 13:37:50 2018
> New Revision: 1819969
> 
> URL: http://svn.apache.org/viewvc?rev=1819969&view=rev
> Log:
> mod_proxy_html: follow up to r1599012.
> 
> To determine whether or not HTML data are lower than 4 bytes, use a retain
> buffer rather than assuming that all should be contained in a single bucket
> with the next one being EOS (if any).
> 
> 
> Modified:
>    httpd/httpd/trunk/modules/filters/mod_proxy_html.c
> 
> Modified: httpd/httpd/trunk/modules/filters/mod_proxy_html.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_proxy_html.c?rev=1819969&r1=1819968&r2=1819969&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_proxy_html.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_proxy_html.c Wed Jan  3 13:37:50 
> 2018
> @@ -109,6 +109,9 @@ typedef struct {
>     const char *encoding;
>     urlmap *map;
>     const char *etag;
> +    char rbuf[4];
> +    apr_size_t rlen;
> +    apr_size_t rmin;
> } saxctxt;
> 
> 
> @@ -873,6 +876,17 @@ static saxctxt *check_filter_init (ap_fi
>     return f->ctx;
> }
> 
> +static void prepend_rbuf(saxctxt *ctxt, apr_bucket_brigade *bb)
> +{
> +    if (ctxt->rlen) {
> +        apr_bucket *b = apr_bucket_transient_create(ctxt->rbuf,
> +                                                    ctxt->rlen,
> +                                                    bb->bucket_alloc);
> +        APR_BRIGADE_INSERT_HEAD(bb, b);
> +        ctxt->rlen = 0;
> +    }
> +}
> +
> static apr_status_t proxy_html_filter(ap_filter_t *f, apr_bucket_brigade *bb)
> {
>     apr_bucket* b;
> @@ -894,11 +908,15 @@ static apr_status_t proxy_html_filter(ap
>         if (APR_BUCKET_IS_METADATA(b)) {
>             if (APR_BUCKET_IS_EOS(b)) {
>                 if (ctxt->parser != NULL) {
> -                    consume_buffer(ctxt, buf, 0, 1);
> +                    consume_buffer(ctxt, "", 0, 1);
> +                }
> +                else {
> +                    prepend_rbuf(ctxt, ctxt->bb);
>                 }
>                 APR_BRIGADE_INSERT_TAIL(ctxt->bb,
> -                apr_bucket_eos_create(ctxt->bb->bucket_alloc));
> +                    apr_bucket_eos_create(ctxt->bb->bucket_alloc));
>                 ap_pass_brigade(ctxt->f->next, ctxt->bb);
> +                apr_brigade_cleanup(ctxt->bb);
>             }
>             else if (APR_BUCKET_IS_FLUSH(b)) {
>                 /* pass on flush, except at start where it would cause
> @@ -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;
> +                }
> +                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;
>                 }
> 
>                 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;
> +
> +                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;
> +                }
>                 if (ctxt->parser == NULL) {
> -                    apr_status_t rv = ap_pass_brigade(f->next, bb);
> +                    prepend_rbuf(ctxt, bb);
>                     ap_remove_output_filter(f);
> -                    return rv;
> +                    return ap_pass_brigade(f->next, bb);
>                 }
> +                ctxt->rlen = 0;
>                 apr_pool_cleanup_register(f->r->pool, ctxt->parser,
>                                           (int(*)(void*))htmlFreeParserCtxt,
>                                           apr_pool_cleanup_null);
> 
> 

Reply via email to