> Am 04.03.2022 um 10:22 schrieb Ruediger Pluem <rpl...@apache.org>:
>
>
>
> On 3/4/22 9:51 AM, ic...@apache.org wrote:
>> Author: icing
>> Date: Fri Mar 4 08:51:47 2022
>> New Revision: 1898586
>>
>> URL: http://svn.apache.org/viewvc?rev=1898586&view=rev
>> Log:
>> merge of 1898146,1898173 from trunk:
>>
>> *) mod_http2: preserve the port number given in a HTTP/1.1
>> request that was Upgraded to HTTP/2. Fixes PR65881.
>>
>>
>> Added:
>> httpd/httpd/branches/2.4.x/changes-entries/pr65881.txt
>> - copied unchanged from r1898146,
>> httpd/httpd/trunk/changes-entries/pr65881.txt
>> httpd/httpd/branches/2.4.x/test/modules/http2/test_502_proxy_port.py
>> - copied unchanged from r1898146,
>> httpd/httpd/trunk/test/modules/http2/test_502_proxy_port.py
>> Modified:
>> httpd/httpd/branches/2.4.x/ (props changed)
>> httpd/httpd/branches/2.4.x/modules/http2/h2_request.c
>> httpd/httpd/branches/2.4.x/test/modules/http2/env.py
>> httpd/httpd/branches/2.4.x/test/modules/http2/htdocs/cgi/hello.py
>>
>> Propchange: httpd/httpd/branches/2.4.x/
>> ------------------------------------------------------------------------------
>> Merged /httpd/httpd/trunk:r1898146,1898173
>>
>> Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_request.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_request.c?rev=1898586&r1=1898585&r2=1898586&view=diff
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/modules/http2/h2_request.c (original)
>> +++ httpd/httpd/branches/2.4.x/modules/http2/h2_request.c Fri Mar 4
>> 08:51:47 2022
>> @@ -69,12 +69,25 @@ apr_status_t h2_request_rcreate(h2_reque
>> return APR_EINVAL;
>> }
>>
>> - if (!ap_strchr_c(authority, ':') && r->server && r->server->port) {
>> - apr_port_t defport = apr_uri_port_of_scheme(scheme);
>> - if (defport != r->server->port) {
>> - /* port info missing and port is not default for scheme: append
>> */
>> - authority = apr_psprintf(pool, "%s:%d", authority,
>> - (int)r->server->port);
>> + /* The authority we carry in h2_request is the 'authority' part of
>> + * the URL for the request. r->hostname has stripped any port info that
>> + * might have been present. Do we need to add it?
>> + */
>> + if (!ap_strchr_c(authority, ':')) {
>> + if (r->parsed_uri.port_str) {
>> + /* Yes, it was there, add it again. */
>> + authority = apr_pstrcat(pool, authority, ":",
>> r->parsed_uri.port_str, NULL);
>> + }
>> + else if (!r->parsed_uri.hostname && r->server && r->server->port) {
>> + /* If there was no hostname in the parsed URL, the URL was
>> relative.
>> + * In that case, we restore port from our server->port, if it
>> + * is known and not the default port for the scheme. */
>> + apr_port_t defport = apr_uri_port_of_scheme(scheme);
>> + if (defport != r->server->port) {
>> + /* port info missing and port is not default for scheme:
>> append */
>> + authority = apr_psprintf(pool, "%s:%d", authority,
>> + (int)r->server->port);
>> + }
>> }
>> }
>
> Sorry for my late comment, but I think the above ignores the setting of
> UseCanonicalPhysicalPort and UseCanonicalName.
>
> I think we should add what is returned by ap_get_server_port in case this
> differs from apr_uri_port_of_scheme(scheme)
>
> I think of something like the below:
>
> Index: modules/http2/h2_request.c
> ===================================================================
> --- modules/http2/h2_request.c (revision 1898509)
> +++ modules/http2/h2_request.c (working copy)
> @@ -95,21 +95,13 @@
> * might have been present. Do we need to add it?
> */
> if (!ap_strchr_c(authority, ':')) {
> - if (r->parsed_uri.port_str) {
> - /* Yes, it was there, add it again. */
> - authority = apr_pstrcat(pool, authority, ":",
> r->parsed_uri.port_str, NULL);
> + apr_port_t defport = apr_uri_port_of_scheme(scheme);
> + apr_port_t port = ap_get_server_port(r);
> +
> + if (defport != port) {
> + /* port info missing and port is not default for scheme: append
> */
> + authority = apr_psprintf(pool, "%s:%d", authority, (int)port);
> }
> - else if (!r->parsed_uri.hostname && r->server && r->server->port) {
> - /* If there was no hostname in the parsed URL, the URL was
> relative.
> - * In that case, we restore port from our server->port, if it
> - * is known and not the default port for the scheme. */
> - apr_port_t defport = apr_uri_port_of_scheme(scheme);
> - if (defport != r->server->port) {
> - /* port info missing and port is not default for scheme:
> append */
> - authority = apr_psprintf(pool, "%s:%d", authority,
> - (int)r->server->port);
> - }
> - }
> }
>
> req = apr_pcalloc(pool, sizeof(*req));
>
Like in r1898593 now?
Kind Regards,
Stefan
>
> Regards
>
> Rüdiger
>
>
