From 806135f1c93c09bb513efc1341d084951b080278 Mon Sep 17 00:00:00 2001
From: hongzhidao <hongzhidao@gmail.com>
Date: Fri, 28 Aug 2020 00:53:36 -0400
Subject: Router: fixed "pass" to upstreams.

Messed up return values in nxt_upstream_find() caused error in applying any
configuration with a valid "pass" value in router configuration pointing to
upstream.  That wasn't the case in "listeners" objects, where the return value
wasn't checked.

Also, it caused segfault in cases where the "pass" option was configured with
variables and resulting value was pointing to a non-existent upstream.

Added missing return checks as well to catch possible memory allocation errors.

The bug was introduced in d32bc428f46b.

This closes #472 issue on GitHub.
---
 src/nxt_http_route.c |  6 +++-
 src/nxt_router.c     |  4 +++
 src/nxt_upstream.c   |  4 +--
 test/test_routing.py | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c
index 0b2103cd..36e003ae 100644
--- a/src/nxt_http_route.c
+++ b/src/nxt_http_route.c
@@ -1594,6 +1594,7 @@ nxt_http_action_t *
 nxt_http_action_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf,
     nxt_str_t *name)
 {
+    nxt_int_t          ret;
     nxt_http_action_t  *action;
 
     action = nxt_mp_alloc(tmcf->router_conf->mem_pool,
@@ -1605,7 +1606,10 @@ nxt_http_action_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf,
     action->name = *name;
     action->handler = NULL;
 
-    nxt_http_action_resolve(task, tmcf, action);
+    ret = nxt_http_action_resolve(task, tmcf, action);
+    if (nxt_slow_path(ret != NXT_OK)) {
+        return NULL;
+    }
 
     return action;
 }
diff --git a/src/nxt_router.c b/src/nxt_router.c
index ea14c6fb..b7408c3c 100644
--- a/src/nxt_router.c
+++ b/src/nxt_router.c
@@ -1725,6 +1725,10 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf,
                                                          tmcf->router_conf,
                                                          &lscf.application);
             }
+
+            if (nxt_slow_path(skcf->action == NULL)) {
+                goto fail;
+            }
         }
     }
 
diff --git a/src/nxt_upstream.c b/src/nxt_upstream.c
index c8ecbbe6..9f81b286 100644
--- a/src/nxt_upstream.c
+++ b/src/nxt_upstream.c
@@ -86,11 +86,11 @@ nxt_upstream_find(nxt_upstreams_t *upstreams, nxt_str_t *name,
             action->u.upstream_number = i;
             action->handler = nxt_upstream_handler;
 
-            return NXT_DECLINED;
+            return NXT_OK;
         }
     }
 
-    return NXT_OK;
+    return NXT_DECLINED;
 }
 
 
diff --git a/test/test_routing.py b/test/test_routing.py
index 4107f57e..734825ef 100644
--- a/test/test_routing.py
+++ b/test/test_routing.py
@@ -329,11 +329,90 @@ class TestRouting(TestApplicationProto):
             [{"match": {"method": "GET"}}], 'routes'
         ), 'route pass absent configure'
 
+    def test_routes_route_pass(self):
+        assert 'success' in self.conf(
+            {
+                "applications": {
+                    "app": {
+                        "type": "python",
+                        "processes": {"spare": 0},
+                        "path": "/app",
+                        "module": "wsgi",
+                    }
+                },
+                "upstreams": {
+                    "one": {
+                        "servers": {
+                            "127.0.0.1:7081": {},
+                            "127.0.0.1:7082": {},
+                        },
+                    },
+                    "two": {
+                        "servers": {
+                            "127.0.0.1:7081": {},
+                            "127.0.0.1:7082": {},
+                        },
+                    },
+                },
+            }
+        )
+
+        assert 'success' in self.conf(
+            [{"action": {"pass": "routes"}}], 'routes'
+        )
+        assert 'success' in self.conf(
+            [{"action": {"pass": "applications/app"}}], 'routes'
+        )
+        assert 'success' in self.conf(
+            [{"action": {"pass": "upstreams/one"}}], 'routes'
+        )
+
     def test_routes_route_pass_absent(self):
         assert 'error' in self.conf(
             [{"match": {"method": "GET"}, "action": {}}], 'routes'
         ), 'route pass absent configure'
 
+    def test_routes_route_pass_invalid(self):
+        assert 'success' in self.conf(
+            {
+                "applications": {
+                    "app": {
+                        "type": "python",
+                        "processes": {"spare": 0},
+                        "path": "/app",
+                        "module": "wsgi",
+                    }
+                },
+                "upstreams": {
+                    "one": {
+                        "servers": {
+                            "127.0.0.1:7081": {},
+                            "127.0.0.1:7082": {},
+                        },
+                    },
+                    "two": {
+                        "servers": {
+                            "127.0.0.1:7081": {},
+                            "127.0.0.1:7082": {},
+                        },
+                    },
+                },
+            }
+        )
+
+        assert 'error' in self.conf(
+            [{"action": {"pass": "blah"}}], 'routes'
+        ), 'route pass invalid'
+        assert 'error' in self.conf(
+            [{"action": {"pass": "routes/blah"}}], 'routes'
+        ), 'route pass routes invalid'
+        assert 'error' in self.conf(
+            [{"action": {"pass": "applications/blah"}}], 'routes'
+        ), 'route pass applications invalid'
+        assert 'error' in self.conf(
+            [{"action": {"pass": "upstreams/blah"}}], 'routes'
+        ), 'route pass upstreams invalid'
+
     def test_routes_action_unique(self):
         assert 'success' in self.conf(
             {
-- 
cgit