You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2022/06/01 10:39:19 UTC
Re: svn commit: r1901485 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On 6/1/22 11:56 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Wed Jun 1 09:56:43 2022
> New Revision: 1901485
>
> URL: http://svn.apache.org/viewvc?rev=1901485&view=rev
> Log:
> mod_proxy: Let fixup hooks know about the Host header (and eventually overwrite it).
>
> If proxy_run_fixups() sets a Host header there will be two ones sent to the
> origin server.
>
> Instead, let the hooks know about the Host by setting it in the r->headers_in
> passed to proxy_run_fixups(), and use the actual value afterwards.
> Note: if proxy_run_fixups() unsets the Host we'll keep ours.
>
> Suggested by: rpluem
>
>
> Modified:
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1901485&r1=1901484&r2=1901485&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Jun 1 09:56:43 2022
> @@ -3975,9 +3975,7 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> if (force10)
> apr_table_unset(r->headers_in, "Trailer");
>
> - /* We used to send `Host: ` always first, so let's keep it that
> - * way. No telling which legacy backend is relying no this.
> - */
> + /* Compute Host header */
> if (dconf->preserve_host == 0) {
> if (ap_strchr_c(uri->hostname, ':')) { /* if literal IPv6 address */
> if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
> @@ -3994,10 +3992,11 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> host = uri->hostname;
> }
> }
> + apr_table_setn(r->headers_in, "Host", host);
> }
> else {
> - /* don't want to use r->hostname, as the incoming header might have a
> - * port attached
> + /* don't want to use r->hostname as the incoming header might have a
> + * port attached, let's use the original header.
> */
> host = saved_host;
> if (!host) {
> @@ -4007,10 +4006,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> "on incoming request and preserve host set "
> "forcing hostname to be %s for uri %s",
> host, r->uri);
> + apr_table_setn(r->headers_in, "Host", host);
> }
> }
Nitpick: Can't we do the apr_table_setn(r->headers_in, "Host", host); here instead of in each if/else branch?
> - ap_h1_append_header(header_brigade, r->pool, "Host", host);
> - apr_table_unset(r->headers_in, "Host");
>
> /* handle Via */
> if (conf->viaopt == via_block) {
Regards
Rüdiger
Re: svn commit: r1901485 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 6/1/22 12:44 PM, Yann Ylavic wrote:
> On Wed, Jun 1, 2022 at 12:39 PM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 6/1/22 11:56 AM, ylavic@apache.org wrote:
>>>
>>> + /* Compute Host header */
>>> if (dconf->preserve_host == 0) {
>>> if (ap_strchr_c(uri->hostname, ':')) { /* if literal IPv6 address */
>>> if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
>>> @@ -3994,10 +3992,11 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
>>> host = uri->hostname;
>>> }
>>> }
>>> + apr_table_setn(r->headers_in, "Host", host);
>>> }
>>> else {
>>> - /* don't want to use r->hostname, as the incoming header might have a
>>> - * port attached
>>> + /* don't want to use r->hostname as the incoming header might have a
>>> + * port attached, let's use the original header.
>>> */
>>> host = saved_host;
>>> if (!host) {
>>> @@ -4007,10 +4006,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
>>> "on incoming request and preserve host set "
>>> "forcing hostname to be %s for uri %s",
>>> host, r->uri);
>>> + apr_table_setn(r->headers_in, "Host", host);
>>> }
>>> }
>>
>> Nitpick: Can't we do the apr_table_setn(r->headers_in, "Host", host); here instead of in each if/else branch?
>
> This is a small optimization where if we reuse the existing Host
> header (saved_host) we don't need to set it again.
> But if it harms readability I can certainly change it as you say.
No worries. Leave it as is then. I vote for the performance benefit over the readability benefit in this case :-)
Regards
Rüdiger
Re: svn commit: r1901485 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Jun 1, 2022 at 12:39 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 6/1/22 11:56 AM, ylavic@apache.org wrote:
> >
> > + /* Compute Host header */
> > if (dconf->preserve_host == 0) {
> > if (ap_strchr_c(uri->hostname, ':')) { /* if literal IPv6 address */
> > if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
> > @@ -3994,10 +3992,11 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> > host = uri->hostname;
> > }
> > }
> > + apr_table_setn(r->headers_in, "Host", host);
> > }
> > else {
> > - /* don't want to use r->hostname, as the incoming header might have a
> > - * port attached
> > + /* don't want to use r->hostname as the incoming header might have a
> > + * port attached, let's use the original header.
> > */
> > host = saved_host;
> > if (!host) {
> > @@ -4007,10 +4006,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> > "on incoming request and preserve host set "
> > "forcing hostname to be %s for uri %s",
> > host, r->uri);
> > + apr_table_setn(r->headers_in, "Host", host);
> > }
> > }
>
> Nitpick: Can't we do the apr_table_setn(r->headers_in, "Host", host); here instead of in each if/else branch?
This is a small optimization where if we reuse the existing Host
header (saved_host) we don't need to set it again.
But if it harms readability I can certainly change it as you say.
Regards;
Yann.