Re: svn commit: r1879546 - in /httpd/httpd/trunk: CHANGES modules/http2/h2_from_h1.c

[Ruediger Pluem <rp...@apache.org>]

On 7/6/20 2:13 PM, rpluem@apache.org wrote:
> Author: rpluem
> Date: Mon Jul  6 12:13:33 2020
> New Revision: 1879546
> 
> URL: http://svn.apache.org/viewvc?rev=1879546&view=rev
> Log:
> * Make get_line more robust in the case that it is called multiple times:
> 
>   - Safe the brigade between mutiple calls to correctly handle transient
>     buckets.
>   - Detect possible endless loops.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/http2/h2_from_h1.c
> 

> Modified: httpd/httpd/trunk/modules/http2/h2_from_h1.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_from_h1.c?rev=1879546&r1=1879545&r2=1879546&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_from_h1.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_from_h1.c Mon Jul  6 12:13:33 2020

> @@ -351,13 +352,17 @@ static apr_status_t get_line(h2_response
>          parser->tmp = apr_brigade_create(task->pool, task->c->bucket_alloc);
>      }
>      status = apr_brigade_split_line(parser->tmp, bb, APR_BLOCK_READ, 
> -                                    HUGE_STRING_LEN);
> +                                    len);
>      if (status == APR_SUCCESS) {
>          --len;
>          status = apr_brigade_flatten(parser->tmp, line, &len);
>          if (status == APR_SUCCESS) {
>              /* we assume a non-0 containing line and remove trailing crlf. */
>              line[len] = '\0';
> +            /*
> +             * XXX: What to do if there is an LF but no CRLF?
> +             *      Should we error out?
> +             */
>              if (len >= 2 && !strcmp(H2_CRLF, line + len - 2)) {
>                  len -= 2;
>                  line[len] = '\0';
> @@ -367,10 +372,47 @@ static apr_status_t get_line(h2_response
>                                task->id, line);
>              }
>              else {
> +                apr_off_t brigade_length;
> +
> +                /*
> +                 * If the brigade parser->tmp becomes longer than our buffer
> +                 * for flattening we never have a chance to get a complete
> +                 * line. This can happen if we are called multiple times after
> +                 * previous calls did not find a H2_CRLF and we returned
> +                 * APR_EAGAIN. In this case parser->tmp (correctly) grows
> +                 * with each call to apr_brigade_split_line.
> +                 *
> +                 * XXX: Currently a stack based buffer of HUGE_STRING_LEN is
> +                 * used. This means we cannot cope with lines larger than
> +                 * HUGE_STRING_LEN which might be an issue.
> +                 */


Any suggestions on both XXX comments above?

Regards

Rüdiger