On 6/1/22 12:44 PM, Yann Ylavic wrote:
> On Wed, Jun 1, 2022 at 12:39 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 6/1/22 11:56 AM, yla...@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
