Re: svn commit: r1898586 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr65881.txt modules/http2/h2_request.c test/modules/http2/env.py test/modules/http2/htdocs/cgi/hello.py test/modules/http2
> Am 04.03.2022 um 11:09 schrieb Ruediger Pluem : > > > > On 3/4/22 10:50 AM, Stefan Eissing wrote: >> >> >>> Am 04.03.2022 um 10:22 schrieb Ruediger Pluem : >>> >>> >>> >>> 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=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=1898585=1898586=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. >>> -
Re: svn commit: r1898586 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr65881.txt modules/http2/h2_request.c test/modules/http2/env.py test/modules/http2/htdocs/cgi/hello.py test/modules/http2
On 3/4/22 10:50 AM, Stefan Eissing wrote: > > >> Am 04.03.2022 um 10:22 schrieb Ruediger Pluem : >> >> >> >> 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=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=1898585=1898586=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 =
Re: svn commit: r1898586 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr65881.txt modules/http2/h2_request.c test/modules/http2/env.py test/modules/http2/htdocs/cgi/hello.py test/modules/http2
> Am 04.03.2022 um 10:22 schrieb Ruediger Pluem : > > > > 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=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=1898585=1898586=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
Re: svn commit: r1898586 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr65881.txt modules/http2/h2_request.c test/modules/http2/env.py test/modules/http2/htdocs/cgi/hello.py test/modules/http2
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=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=1898585=1898586=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); -} -} }