I would like to unblock the test failure on trunk soon. Any comments on the 
below?

Regards

Rüdiger

On 6/10/20 4:31 PM, Ruediger Pluem wrote:
> Sorry for not testing before and breaking stuff with r1878708. There are 
> basically two failures here:
> 
> 1. The test failure in t/apache/http_strict.t is because I missed to adjust 
> the test and I think the following would do
>    (at least it now passes again):
> 
> Index: t/apache/http_strict.t
> ===================================================================
> --- t/apache/http_strict.t    (revision 1878712)
> +++ t/apache/http_strict.t    (working copy)
> @@ -12,6 +12,10 @@
>                  need_min_apache_fix("2.4.34", "2.5.1") :
>                  need_min_apache_version('2.4.34');
> 
> +my $test_unsupported_version = defined(&need_min_apache_fix) ?
> +                need_min_apache_fix("2.5.1") :
> +                need_min_apache_version('2.5.1');
> +
>  # possible expected results:
>  #   0:       any HTTP error
>  #   1:       any HTTP success
> @@ -32,7 +36,7 @@
>      [ "GET\t/ HTTP/1.0\r\n\r\n"                               => 400],
>      [ "GET / HTT/1.0\r\n\r\n"                                 =>   0],
>      [ "GET / HTTP/1.0\r\nHost: localhost\r\n\r\n"             =>   1],
> -    [ "GET / HTTP/2.0\r\nHost: localhost\r\n\r\n"             =>   1],
> +    [ "GET / HTTP/2.0\r\nHost: localhost\r\n\r\n"             =>   1, 505, 
> $test_unsupported_version],
>      [ "GET / HTTP/1.2\r\nHost: localhost\r\n\r\n"             =>   1],
>      [ "GET / HTTP/1.11\r\nHost: localhost\r\n\r\n"            => 400],
>      [ "GET / HTTP/10.0\r\nHost: localhost\r\n\r\n"            => 400],
> 
> 
> 2. The test failures in t/modules/http2.t are caused because in 
> modules/http2/h2_request.c(h2_request_create_rec)
>    we call ap_parse_request_line with a static 'HTTP/2.0' version string in 
> order to
>    'Validate HTTP/1 request' as the comment states. In practice this request 
> that we give to ap_parse_request_line
>    should be a valid HTTP/1.x request as otherwise ap_parse_request_line 
> could fail. So I though of the following patch
>    below which makes the tests pass again. I will reset r->proto_num and 
> r->protocol afterwards to ensure that for
>    further processing (e.g. env vars, logging) it is clear that this was 
> actually a HTTP/2.0 request and keep the previous
>    behavior with regards to this.
>    Comments or different approaches?
> 
> Index: modules/http2/h2_request.c
> ===================================================================
> --- modules/http2/h2_request.c        (revision 1878470)
> +++ modules/http2/h2_request.c        (working copy)
> @@ -278,7 +278,11 @@ request_rec *h2_request_create_rec(const h2_reques
> 
>      /* Time to populate r with the data we have. */
>      r->request_time = req->request_time;
> -    r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0",
> +    /*
> +     * Use HTTP/1.1 as ap_parse_request_line only deals with
> +     * HTTP/1.x requests.
> +     */
> +    r->the_request = apr_psprintf(r->pool, "%s %s HTTP/1.1",
>                                    req->method, req->path ? req->path : "");
>      r->headers_in = apr_table_clone(r->pool, req->headers);
> 
> @@ -293,8 +297,14 @@ request_rec *h2_request_create_rec(const h2_reques
>          r->per_dir_config = r->server->lookup_defaults;
>          access_status = r->status;
>          r->status = HTTP_OK;
> +        /* Note that this is actually a HTTP/2.0 request */
> +        r->proto_num = HTTP_VERSION(2,0);
> +        r->protocol  = "HTTP/2.0";
>          goto die;
>      }
> +    /* Note that this is actually a HTTP/2.0 request */
> +    r->proto_num = HTTP_VERSION(2,0);
> +    r->protocol  = "HTTP/2.0";
> 
>      /* we may have switched to another server */
>      r->per_dir_config = r->server->lookup_defaults;
> 
> 
> Regards
> 
> Rüdiger
> 
> On 6/10/20 2:46 PM, Travis CI wrote:
>> apache
>>
>> /
>>
>> httpd
>>
>> <https://travis-ci.org/github/apache/httpd?utm_medium=notification&utm_source=email>
>>
>> branch icontrunk <https://github.com/apache/httpd/tree/trunk>
>>
>> build has failed
>> Build #804 was broken 
>> <https://travis-ci.org/github/apache/httpd/builds/696809088?utm_medium=notification&utm_source=email>
>> arrow to build time
>> clock icon12 mins and 35 secs
>>
>> Ruediger Pluem avatarRuediger Pluem
>>
>> 97bc128 CHANGESET → 
>> <https://github.com/apache/httpd/compare/75350541f0af...97bc128df241>
>>
>> * Have the HTTP 0.9 / 1.1 processing code reject requests for
>> HTTP >= 2.0 with a HTTP Version Not Support status code.
>>
>>
>> git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1878708 
>> 13f79535-47bb-0310-9956-ffa450edef68
>>
> 

Reply via email to