On 2/10/22 2:18 PM, Stefan Eissing wrote:
>
>
>> Am 10.02.2022 um 14:00 schrieb Ruediger Pluem <rpl...@apache.org>:
>>
>>
>>
>> On 2/10/22 11:59 AM, ic...@apache.org wrote:
>>> Author: icing
>>> Date: Thu Feb 10 10:59:08 2022
>>> New Revision: 1897940
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1897940&view=rev
>>> Log:
>>> *) mod_http2: :scheme pseudo-header values, not matching the
>>> connection scheme, are forwarded via absolute uris to the
>>> http protocol processing to preserve semantics of the request.
>>> Checks on combinations of pseudo-headers values/absence
>>> have been added as described in RFC 7540.
>>> Fixes <https://github.com/icing/mod_h2/issues/230>.
>>>
>>>
>>> Modified:
>>> httpd/httpd/trunk/changes-entries/http2_request_scheme.txt
>>> httpd/httpd/trunk/modules/http2/h2_c2.c
>>> httpd/httpd/trunk/modules/http2/h2_request.c
>>> httpd/httpd/trunk/modules/http2/h2_stream.c
>>> httpd/httpd/trunk/test/modules/http2/test_003_get.py
>>>
>>
>>> Modified: httpd/httpd/trunk/modules/http2/h2_request.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_request.c?rev=1897940&r1=1897939&r2=1897940&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/http2/h2_request.c (original)
>>> +++ httpd/httpd/trunk/modules/http2/h2_request.c Thu Feb 10 10:59:08 2022
>>
>>> @@ -295,9 +300,30 @@ request_rec *h2_create_request_rec(const
>>>
>>> /* 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",
>>> - req->method, req->path ? req->path : "");
>>> - r->headers_in = apr_table_clone(r->pool, req->headers);
>>> + AP_DEBUG_ASSERT(req->authority);
>>> + if (req->scheme && (ap_cstr_casecmp(req->scheme,
>>> + ap_ssl_conn_is_ssl(c->master? c->master : c)?
>>> "https" : "http")
>>> + || !ap_cstr_casecmp("CONNECT", req->method))) {
>>> + /* Client sent a non-matching ':scheme' pseudo header. Forward this
>>> + * via an absolute URI in the request line.
>>> + */
>>> + r->the_request = apr_psprintf(r->pool, "%s %s://%s%s HTTP/2.0",
>>> + req->method, req->scheme,
>>> req->authority,
>>> + req->path ? req->path : "");
>>> + }
>>> + else if (req->path) {
>>> + r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0",
>>> + req->method, req->path);
>>> + }
>>> + else {
>>> + /* We should only come here on a request that is errored already.
>>> + * create a request line that passes parsing, we'll die anyway.
>>> + */
>>> + AP_DEBUG_ASSERT(req->http_status != H2_HTTP_STATUS_UNSET);
>>> + r->the_request = apr_psprintf(r->pool, "%s / HTTP/2.0",
>>> req->method);
>>> + }
>>> +
>>> + r->headers_in = apr_table_copy(r->pool, req->headers);
>>
>> Why no longer cloning? Are all req->headers elements allocated from r->pool
>> anyway and are possible modifications of the headers
>> on one side no issue for the other side?
>
> The table itself can be modified during processing. The existing values and
> keys live in stream->pool and that one lives as long as the request. So, a
> copy of the table should suffice, IMO.
Agree on the lifecycle topic. What if header (values) would get changed in
place on either side? Do you see an issue for the other
side or are we sure that these changes never happen and if values to headers
need to be changed the pointer to the value will be
replaced in the respective table with a new pointer to the new value.
Regards
Rüdiger
