> Am 15.06.2020 um 21:46 schrieb Ruediger Pluem <[email protected]>:
>
> I would like to unblock the test failure on trunk soon. Any comments on the
> below?
I am not really familiar with ap_parse_request_line() and why it was added
there. Yann?
As to "faking" the http version of a request, that does not look good. Do we
need to preserve the fairy tale for our code that everything is HTTP/1.x?
- Stefan
> 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
>>>
>>
