[PATCH] Use port in Host header for redirects

[rian at thelig.ht]

Hi there,

Just to explain the motivation for this patch. I run nginx behind a port
forwarding tcp proxy at my home under a different port from the one
nginx is listening on. When nginx generates redirects for URIs without
trailing slashes, the "Location" header is incorrect. For instance, if I
navigate to "https://myhome.com:2763/dir" it would send me to
"https://myhome.com/dir/".

The documentation mentions a configuration option called
"server_name_in_redirect" but that only applies to the using host name
without the trailing port from the "Host" header to generate the
"Location" header. This is due to the combination of
ngx_http_validate_host() and logic in ngx_http_header_filter().

I'm not sure how intentional the preexisting logic was but it seemed
incomplete. Going further, if nginx is serving http behind a transparent
SSL proxy then it will generate incorrect "Location" header in that case
as well. I recommend a general overhaul of "Location" header generation
logic and/or the server_name_in_redirect / port_in_redirect options.
Intuitively, it seems the least brittle option is to not modify the
relative URIs in the "Location" header at all but there might be an
important reason for it that I'm unaware of.

Thanks for reading!

Rian


HG changeset patch
# User Rian Hunter <rian at thelig.ht>
# Date 1427071183 25200
#      Sun Mar 22 17:39:43 2015 -0700
# Node ID 389f04c80ffa30078529e73504dbea533d079894
# Parent  8e66a83d16ae07fa1f3a5e93e60749da63175653
Use port in Host header for redirects

Before this change, if server_name_in_redirect was false, nginx would
use the hostname from the Host header for constructing a URI for the
Location
header during a redirect. The problem with this was that it would ignore
the trailing ":port" in the Host header.

This change makes it so nginx respects the trailing ":port" from the
Host
header if it exists when constructing Location headers during a
redirect.

diff -r 8e66a83d16ae -r 389f04c80ffa
src/http/ngx_http_header_filter_module.c
--- a/src/http/ngx_http_header_filter_module.c    Thu Mar 19 19:29:43 
2015
+0300
+++ b/src/http/ngx_http_header_filter_module.c    Sun Mar 22 17:39:43 
2015
-0700
@@ -320,10 +320,10 @@
           if (clcf->server_name_in_redirect) {
               cscf = ngx_http_get_module_srv_conf(r,
ngx_http_core_module);
               host = cscf->server_name;
-
+        } else if (r->headers_in.host && r->headers_in.host->value.len)
{
+            host = r->headers_in.host->value;
           } else if (r->headers_in.server.len) {
               host = r->headers_in.server;
-
           } else {
               host.len = NGX_SOCKADDR_STRLEN;
               host.data = addr;
@@ -333,44 +333,50 @@
               }
           }

-        switch (c->local_sockaddr->sa_family) {
+        len += sizeof("Location: https://") - 1
+          + host.len
+          + r->headers_out.location->value.len + 2;
+
+        /* only add port if there isn't one in the host str */
+        if (!ngx_strnstr(host.data, ":", host.len)) {
+          switch (c->local_sockaddr->sa_family) {

   #if (NGX_HAVE_INET6)
-        case AF_INET6:
+          case AF_INET6:
               sin6 = (struct sockaddr_in6 *) c->local_sockaddr;
               port = ntohs(sin6->sin6_port);
               break;
   #endif
   #if (NGX_HAVE_UNIX_DOMAIN)
-        case AF_UNIX:
+          case AF_UNIX:
               port = 0;
               break;
   #endif
-        default: /* AF_INET */
+          default: /* AF_INET */
               sin = (struct sockaddr_in *) c->local_sockaddr;
               port = ntohs(sin->sin_port);
               break;
-        }
+          }

-        len += sizeof("Location: https://") - 1
-               + host.len
-               + r->headers_out.location->value.len + 2;
-
-        if (clcf->port_in_redirect) {
+          if (clcf->port_in_redirect) {

   #if (NGX_HTTP_SSL)
               if (c->ssl)
-                port = (port == 443) ? 0 : port;
+              port = (port == 443) ? 0 : port;
               else
   #endif
-                port = (port == 80) ? 0 : port;
+              port = (port == 80) ? 0 : port;

-        } else {
+          } else {
               port = 0;
+          }
+
+          if (port) {
+            len += sizeof(":65535") - 1;
+          }
           }
-
-        if (port) {
-            len += sizeof(":65535") - 1;
+        else {
+          port = 0;
           }

       } else {