commit 926ab7ebba8345ef2c407e5f70304edd181daca6
Author: Sergey Poznyakoff <gray@gnu.org>
Date:   Wed Aug 21 11:42:08 2024 +0300
Forwarded: not-needed

    Revamp RewriteLocation support.
    
    * src/config.c (backend_parse_servername): Don't require HTTPS.
    ServerName can be used with plain HTTP backends as well.
    * src/svc.c (need_rewrite): Rewrite from scratch.
    * tests/poundharness.pl (http_redirect): Send original location in
    X-Orig-Location header.
    * tests/rewriteloc.at: Rewrite.
    * tests/rewriteloc_https.at: New test.
    * tests/testsuite.at: Include new test.
    * tests/Makefile.am: Add new test.
    * doc/pound.texi: Document changes.

diff --git a/doc/pound.texi b/doc/pound.texi
index 6592d70..101336c 100644
--- a/doc/pound.texi
+++ b/doc/pound.texi
@@ -326,12 +326,18 @@ statements.  This is the default.  To suppress deprecation
 messages, use @option{-W no-warn-deprecated}.
 @end defvr
 
+@anchor{dns}
 @defvr Feature dns
-Resolve host names found in configuration file.  This is the default.
+Resolve host names found in configuration file and returned in the
+@code{Location:} header.  This is the default.
+
 You can use @option{-W no-dns} to disable it, in order to
 suppress potentially lengthy network host address lookups.  Make sure
 if your configuration file refers to backends only by their IP
 addresses in this case.
+
+This setting affects also redirection location rewriting:
+@xref{Response Modification, RewriteLocation}.
 @end defvr
 
 @defvr Feature include-dir=@var{dir}
@@ -3211,6 +3217,17 @@ the request host name will be used instead.  This is the default.
 If @var{n} is 2, do the same, but compare only the backend address;
 this is useful for redirecting a request to an HTTPS listener on
 the same server as the HTTP listener.
+
+To check whether the location points to the listener or to the
+backend, its hostname part is resolved and the obtained IP address (or
+addresses) are compared with that of listener or backend.  This
+process is affected by the @option{dns} feature setting
+(@pxref{dns}).  If it is disabled (@option{-W no-dns} option is
+given), no resolving takes place.  In this case the location is deemed
+to point to the listener if its hostname part matches that of the
+incoming request.  For backends, the hostname is compared with the
+value of the @code{ServerName} setting of that backend
+(@pxref{ServerName}), if any.
 @end deffn
 
 @node rewrite response
@@ -3821,6 +3838,7 @@ following directives configure HTTPS backends:
 This directive indicates that the remote server speaks HTTPS.
 @end deffn
 
+@anchor{ServerName}
 @deffn {Backend directive} ServerName "@var{name}"
 This directive specifies the name to use for server name
 identification (@dfn{SNI}).  It also rewrites the @code{Host:} header
diff --git a/src/config.c b/src/config.c
index 641ba33..eece8a6 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2114,12 +2114,6 @@ backend_parse_servername (void *call_data, void *section_data)
   BACKEND *be = call_data;
   struct token *tok;
 
-  if (be->v.reg.ctx == NULL)
-    {
-      conf_error ("%s", "HTTPS must be used before this statement");
-      return PARSER_FAIL;
-    }
-
   if ((tok = gettkn_expect (T_STRING)) == NULL)
     return PARSER_FAIL;
   be->v.reg.servername = xstrdup (tok->str);
diff --git a/src/svc.c b/src/svc.c
index 776646c..5fe093a 100644
--- a/src/svc.c
+++ b/src/svc.c
@@ -981,30 +981,134 @@ get_host (char *const name, struct addrinfo *res, int ai_family)
   return ret_val;
 }
 
+/* Return 1 if the protocol designation PROTO of length LEN equals PAT. */
 static inline int
 is_proto (char const *pat, char const *proto, size_t len)
 {
   return len == strlen (pat) && strncasecmp (proto, pat, len) == 0;
 }
 
+union sockaddr_union
+{
+  struct sockaddr_in i;
+  struct sockaddr_in6 i6;
+  struct sockaddr_storage s;
+};
+
+/* Set port value in the sockaddr_union. */
+static void
+sun_set_port (union sockaddr_union *sun, int port)
+{
+  switch (sun->s.ss_family)
+    {
+    case AF_INET:
+      sun->i.sin_port = port;
+      break;
+
+    case AF_INET6:
+      sun->i6.sin6_port = port;
+      break;
+    }
+}
+
+/*
+ * Return 1 if any of the addresses from ADDR with port changed to PORT
+ * coincides with the address of the backend BE.
+ */
+static int
+is_backend_address (struct addrinfo const *addr, int port, const BACKEND *be)
+{
+  union sockaddr_union sun_be, sun;
+
+  memcpy (&sun_be, be->v.reg.addr.ai_addr, be->v.reg.addr.ai_addrlen);
+
+  for (; addr; addr = addr->ai_next)
+    {
+      if (addr->ai_family == sun_be.s.ss_family)
+	{
+	  memcpy (&sun, addr->ai_addr, addr->ai_addrlen);
+	  sun_set_port (&sun, port);
+	  if (memcmp (&sun_be, &sun, addr->ai_addrlen) == 0)
+	    return 1;
+	}
+    }
+  return 0;
+}
+
+/*
+ * Return 1 if any of the ADDR addresses matches the address of the given
+ * listener.  Ignore port number in comparisons.
+ */
+static int
+is_listener_address (struct addrinfo const *addr, const LISTENER *lstn)
+{
+  union sockaddr_union sun_lstn, sun;
+
+  if (!addr)
+    return 0;
+
+  memcpy (&sun_lstn, lstn->addr.ai_addr, lstn->addr.ai_addrlen);
+  sun_set_port (&sun_lstn, 0);
+
+  for (; addr; addr = addr->ai_next)
+    {
+      if (addr->ai_family != sun_lstn.s.ss_family)
+	{
+	  memcpy (&sun, addr->ai_addr, addr->ai_addrlen);
+	  sun_set_port (&sun, 0);
+	  if (memcmp (&sun_lstn, &sun, addr->ai_addrlen) == 0)
+	    return 1;
+	}
+    }
+  return 0;
+}
+
+/* Return port number of the listener LSTN. */
+static int
+listener_port (const LISTENER *lstn)
+{
+  union sockaddr_union *p = (union sockaddr_union *)&lstn->addr.ai_addr;
+  switch (p->s.ss_family)
+    {
+    case AF_INET:
+      return p->i.sin_port;
+
+    case AF_INET6:
+      return p->i6.sin6_port;
+    }
+  return 0;
+}
+
 /*
- * Find if a redirect needs rewriting
- * In general we have two possibilities that require it:
- * (1) if the redirect was done to the correct location with the wrong port
- * (2) if the redirect was done to the back-end rather than the listener
+ * Test if the redirection location value needs rewriting.  Return 1 if so.
+ * Depending on the value of the RewriteLocation setting (lstn->rewr_loc):
+ *
+ *   when 0       - no rewrite occurs; the function returns 0;
+ *   when 1       - rewrite is needed if the location points to the backend
+ *                  or to the listener with wrong port number of protocol;
+ *   when 2       - rewrite is needed if the location points to the backend.
+ *
+ * Input arguments:
+ *   location     -  value of the Location: header;
+ *   v_host       -  host part of the incoming request;
+ *   lstn         -  selected listener;
+ *   be           -  backend the response came from;
+ * Output argument:
+ *   ppath        -  return path part of the location.
  */
 int
 need_rewrite (const char *location, const char *v_host,
 	      const LISTENER *lstn, const BACKEND *be, const char **ppath)
 {
-  struct addrinfo addr;
-  struct sockaddr_in in_addr, be_addr;
-  struct sockaddr_in6 in6_addr, be6_addr;
   POUND_REGMATCH matches[4];
   char const *proto;
   char const *path;
-  char *host, *vhost, *port, *cp;
+  int port;
+  char *cp;
   size_t len;
+  char *host, *vhost;
+  int result = 0;
+  struct addrinfo hints, *addr = NULL;
 
   if (lstn->rewr_loc == 0)
     return 0;
@@ -1031,145 +1135,60 @@ need_rewrite (const char *location, const char *v_host,
   memcpy (host, location + matches[2].rm_so, len);
   host[len] = 0;
 
-  if ((port = strchr (host, ':')) != NULL)
-    *port++ = '\0';
-
-  /*
-   * Check if the location has the same address as the listener or the back-end
-   */
-  memset (&addr, 0, sizeof (addr));
-  if (get_host (host, &addr, be->v.reg.addr.ai_family))
-    return 0;
-
-  /*
-   * compare the back-end
-   */
-  if (addr.ai_family != be->v.reg.addr.ai_family)
-    {
-      free (addr.ai_addr);
-      free (host);
-      return 0;
-    }
-  if (addr.ai_family == AF_INET)
-    {
-      memcpy (&in_addr, addr.ai_addr, sizeof (in_addr));
-      memcpy (&be_addr, be->v.reg.addr.ai_addr, sizeof (be_addr));
-      if (port)
-	in_addr.sin_port = (in_port_t) htons (atoi (port));
-      else if (is_proto ("https", proto, matches[1].rm_eo - matches[1].rm_so))
-	in_addr.sin_port = (in_port_t) htons (443);
-      else
-	in_addr.sin_port = (in_port_t) htons (80);
-      /*
-       * check if the Location points to the back-end
-       */
-      if (memcmp (&be_addr.sin_addr.s_addr, &in_addr.sin_addr.s_addr,
-		  sizeof (in_addr.sin_addr.s_addr)) == 0
-	  && memcmp (&be_addr.sin_port, &in_addr.sin_port,
-		     sizeof (in_addr.sin_port)) == 0)
-	{
-	  free (addr.ai_addr);
-	  free (host);
-	  *ppath = path;
-	  return 1;
-	}
-    }
-  else				/* AF_INET6 */
-    {
-      memcpy (&in6_addr, addr.ai_addr, sizeof (in6_addr));
-      memcpy (&be6_addr, be->v.reg.addr.ai_addr, sizeof (be6_addr));
-      if (port)
-	in6_addr.sin6_port = (in_port_t) htons (atoi (port));
-      else if (is_proto ("https", proto, matches[1].rm_eo - matches[1].rm_so))
-	in6_addr.sin6_port = (in_port_t) htons (443);
-      else
-	in6_addr.sin6_port = (in_port_t) htons (80);
-      /*
-       * check if the Location points to the back-end
-       */
-      if (memcmp (&be6_addr.sin6_addr.s6_addr, &in6_addr.sin6_addr.s6_addr,
-		  sizeof (in6_addr.sin6_addr.s6_addr)) == 0
-	  && memcmp (&be6_addr.sin6_port, &in6_addr.sin6_port,
-		     sizeof (in6_addr.sin6_port)) == 0)
-	{
-	  free (addr.ai_addr);
-	  free (host);
-	  *ppath = path;
-	  return 1;
-	}
-    }
-
-  /*
-   * compare the listener
-   */
-  if (lstn->rewr_loc != 1 || addr.ai_family != lstn->addr.ai_family)
+  if ((cp = strchr (host, ':')) != NULL)
     {
-      free (addr.ai_addr);
-      free (host);
-      return 0;
+      *cp++ = '\0';
+      port = htons (atoi (cp));
     }
+  else if (is_proto ("https", proto, matches[1].rm_eo - matches[1].rm_so))
+    port = htons (443);
+  else
+    port = htons (80);
 
   if ((vhost = strdup (v_host)) == NULL)
     {
       lognomem ();
-      free (addr.ai_addr);
       free (host);
       return 0;
     }
 
   if ((cp = strchr (vhost, ':')) != NULL)
     *cp = '\0';
-  if (addr.ai_family == AF_INET)
-    {
-      memcpy (&be_addr, lstn->addr.ai_addr, sizeof (be_addr));
-      /*
-       * check if the Location points to the Listener but with the wrong
-       * port or protocol
-       */
-      if ((memcmp (&be_addr.sin_addr.s_addr, &in_addr.sin_addr.s_addr,
-		   sizeof (in_addr.sin_addr.s_addr)) == 0
-	   || strcasecmp (host, vhost) == 0)
-	  && (memcmp (&be_addr.sin_port, &in_addr.sin_port,
-		      sizeof (in_addr.sin_port)) != 0
-	      || is_proto (!SLIST_EMPTY (&lstn->ctx_head) ? "https" : "http",
-			   proto,
-			   matches[1].rm_eo - matches[1].rm_so)))
-	{
-	  free (addr.ai_addr);
-	  free (host);
-	  free (vhost);
-	  *ppath = path;
-	  return 1;
-	}
+
+  memset (&hints, 0, sizeof (hints));
+  hints.ai_family = AF_UNSPEC;
+  hints.ai_socktype = SOCK_STREAM;
+  hints.ai_flags = AI_CANONNAME |
+		    (feature_is_set (FEATURE_DNS) ? 0 : AI_NUMERICHOST);
+  if (getaddrinfo (host, NULL, &hints, &addr) == 0)
+    {
+      result = is_backend_address (addr, port, be);
     }
   else
+    result = (be->v.reg.servername &&
+	      strcasecmp (host, be->v.reg.servername) == 0 &&
+	      is_proto (be->v.reg.ctx ? "https" : "http",
+			proto,
+			matches[1].rm_eo - matches[1].rm_so));
+
+  if (result == 0 && lstn->rewr_loc == 1)
     {
-      memcpy (&be6_addr, lstn->addr.ai_addr, sizeof (be6_addr));
-      /*
-       * check if the Location points to the Listener but with the wrong
-       * port or protocol
-       */
-      if ((memcmp (&be6_addr.sin6_addr.s6_addr, &in6_addr.sin6_addr.s6_addr,
-		   sizeof (in6_addr.sin6_addr.s6_addr)) == 0
-	   || strcasecmp (host, vhost) == 0)
-	  && (memcmp (&be6_addr.sin6_port, &in6_addr.sin6_port,
-		      sizeof (in6_addr.sin6_port)) != 0
-	      || is_proto (!SLIST_EMPTY (&lstn->ctx_head) ? "https" : "http",
-			   proto,
-			   matches[1].rm_eo - matches[1].rm_so)))
-	{
-	  free (addr.ai_addr);
-	  free (host);
-	  free (vhost);
-	  *ppath = path;
-	  return 1;
-	}
+      result = ((is_listener_address (addr, lstn) ||
+		 strcasecmp (host, vhost) == 0) ||
+		((port != listener_port (lstn) ||
+		  !is_proto (SLIST_EMPTY (&lstn->ctx_head) ? "http" : "https",
+			     proto,
+			     matches[1].rm_eo - matches[1].rm_so))));
     }
 
-  free (addr.ai_addr);
-  free (host);
+  if (addr)
+    freeaddrinfo (addr);
   free (vhost);
-  return 0;
+  free (host);
+
+  if (result)
+    *ppath = path;
+  return result;
 }
 
 /*
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 35cac50..1eba08f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -114,6 +114,7 @@ TESTSUITE_AT = \
  redirect.at\
  resprw.at\
  rewriteloc.at\
+ rewriteloc_https.at\
  rwchain.at\
  sessauth.at\
  sessctl.at\
diff --git a/tests/poundharness.pl b/tests/poundharness.pl
index 750ca54..03885a5 100644
--- a/tests/poundharness.pl
+++ b/tests/poundharness.pl
@@ -1282,8 +1282,9 @@ sub http_redirect {
     my ($http, $rest) = @_;
     my $redir = $http->header('x-redirect')//'';
     $http->reply(301, "Moved Permanently", headers => {
-			'location' => $redir . '/echo' . $rest
-		 });
+	'location' => $redir . '/echo' . $rest,
+	'x-orig-location' => $redir . '/echo' . $rest
+    });
 }
 
 sub process_http_request {
diff --git a/tests/rewriteloc.at b/tests/rewriteloc.at
index 61b3ae8..de5bd30 100644
--- a/tests/rewriteloc.at
+++ b/tests/rewriteloc.at
@@ -16,8 +16,9 @@
 
 AT_SETUP([RewriteLocation])
 AT_KEYWORDS([rewriteloc])
+
 PT_CHECK([ListenHTTP
-	RewriteLocation 0
+	# RewriteLocation true is the default.
 	Service
 		Backend
 			Address
@@ -26,58 +27,43 @@ PT_CHECK([ListenHTTP
 	End
 End
 ],
-[GET /redirect/foo
-end
-
-301
-location: /echo/foo
-end
-
-
+[# Case 1:
+# Original location points to the listener with another port (without port
+# in this case, which means default port 80 is assumed).
+# The location is rewritten.
 GET /redirect/foo
-x-redirect: http://${BACKEND}
-end
-
-301
-location: http://${BACKEND}/echo/foo
-end
-
-GET /redirect/foo
-x-redirect: http://${LISTENER}
+x-redirect: http://${LISTENER:IP}
 end
 
 301
 location: http://${LISTENER}/echo/foo
 end
-])
 
-PT_CHECK([ListenHTTP
-	RewriteLocation 1
-	Service
-		Backend
-			Address
-			Port
-		End
-	End
-End
-],
-[GET /redirect/foo
+# Case 2:
+# Same as above but with another Host: header.
+GET /redirect/foo
+Host: example.com
+x-redirect: http://${LISTENER:IP}
 end
 
 301
-location: /echo/foo
+location: http://example.com:${LISTENER:PORT}/echo/foo
 end
 
+# Case 3
+# Redirect protocol differs.
+# Location is rewritten.
 GET /redirect/foo
-x-redirect: http://${BACKEND}
+x-redirect: https://${LISTENER:IP}
 end
 
 301
 location: http://${LISTENER}/echo/foo
 end
 
+# Case 4: Redirect points to the backend.
 GET /redirect/foo
-x-redirect: http://${LISTENER}
+x-redirect: http://${BACKEND}
 end
 
 301
@@ -86,6 +72,7 @@ end
 ])
 
 PT_CHECK([ListenHTTP
+	RewriteLocation 2
 	Service
 		Backend
 			Address
@@ -94,23 +81,36 @@ PT_CHECK([ListenHTTP
 	End
 End
 ],
-[GET /redirect/foo
+[# The first three cases are not rewritten: listener address is not compared.
+GET /redirect/foo
+x-redirect: http://${LISTENER:IP}
 end
 
 301
-location: /echo/foo
+location: http://${LISTENER:IP}/echo/foo
 end
 
 GET /redirect/foo
-x-redirect: http://${BACKEND}
+Host: example.com
+x-redirect: http://${LISTENER:IP}
 end
 
 301
-location: http://${LISTENER}/echo/foo
+location: http://${LISTENER:IP}/echo/foo
+end
+
+GET /redirect/foo
+x-redirect: https://${LISTENER:IP}
 end
 
+301
+location: https://${LISTENER:IP}/echo/foo
+end
+
+# Case 4: Redirect points to the backend.  It is handled the same as for
+# RewriteLocation 1.
 GET /redirect/foo
-x-redirect: http://${LISTENER}
+x-redirect: http://${BACKEND}
 end
 
 301
@@ -119,7 +119,7 @@ end
 ])
 
 PT_CHECK([ListenHTTP
-	RewriteLocation 2
+	RewriteLocation 0
 	Service
 		Backend
 			Address
@@ -128,27 +128,40 @@ PT_CHECK([ListenHTTP
 	End
 End
 ],
-[GET /redirect/foo
+[
+GET /redirect/foo
+x-redirect: http://${LISTENER:IP}
 end
 
 301
-location: /echo/foo
+location: http://${LISTENER:IP}/echo/foo
 end
 
 GET /redirect/foo
-x-redirect: http://${BACKEND}
+Host: example.com
+x-redirect: http://${LISTENER:IP}
 end
 
 301
-location: http://${LISTENER}/echo/foo
+location: http://${LISTENER:IP}/echo/foo
 end
 
 GET /redirect/foo
-x-redirect: http://${LISTENER}
+x-redirect: https://${LISTENER:IP}
 end
 
 301
-location: http://${LISTENER}/echo/foo
+location: https://${LISTENER:IP}/echo/foo
+end
+
+# Case 4: Redirect points to the backend.  It is handled the same as for
+# RewriteLocation 1.
+GET /redirect/foo
+x-redirect: http://${BACKEND}
+end
+
+301
+location: http://${BACKEND}/echo/foo
 end
 ])
 
diff --git a/tests/rewriteloc_https.at b/tests/rewriteloc_https.at
new file mode 100644
index 0000000..afb4806
--- /dev/null
+++ b/tests/rewriteloc_https.at
@@ -0,0 +1,35 @@
+AT_SETUP([RewriteLocation (https)])
+AT_KEYWORDS([rewriteloc https rewriteloc_https])
+
+AT_CHECK([openssl req -new -newkey rsa:2048 -days 1 -nodes -x509 \
+ -subj "/CN=www.example.com" -keyout key.pem -out crt.pem || exit 77
+cat crt.pem key.pem > example.pem
+],
+[0],
+[ignore],
+[ignore])
+
+PT_CHECK([ListenHTTPS
+	Cert "example.pem"
+	RewriteLocation 1
+	Service
+		Backend
+			Address
+			Port
+		End
+	End
+End
+],
+[GET /redirect/foo
+Host: example.com
+x-redirect: http://example.com
+end
+
+301
+location: https://example.com:${LISTENER:PORT}/echo/foo
+end
+])
+
+AT_CLEANUP
+
+
diff --git a/tests/testsuite.at b/tests/testsuite.at
index c0faf7c..7d78a3b 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -61,6 +61,7 @@ m4_include([errfile.at])
 m4_include([maxrequest.at])
 m4_include([maxuri.at])
 m4_include([rewriteloc.at])
+m4_include([rewriteloc_https.at])
 m4_include([nb.at])
 m4_include([chunked.at])
 m4_include([invenc.at])
