> Am 10.02.2022 um 15:26 schrieb Ruediger Pluem <rpl...@apache.org>:
>
>
>
> 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.
If my understanding of apr_table is correct, apr_table_copy() copies all
apr_table_entry_t by value. So only the key and value strings are not dup()ed
in comparison to apr_table_clone().
Replacing value pointers to a new string would then not be a problem. Modifying
a value 'in place' would show also in the original table. I don't think we do
that, but I do not think that it would pose a problem in this case. mod_http2
makes no use of the source table afterwards, really.
I think. But it is good to have your keen eyes on this.
Kind Regards,
Stefan
> Regards
>
> Rüdiger
