From a35059e3f128ed7e0de9c7c808d816693f3455f8 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Tue, 16 Apr 2019 21:31:32 +0900
Subject: [PATCH] nghttpx: Fix bug that altered authority and path affect
 backend selection

Fix bug that altered authority and path by per-pattern mruby script
affect backend selection on retry.
---
 src/shrpx_client_handler.cc | 28 +++++++++++++++++++++-------
 src/shrpx_downstream.h      | 13 ++++++++++++-
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc
index 7789bdb4..6266c5fa 100644
--- a/src/shrpx_client_handler.cc
+++ b/src/shrpx_client_handler.cc
@@ -893,7 +893,7 @@ ClientHandler::get_downstream_connection(int &err, Downstream *downstream) {
   auto catch_all = downstreamconf.addr_group_catch_all;
   auto &groups = worker_->get_downstream_addr_groups();
 
-  const auto &req = downstream->request();
+  auto &req = downstream->request();
 
   err = 0;
 
@@ -908,11 +908,14 @@ ClientHandler::get_downstream_connection(int &err, Downstream *downstream) {
 
   auto &balloc = downstream->get_block_allocator();
 
-  // Fast path.  If we have one group, it must be catch-all group.
-  if (groups.size() == 1) {
-    group_idx = 0;
+  StringRef authority, path;
+
+  if (req.forwarded_once) {
+    if (groups.size() != 1) {
+      authority = req.orig_authority;
+      path = req.orig_path;
+    }
   } else {
-    StringRef authority;
     if (faddr_->sni_fwd) {
       authority = sni_;
     } else if (!req.authority.empty()) {
@@ -924,13 +927,24 @@ ClientHandler::get_downstream_connection(int &err, Downstream *downstream) {
       }
     }
 
-    StringRef path;
     // CONNECT method does not have path.  But we requires path in
-    // host-path mapping.  As workaround, we assume that path is "/".
+    // host-path mapping.  As workaround, we assume that path is
+    // "/".
     if (!req.regular_connect_method()) {
       path = req.path;
     }
 
+    // Cache the authority and path used for the first-time backend
+    // selection because per-pattern mruby script can change them.
+    req.orig_authority = authority;
+    req.orig_path = path;
+    req.forwarded_once = true;
+  }
+
+  // Fast path.  If we have one group, it must be catch-all group.
+  if (groups.size() == 1) {
+    group_idx = 0;
+  } else {
     group_idx = match_downstream_addr_group(routerconf, authority, path, groups,
                                             catch_all, balloc);
   }
diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h
index 6684a8af..5b4bd207 100644
--- a/src/shrpx_downstream.h
+++ b/src/shrpx_downstream.h
@@ -153,7 +153,8 @@ struct Request {
         http2_upgrade_seen(false),
         connection_close(false),
         http2_expect_body(false),
-        no_authority(false) {}
+        no_authority(false),
+        forwarded_once(false) {}
 
   void consume(size_t len) {
     assert(unconsumed_body_length >= len);
@@ -184,6 +185,12 @@ struct Request {
   // request-target.  For HTTP/2, this is :path header field value.
   // For CONNECT request, this is empty.
   StringRef path;
+  // This is original authority which cannot be changed by per-pattern
+  // mruby script.
+  StringRef orig_authority;
+  // This is original path which cannot be changed by per-pattern
+  // mruby script.
+  StringRef orig_path;
   // the length of request body received so far
   int64_t recv_body_length;
   // The number of bytes not consumed by the application yet.
@@ -209,6 +216,10 @@ struct Request {
   // This happens when: For HTTP/2 request, :authority is missing.
   // For HTTP/1 request, origin or asterisk form is used.
   bool no_authority;
+  // true if backend selection is done for request once.
+  // orig_authority and orig_path have the authority and path which
+  // are used for the first backend selection.
+  bool forwarded_once;
 };
 
 struct Response {