[PATCH] Fixed checking for no ports in the upstream block references

Valentin V. Bartenev vbart at wbsrv.ru
Sun Jun 16 11:28:09 UTC 2024


Improved commit message.
The previous one was a bit messy with missing fragments.


# HG changeset patch
# User Valentin Bartenev <vbart at wbsrv.ru>
# Date 1718536452 -10800
#      Sun Jun 16 14:14:12 2024 +0300
# Node ID 4fbc38ad3c8a8ed7986aaaa76b8aceda5ed70ef3
# Parent  02e9411009b987f408214ab4a8b6b6093f843bcd
Fixed checking for no ports in the upstream block references.

Configurations like this one:

   server {
       proxy_pass backend:80;
   }

   server {
       proxy_pass backend;
   }

   upstream backend {
       server 127.0.0.1:443;
   }

are rejected, since a "proxy_pass" reference of the upstream block cannot
have a port specified.  But if the "proxy_pass" directive with no port
appeared first then the check for no port didn't work properly and such
configurations were allowed:

   server {
       proxy_pass backend;
   }

   server {
       proxy_pass backend:80;
   }

   upstream backend {
       server 127.0.0.1:443;
   }

So, the fix is to continue lookup for all upstreams in the list checking
for duplicate references with port specified.

The issue was present in both "stream" and "http" modules.  And in case of
the "http" module, the fix is a bit more tricky there, since its "proxy_pass"
directive assumes port 80 (or 443 for https) by default.  For example, in the
configuration below:

   server {
       location / {
           proxy_pass http://backend;
       }
   }

   server {
       location / {
           proxy_pass http://backend:80;
       }
   }

   upstream backend {
       server 127.0.0.1:8000;
   }

the second "proxy_pass" is aggregated with the first one, but the "no_port"
flag was left intact.  In order to properly detect such cases, now the flag
is reset in the first case of reference with an explicit port set and also
the position of such reference is stored for a proper error handling.

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -6341,6 +6341,7 @@ ngx_http_upstream_add(ngx_conf_t *cf, ng
      umcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_upstream_module);
  
      uscfp = umcf->upstreams.elts;
+    uscf = NULL;
  
      for (i = 0; i < umcf->upstreams.nelts; i++) {
  
@@ -6383,11 +6384,23 @@ ngx_http_upstream_add(ngx_conf_t *cf, ng
          if (flags & NGX_HTTP_UPSTREAM_CREATE) {
              uscfp[i]->flags = flags;
              uscfp[i]->port = 0;
+            uscf = uscfp[i];
+            continue;
+        }
+
+        if (uscfp[i]->no_port && !u->no_port) {
+            uscfp[i]->no_port = 0;
+            uscfp[i]->file_name = cf->conf_file->file.name.data;
+            uscfp[i]->line = cf->conf_file->line;
          }
  
          return uscfp[i];
      }
  
+    if (uscf) {
+        return uscf;
+    }
+
      uscf = ngx_pcalloc(cf->pool, sizeof(ngx_http_upstream_srv_conf_t));
      if (uscf == NULL) {
          return NULL;
diff --git a/src/stream/ngx_stream_upstream.c b/src/stream/ngx_stream_upstream.c
--- a/src/stream/ngx_stream_upstream.c
+++ b/src/stream/ngx_stream_upstream.c
@@ -583,6 +583,7 @@ ngx_stream_upstream_add(ngx_conf_t *cf,
      umcf = ngx_stream_conf_get_module_main_conf(cf, ngx_stream_upstream_module);
  
      uscfp = umcf->upstreams.elts;
+    uscf = NULL;
  
      for (i = 0; i < umcf->upstreams.nelts; i++) {
  
@@ -622,11 +623,17 @@ ngx_stream_upstream_add(ngx_conf_t *cf,
  
          if (flags & NGX_STREAM_UPSTREAM_CREATE) {
              uscfp[i]->flags = flags;
+            uscf = uscfp[i];
+            continue;
          }
  
          return uscfp[i];
      }
  
+    if (uscf) {
+        return uscf;
+    }
+
      uscf = ngx_pcalloc(cf->pool, sizeof(ngx_stream_upstream_srv_conf_t));
      if (uscf == NULL) {
          return NULL;



More information about the nginx-devel mailing list