From 6c42351a2223977ccf0d6e9277eea5fa7640e0a5 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Mon, 9 Nov 2020 17:06:39 -0800
Subject: [PATCH 01/14] Client cookie middleware

---
 .../arrow/flight/client_cookie_middleware.cc  | 72 +++++++++++++++++++
 .../arrow/flight/client_cookie_middleware.h   | 55 ++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100644 cpp/src/arrow/flight/client_cookie_middleware.cc
 create mode 100644 cpp/src/arrow/flight/client_cookie_middleware.h

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
new file mode 100644
index 00000000000..d2c3452e399
--- /dev/null
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -0,0 +1,72 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/client_cookie_middleware.h"
+
+// Platform-specific defines
+#include "arrow/flight/platform.h"
+
+namespace arrow {
+
+namespace flight {
+
+class ClientCookieMiddleware::Impl {
+ public:
+  void SendingHeaders(AddCallHeaders* outgoing_headers) {
+  }
+
+  void ReceivedHeaders(const CallHeaders& incoming_headers) {
+      // Parse the Set-Cookie header values here, and write to the collection managed in Factory::Impl
+  }
+
+  void CallCompleted(const Status& status) {
+  }
+};
+
+class ClientCookieMiddleware::Factory::Impl {
+ public:
+  void StartCall(const CallInfo& info,
+                            std::unique_ptr<ClientMiddleware::Impl>* middleware) {
+    // Instantiate middleware and have it store the 
+  }
+ private:
+      // Create a object class and store a collection of them here.
+      // Needs to be a concurrent data structure, or use locks to protected it.
+
+};
+
+void ClientCookieMiddleware::SendingHeaders(AddCallHeaders* outgoing_headers) {
+  impl_->SendingHeaders(outgoing_headers);
+}
+
+void ClientCookieMiddleware::ReceivedHeaders(const CallHeaders& incoming_headers) {
+  impl_->ReceivedHeaders(incoming_headers);
+}
+
+void ClientCookieMiddleware::CallCompleted(const Status& status) {
+  impl_->CallCompleted(status);
+
+ClientCookieMiddleware::ClientCookieMiddleware() {
+}
+
+ClientCookieMiddleware::Factory::Factory() {
+}
+
+Cli
+
+}  // namespace flight
+}  // namespace arrow
\ No newline at end of file
diff --git a/cpp/src/arrow/flight/client_cookie_middleware.h b/cpp/src/arrow/flight/client_cookie_middleware.h
new file mode 100644
index 00000000000..c8bb7ccf033
--- /dev/null
+++ b/cpp/src/arrow/flight/client_cookie_middleware.h
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Middleware implementation for sending and receiving HTTP cookies.
+
+#pragma once
+
+#include <memory>
+
+#include "arrow/flight/client_middleware.h"
+
+namespace arrow {
+namespace flight {
+
+/// \brief Client-side middleware for sending/receiving HTTP cookies.
+class ARROW_FLIGHT_EXPORT ClientCookieMiddleware : public ClientMiddleware {
+ public:
+  void SendingHeaders(AddCallHeaders* outgoing_headers) override;
+
+  void ReceivedHeaders(const CallHeaders& incoming_headers) override;
+
+  void CallCompleted(const Status& status) override;
+
+  class Factory : public ClientMiddlewareFactory {
+   public:
+     void StartCall(const CallInfo& info,
+                            std::unique_ptr<ClientMiddleware>* middleware) override;
+   private:
+     Factory();
+     class Impl;
+     std::unique_ptr<Factory::Impl> impl_;
+  };
+
+ private:
+  ClientCookieMiddleware();
+  class Impl;
+  std::unique_ptr<Impl> impl_;
+};
+
+}  // namespace flight
+}  // namespace arrow

From ee4658646dc5d334c475acf8caa3515a6389d58c Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Thu, 12 Nov 2020 16:05:51 -0800
Subject: [PATCH 02/14] Client cookie middleware with stubs for parsing cookies

---
 .../arrow/flight/client_cookie_middleware.cc  | 89 +++++++++++++++++--
 1 file changed, 81 insertions(+), 8 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index d2c3452e399..a820b471619 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -20,33 +20,105 @@
 // Platform-specific defines
 #include "arrow/flight/platform.h"
 
+namespace {
+  class Cookie {
+   public:
+    static Cookie parse(const util::string_view& cookie_value) {
+      // Parse the cookie string. If the cookie has an expiration, record it.
+      // If the cookie has a max-age, calculate the current time + max_age and set that as the expiration.
+      return Cookie();
+    }
+
+    bool IsExpired() {
+      // Check if current-time is less than creation time.
+      return false;
+    }
+
+    std::string AsCookieString() {
+      // Return the string for the cookie as it would appear in a Cookie header.
+      // Keys must be wrapped in quotes depending on if this is a v1 or v2 cookie.
+      return "";
+    }
+
+   private:
+    std::string cookie_name_;
+    std::string cookie_value_;
+    time_t expiration_time_;
+    bool is_v1_cookie_;
+  };
+} // end of anonymous namespace
+
 namespace arrow {
 
 namespace flight {
 
+using CookieCache = std::map<std::string, std::string>;
+
 class ClientCookieMiddleware::Impl {
  public:
   void SendingHeaders(AddCallHeaders* outgoing_headers) {
+    const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
+    if (!cookie_string.empty()) {
+      outgoing_headers->AddHeader("Cookie", cookie_string);
+    }
   }
 
   void ReceivedHeaders(const CallHeaders& incoming_headers) {
-      // Parse the Set-Cookie header values here, and write to the collection managed in Factory::Impl
+    const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& iter_pair =
+      incoming_headers.equal_range("Set-Cookie");
+    for (auto it = iter_pair.first; it != iter_pair.second; ++it) {
+      // Alternatively we can send every cookie to the factory in one call to reduce
+      // the amount of locking (by sending the interval of Set-Cookie values).
+      const util::string_view& value = (*iter_pair.first).second;
+      factory_impl_.UpdateCachedCookie(value);
+    }
+
+    factory_impl_.DiscardExpiredCookies();
   }
 
   void CallCompleted(const Status& status) {
   }
+ private:
+  Factory::Impl factory_impl_;
+  Impl(Factory::Impl& factory_impl) :
+   factory_impl_(factory_impl) {
+   }
 };
 
 class ClientCookieMiddleware::Factory::Impl {
  public:
   void StartCall(const CallInfo& info,
-                            std::unique_ptr<ClientMiddleware::Impl>* middleware) {
-    // Instantiate middleware and have it store the 
+                            std::unique_ptr<ClientMiddleware>* middleware) {
+    ARROW_UNUSED(info);
+    std::unique_ptr<ClientCookieMiddleware> cookie_middleware = std::make_shared<ClientCookieMiddleware>();
+    cookie_middleware->impl_ = std::make_shared(*this);
+    *middleware = std::move(cookie_middleware);
   }
  private:
-      // Create a object class and store a collection of them here.
-      // Needs to be a concurrent data structure, or use locks to protected it.
+  std::string GetValidCookiesAsString() const {
+    return "";
+  }
+
+  void UpdateCachedCookie(const util::string_view& cookie_value) {
+    std::lock_guard<std::mutex> guard(mutex_);
+
+    // Parse the Set-Cookie header values here
+  }
 
+  void DiscardExpiredCookies() {
+    std::lock_guard<std::mutex> guard(mutex_);
+
+    for (auto it = cookie_cache_.begin(); it != cookie_cache_.end(); ) {
+      if (it->second.IsExpired()) {
+        it = cookie_cache_.erase(it);
+      } else {
+        ++it;
+      }
+    }
+  }
+
+  CookieCache cookie_cache_;
+  std::mutex mutex_;
 };
 
 void ClientCookieMiddleware::SendingHeaders(AddCallHeaders* outgoing_headers) {
@@ -59,14 +131,15 @@ void ClientCookieMiddleware::ReceivedHeaders(const CallHeaders& incoming_headers
 
 void ClientCookieMiddleware::CallCompleted(const Status& status) {
   impl_->CallCompleted(status);
+}
 
 ClientCookieMiddleware::ClientCookieMiddleware() {
 }
 
-ClientCookieMiddleware::Factory::Factory() {
-}
+ClientCookieMiddleware::Factory::Factory() :
+ impl_(std::make_shared<>()) {
 
-Cli
+}
 
 }  // namespace flight
 }  // namespace arrow
\ No newline at end of file

From ff76621d3e59f0501e8aa05dd86d5841cd7d8154 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Fri, 13 Nov 2020 10:46:36 -0800
Subject: [PATCH 03/14] Don't expose ClientCookieMiddleware, just the factory.

---
 .../arrow/flight/client_cookie_middleware.cc  | 80 ++++++++-----------
 .../arrow/flight/client_cookie_middleware.h   | 24 ++----
 2 files changed, 37 insertions(+), 67 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index a820b471619..05ef7d18f29 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -46,46 +46,46 @@ namespace {
     time_t expiration_time_;
     bool is_v1_cookie_;
   };
-} // end of anonymous namespace
-
-namespace arrow {
 
-namespace flight {
+  class ClientCookieMiddleware : public ClientMiddleware {
+   public:
+    void SendingHeaders(AddCallHeaders* outgoing_headers) override {
+      const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
+      if (!cookie_string.empty()) {
+        outgoing_headers->AddHeader("Cookie", cookie_string);
+      }
+    }
 
-using CookieCache = std::map<std::string, std::string>;
+    void ReceivedHeaders(const CallHeaders& incoming_headers) override {
+      const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& iter_pair =
+        incoming_headers.equal_range("Set-Cookie");
+      for (auto it = iter_pair.first; it != iter_pair.second; ++it) {
+        // Alternatively we can send every cookie to the factory in one call to reduce
+        // the amount of locking (by sending the interval of Set-Cookie values).
+        const util::string_view& value = (*iter_pair.first).second;
+        factory_impl_.UpdateCachedCookie(value);
+      }
 
-class ClientCookieMiddleware::Impl {
- public:
-  void SendingHeaders(AddCallHeaders* outgoing_headers) {
-    const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
-    if (!cookie_string.empty()) {
-      outgoing_headers->AddHeader("Cookie", cookie_string);
+      factory_impl_.DiscardExpiredCookies();
     }
-  }
 
-  void ReceivedHeaders(const CallHeaders& incoming_headers) {
-    const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& iter_pair =
-      incoming_headers.equal_range("Set-Cookie");
-    for (auto it = iter_pair.first; it != iter_pair.second; ++it) {
-      // Alternatively we can send every cookie to the factory in one call to reduce
-      // the amount of locking (by sending the interval of Set-Cookie values).
-      const util::string_view& value = (*iter_pair.first).second;
-      factory_impl_.UpdateCachedCookie(value);
+    void CallCompleted(const Status& status) override {
+    }
+   private:
+    ClientCookieMiddlewareFactory::Impl& factory_impl_;
+    ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
+     factory_impl_(factory_impl) {
     }
+  };
+} // end of anonymous namespace
 
-    factory_impl_.DiscardExpiredCookies();
-  }
+namespace arrow {
 
-  void CallCompleted(const Status& status) {
-  }
- private:
-  Factory::Impl factory_impl_;
-  Impl(Factory::Impl& factory_impl) :
-   factory_impl_(factory_impl) {
-   }
-};
+namespace flight {
+
+using CookieCache = std::map<std::string, std::string>;
 
-class ClientCookieMiddleware::Factory::Impl {
+class ClientCookieMiddlewareFactory::Impl {
  public:
   void StartCall(const CallInfo& info,
                             std::unique_ptr<ClientMiddleware>* middleware) {
@@ -121,24 +121,8 @@ class ClientCookieMiddleware::Factory::Impl {
   std::mutex mutex_;
 };
 
-void ClientCookieMiddleware::SendingHeaders(AddCallHeaders* outgoing_headers) {
-  impl_->SendingHeaders(outgoing_headers);
-}
-
-void ClientCookieMiddleware::ReceivedHeaders(const CallHeaders& incoming_headers) {
-  impl_->ReceivedHeaders(incoming_headers);
-}
-
-void ClientCookieMiddleware::CallCompleted(const Status& status) {
-  impl_->CallCompleted(status);
-}
-
-ClientCookieMiddleware::ClientCookieMiddleware() {
-}
-
-ClientCookieMiddleware::Factory::Factory() :
+ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory() :
  impl_(std::make_shared<>()) {
-
 }
 
 }  // namespace flight
diff --git a/cpp/src/arrow/flight/client_cookie_middleware.h b/cpp/src/arrow/flight/client_cookie_middleware.h
index c8bb7ccf033..969d3b3c7e2 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.h
+++ b/cpp/src/arrow/flight/client_cookie_middleware.h
@@ -27,29 +27,15 @@ namespace arrow {
 namespace flight {
 
 /// \brief Client-side middleware for sending/receiving HTTP cookies.
-class ARROW_FLIGHT_EXPORT ClientCookieMiddleware : public ClientMiddleware {
+class ARROW_FLIGHT_EXPORT ClientCookieMiddlewareFactory : public ClientMiddlewareFactory {
  public:
-  void SendingHeaders(AddCallHeaders* outgoing_headers) override;
-
-  void ReceivedHeaders(const CallHeaders& incoming_headers) override;
-
-  void CallCompleted(const Status& status) override;
-
-  class Factory : public ClientMiddlewareFactory {
-   public:
-     void StartCall(const CallInfo& info,
-                            std::unique_ptr<ClientMiddleware>* middleware) override;
-   private:
-     Factory();
-     class Impl;
-     std::unique_ptr<Factory::Impl> impl_;
-  };
-
+  ClientCookieMiddlewareFactory();
+  void StartCall(const CallInfo& info,
+                        std::unique_ptr<ClientMiddleware>* middleware) override;
  private:
-  ClientCookieMiddleware();
   class Impl;
   std::unique_ptr<Impl> impl_;
 };
 
-}  // namespace flight
+} a // namespace flight
 }  // namespace arrow

From 1572dc41bfa13935ff96a8cb7a3ef8cb724e1dfa Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Fri, 13 Nov 2020 15:28:12 -0800
Subject: [PATCH 04/14] Compilation fixes

---
 .../arrow/flight/client_cookie_middleware.cc  | 90 +++++++++++--------
 .../arrow/flight/client_cookie_middleware.h   |  2 +-
 2 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 05ef7d18f29..0a9ae8e8c46 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -20,10 +20,15 @@
 // Platform-specific defines
 #include "arrow/flight/platform.h"
 
+#include "arrow/util/make_unique.h"
+
+#include <map>
+#include <mutex>
+#include <string>
+
 namespace {
-  class Cookie {
-   public:
-    static Cookie parse(const util::string_view& cookie_value) {
+  struct Cookie {
+    static Cookie parse(const arrow::util::string_view& cookie_value) {
       // Parse the cookie string. If the cookie has an expiration, record it.
       // If the cookie has a max-age, calculate the current time + max_age and set that as the expiration.
       return Cookie();
@@ -40,15 +45,34 @@ namespace {
       return "";
     }
 
-   private:
     std::string cookie_name_;
     std::string cookie_value_;
     time_t expiration_time_;
     bool is_v1_cookie_;
   };
+} // end of anonymous namespace
+
+namespace arrow {
+
+namespace flight {
+
+using CookieCache = std::map<std::string, Cookie>;
 
+class ClientCookieMiddlewareFactory::Impl {
+ public:
+  void StartCall(const CallInfo& info,
+                            std::unique_ptr<ClientMiddleware>* middleware) {
+    ARROW_UNUSED(info);
+    *middleware = internal::make_unique<ClientCookieMiddleware>(*this);
+  }
+ private:
   class ClientCookieMiddleware : public ClientMiddleware {
    public:
+    // This is public so that it can be used with make_unique.
+    ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
+     factory_impl_(factory_impl) {
+    }
+
     void SendingHeaders(AddCallHeaders* outgoing_headers) override {
       const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
       if (!cookie_string.empty()) {
@@ -57,56 +81,45 @@ namespace {
     }
 
     void ReceivedHeaders(const CallHeaders& incoming_headers) override {
-      const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& iter_pair =
+      const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& cookie_header_values =
         incoming_headers.equal_range("Set-Cookie");
-      for (auto it = iter_pair.first; it != iter_pair.second; ++it) {
-        // Alternatively we can send every cookie to the factory in one call to reduce
-        // the amount of locking (by sending the interval of Set-Cookie values).
-        const util::string_view& value = (*iter_pair.first).second;
-        factory_impl_.UpdateCachedCookie(value);
-      }
-
-      factory_impl_.DiscardExpiredCookies();
+      factory_impl_.UpdateCachedCookies(cookie_header_values);
     }
 
     void CallCompleted(const Status& status) override {
     }
    private:
     ClientCookieMiddlewareFactory::Impl& factory_impl_;
-    ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
-     factory_impl_(factory_impl) {
-    }
   };
-} // end of anonymous namespace
-
-namespace arrow {
-
-namespace flight {
 
-using CookieCache = std::map<std::string, std::string>;
+  // Retrieve the cached cookie values as a string.
+  //
+  // @return a string that can be used in a Cookie header representing the cookies that have been cached.
+  std::string GetValidCookiesAsString() {
+    const std::lock_guard<std::mutex> guard(mutex_);
 
-class ClientCookieMiddlewareFactory::Impl {
- public:
-  void StartCall(const CallInfo& info,
-                            std::unique_ptr<ClientMiddleware>* middleware) {
-    ARROW_UNUSED(info);
-    std::unique_ptr<ClientCookieMiddleware> cookie_middleware = std::make_shared<ClientCookieMiddleware>();
-    cookie_middleware->impl_ = std::make_shared(*this);
-    *middleware = std::move(cookie_middleware);
-  }
- private:
-  std::string GetValidCookiesAsString() const {
+    DiscardExpiredCookies();
     return "";
   }
 
-  void UpdateCachedCookie(const util::string_view& cookie_value) {
-    std::lock_guard<std::mutex> guard(mutex_);
+  // Updates the cache of cookies with new Set-Cookie header values.
+  //
+  // @param header_values The range representing header values.
+  void UpdateCachedCookies(const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& header_values) {
+    const std::lock_guard<std::mutex> guard(mutex_);
+
+    for (auto it = header_values.first; it != header_values.second; ++it) {
+      const util::string_view& value = header_values.first->second;
+      Cookie cookie = Cookie::parse(value);
+      cookie_cache_.insert({cookie.cookie_name_, cookie});
+    }
 
-    // Parse the Set-Cookie header values here
+    DiscardExpiredCookies();
   }
 
+  // Removes cookies that are marked as expired from the cache.
   void DiscardExpiredCookies() {
-    std::lock_guard<std::mutex> guard(mutex_);
+    const std::lock_guard<std::mutex> guard(mutex_);
 
     for (auto it = cookie_cache_.begin(); it != cookie_cache_.end(); ) {
       if (it->second.IsExpired()) {
@@ -117,12 +130,13 @@ class ClientCookieMiddlewareFactory::Impl {
     }
   }
 
+  // The cached cookies. Access should be protected by mutex_.
   CookieCache cookie_cache_;
   std::mutex mutex_;
 };
 
 ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory() :
- impl_(std::make_shared<>()) {
+ impl_(internal::make_unique<ClientCookieMiddlewareFactory::Impl>()) {
 }
 
 }  // namespace flight
diff --git a/cpp/src/arrow/flight/client_cookie_middleware.h b/cpp/src/arrow/flight/client_cookie_middleware.h
index 969d3b3c7e2..dd600390d7d 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.h
+++ b/cpp/src/arrow/flight/client_cookie_middleware.h
@@ -37,5 +37,5 @@ class ARROW_FLIGHT_EXPORT ClientCookieMiddlewareFactory : public ClientMiddlewar
   std::unique_ptr<Impl> impl_;
 };
 
-} a // namespace flight
+}  // namespace flight
 }  // namespace arrow

From 6198b6ab181e56a3afc60acc461b1b5e7231cf5a Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 03:01:04 -0800
Subject: [PATCH 05/14] Parsing cookie strings

---
 .../arrow/flight/client_cookie_middleware.cc  | 128 ++++++++++++++++--
 .../arrow/flight/client_cookie_middleware.h   |   3 +-
 cpp/src/arrow/util/uri.cc                     |  18 +--
 cpp/src/arrow/util/uri.h                      |   4 +
 4 files changed, 131 insertions(+), 22 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 0a9ae8e8c46..581c7c99e5e 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -21,34 +21,127 @@
 #include "arrow/flight/platform.h"
 
 #include "arrow/util/make_unique.h"
+#include "arrow/util/value_parsing.h"
+#include "arrow/util/string.h"
+#include "arrow/util/uri.h"
 
+#include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
 
 namespace {
+  #ifdef _WIN32
+  #define strcasecmp stricmp
+  #endif
+
+  struct CaseInsensitiveComparator : public std::binary_function<std::string, std::string, bool> {
+    bool operator()(const std::string &lhs, const std::string &rhs) const {
+      return strcasecmp(lhs.c_str(), rhs.c_str()) < 0;
+    }
+  };
+
+  // Parse a cookie header string beginning at the given start_pos and identify the name and value of an attribute.
+  //
+  // @param cookie_header_value The value of the Set-Cookie header.
+  // @param start_pos           An input/output parameter indicating the starting position of the attribute.
+  //                            It will store the position of the next attribute when the function returns.
+  // @param out_key             The name of the attribute.
+  // @param out_value           The value of the attribute.
+  //
+  // @return true if an attribute is found.
+  bool ParseCookieAttribute(std::string cookie_header_value, std::string::size_type& start_pos,
+    std::string& out_key, std::string& out_value) {
+      std::string::size_type equals_pos = cookie_header_value.find('=', start_pos);
+      if (std::string::npos == equals_pos) {
+        // No cookie attribute.
+        return false;
+      }
+
+      std::string::size_type semi_col_pos = cookie_header_value.find(';', equals_pos);
+      out_key = arrow::internal::TrimString(cookie_header_value.substr(start_pos, equals_pos));
+      if (std::string::npos == semi_col_pos && semi_col_pos > equals_pos) {
+        out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos+1));
+      } else {
+        out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos+1, semi_col_pos - equals_pos));
+      }
+
+      // Key/Value may be URI-encoded.
+      out_key = arrow::internal::UriUnescape(arrow::util::string_view(out_key));
+      out_value = arrow::internal::UriUnescape(arrow::util::string_view(out_value));
+
+      // Strip outer quotes on the value.
+      if (out_value.size() >= 2 && out_value[0] == '"' && out_value[out_value.size() - 1] == '"') {
+        out_value = out_value.substr(1, out_value.size() - 2);
+      }
+
+      // Update the start position for subsequent calls to this function.
+      start_pos = semi_col_pos + 1;
+      return true;
+  }
+
   struct Cookie {
-    static Cookie parse(const arrow::util::string_view& cookie_value) {
+    static Cookie parse(const arrow::util::string_view& cookie_header_value) {
       // Parse the cookie string. If the cookie has an expiration, record it.
       // If the cookie has a max-age, calculate the current time + max_age and set that as the expiration.
-      return Cookie();
+      Cookie cookie;
+      cookie.has_expiry_ = false;
+      std::string cookie_value_str(cookie_header_value);
+
+      // There should always be a first match which should be the name and value of the cookie.
+      std::string::size_type pos = 0;
+      if (!ParseCookieAttribute(cookie_value_str, pos, cookie.cookie_name_, cookie.cookie_value_)) {
+        // No cookie found. Mark the output cookie as expired.
+        cookie.has_expiry_ = true;
+        cookie.expiration_time_ = std::chrono::system_clock::now();
+      }
+
+      std::string cookie_attr_name;
+      std::string cookie_attr_value;
+      while (pos < cookie_value_str.size() &&
+          ParseCookieAttribute(cookie_value_str, pos, cookie_attr_name, cookie_attr_value)) {
+        if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name, "max-age")) {
+          // Note: max-age takes precedence over expires. We don't really care about other attributes and will
+          // arbitrarily take the first max-age. We can stop the loop here.
+          cookie.has_expiry_ = true;
+          int max_age = std::stoi(cookie_attr_value);
+          if (max_age <= 0) {
+            // Force expiration.
+            cookie.expiration_time_ = std::chrono::system_clock::now();
+          } else {
+            // Max-age is in seconds.
+            cookie.expiration_time_ = std::chrono::system_clock::now() + std::chrono::seconds(max_age);
+          }
+          break;
+        } else if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name, "expires")) {
+          cookie.has_expiry_ = true;
+          int64_t seconds = 0;
+          const char* COOKIE_EXPIRES_FORMAT = "%a, %d %b %Y %H:%M:%S GMT";
+          if (arrow::internal::ParseTimestampStrptime(cookie_attr_value.c_str(), cookie_attr_value.size(),
+              COOKIE_EXPIRES_FORMAT, false, true, arrow::TimeUnit::SECOND, &seconds)) {
+            cookie.expiration_time_ = std::chrono::system_clock::now() + std::chrono::seconds(seconds);
+          }
+        }
+      }
+
+      return cookie;
     }
 
-    bool IsExpired() {
+    bool IsExpired() const {
       // Check if current-time is less than creation time.
-      return false;
+      return has_expiry_ && expiration_time_ >= std::chrono::system_clock::now();
     }
 
     std::string AsCookieString() {
       // Return the string for the cookie as it would appear in a Cookie header.
       // Keys must be wrapped in quotes depending on if this is a v1 or v2 cookie.
-      return "";
+      return cookie_name_ + "=\"" + cookie_value_ + "\"";
     }
 
     std::string cookie_name_;
     std::string cookie_value_;
-    time_t expiration_time_;
-    bool is_v1_cookie_;
+    std::chrono::time_point<std::chrono::system_clock> expiration_time_;
+    bool has_expiry_;
   };
 } // end of anonymous namespace
 
@@ -56,7 +149,7 @@ namespace arrow {
 
 namespace flight {
 
-using CookieCache = std::map<std::string, Cookie>;
+using CookieCache = std::map<std::string, Cookie, CaseInsensitiveComparator>;
 
 class ClientCookieMiddlewareFactory::Impl {
  public:
@@ -99,7 +192,17 @@ class ClientCookieMiddlewareFactory::Impl {
     const std::lock_guard<std::mutex> guard(mutex_);
 
     DiscardExpiredCookies();
-    return "";
+    std::string cookie_string;
+    bool first = true;
+    for (auto it = cookie_cache_.begin(); cookie_cache_.end() != it; ++it) {
+      if (first) {
+        first = false;
+      } else {
+        cookie_string += "; ";
+      }
+      cookie_string += it->second.AsCookieString();
+    }
+    return cookie_string;
   }
 
   // Updates the cache of cookies with new Set-Cookie header values.
@@ -111,6 +214,9 @@ class ClientCookieMiddlewareFactory::Impl {
     for (auto it = header_values.first; it != header_values.second; ++it) {
       const util::string_view& value = header_values.first->second;
       Cookie cookie = Cookie::parse(value);
+
+      // Cache cookies regardless of whether or not they are expired. The server may have explicitly
+      // sent a Set-Cookie to expire a cached cookie.
       cookie_cache_.insert({cookie.cookie_name_, cookie});
     }
 
@@ -119,8 +225,6 @@ class ClientCookieMiddlewareFactory::Impl {
 
   // Removes cookies that are marked as expired from the cache.
   void DiscardExpiredCookies() {
-    const std::lock_guard<std::mutex> guard(mutex_);
-
     for (auto it = cookie_cache_.begin(); it != cookie_cache_.end(); ) {
       if (it->second.IsExpired()) {
         it = cookie_cache_.erase(it);
@@ -140,4 +244,4 @@ ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory() :
 }
 
 }  // namespace flight
-}  // namespace arrow
\ No newline at end of file
+}  // namespace arrow
diff --git a/cpp/src/arrow/flight/client_cookie_middleware.h b/cpp/src/arrow/flight/client_cookie_middleware.h
index dd600390d7d..608c35e11d9 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.h
+++ b/cpp/src/arrow/flight/client_cookie_middleware.h
@@ -31,7 +31,8 @@ class ARROW_FLIGHT_EXPORT ClientCookieMiddlewareFactory : public ClientMiddlewar
  public:
   ClientCookieMiddlewareFactory();
   void StartCall(const CallInfo& info,
-                        std::unique_ptr<ClientMiddleware>* middleware) override;
+                 std::unique_ptr<ClientMiddleware>* middleware) override;
+
  private:
   class Impl;
   std::unique_ptr<Impl> impl_;
diff --git a/cpp/src/arrow/util/uri.cc b/cpp/src/arrow/util/uri.cc
index 1261607b6c1..598f36be555 100644
--- a/cpp/src/arrow/util/uri.cc
+++ b/cpp/src/arrow/util/uri.cc
@@ -55,15 +55,6 @@ bool IsDriveSpec(const util::string_view s) {
 }
 #endif
 
-std::string UriUnescape(const util::string_view s) {
-  std::string result(s);
-  if (!result.empty()) {
-    auto end = uriUnescapeInPlaceA(&result[0]);
-    result.resize(end - &result[0]);
-  }
-  return result;
-}
-
 }  // namespace
 
 std::string UriEscape(const std::string& s) {
@@ -80,6 +71,15 @@ std::string UriEscape(const std::string& s) {
   return escaped;
 }
 
+std::string UriUnescape(const util::string_view s) {
+  std::string result(s);
+  if (!result.empty()) {
+    auto end = uriUnescapeInPlaceA(&result[0]);
+    result.resize(end - &result[0]);
+  }
+  return result;
+}
+
 struct Uri::Impl {
   Impl() : string_rep_(""), port_(-1) { memset(&uri_, 0, sizeof(uri_)); }
 
diff --git a/cpp/src/arrow/util/uri.h b/cpp/src/arrow/util/uri.h
index 6ef54900020..13377911261 100644
--- a/cpp/src/arrow/util/uri.h
+++ b/cpp/src/arrow/util/uri.h
@@ -24,6 +24,7 @@
 #include <vector>
 
 #include "arrow/type_fwd.h"
+#include "arrow/util/string_view.h"
 #include "arrow/util/visibility.h"
 
 namespace arrow {
@@ -91,5 +92,8 @@ class ARROW_EXPORT Uri {
 ARROW_EXPORT
 std::string UriEscape(const std::string& s);
 
+ARROW_EXPORT
+std::string UriUnescape(const arrow::util::string_view s);
+
 }  // namespace internal
 }  // namespace arrow

From c1d1308e0fb158b5da3a13a41d574fc09926cc33 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 12:30:33 -0800
Subject: [PATCH 06/14] Fix setting cookie expiration from expires attribute

ParseTimestampStrptime returns time units since the epoch, not
time units since now.
---
 cpp/src/arrow/flight/client_cookie_middleware.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 581c7c99e5e..81b7e3e110e 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -119,7 +119,8 @@ namespace {
           const char* COOKIE_EXPIRES_FORMAT = "%a, %d %b %Y %H:%M:%S GMT";
           if (arrow::internal::ParseTimestampStrptime(cookie_attr_value.c_str(), cookie_attr_value.size(),
               COOKIE_EXPIRES_FORMAT, false, true, arrow::TimeUnit::SECOND, &seconds)) {
-            cookie.expiration_time_ = std::chrono::system_clock::now() + std::chrono::seconds(seconds);
+
+            cookie.expiration_time_ = std::chrono::system_clock::from_time_t(static_cast<time_t>(seconds));
           }
         }
       }
@@ -162,7 +163,7 @@ class ClientCookieMiddlewareFactory::Impl {
   class ClientCookieMiddleware : public ClientMiddleware {
    public:
     // This is public so that it can be used with make_unique.
-    ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
+    explicit ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
      factory_impl_(factory_impl) {
     }
 

From fcf52449bfe9cf480e8aa6c70ef2adc49350b691 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 16:48:11 -0800
Subject: [PATCH 07/14] Missed Makefile update

---
 cpp/src/arrow/flight/CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt
index f0c23efb69f..7e1c01ea915 100644
--- a/cpp/src/arrow/flight/CMakeLists.txt
+++ b/cpp/src/arrow/flight/CMakeLists.txt
@@ -118,6 +118,7 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS_BACKUP}")
 # protobuf-internal.cc
 set(ARROW_FLIGHT_SRCS
     client.cc
+    client_cookie_middleware.cc
     internal.cc
     protocol_internal.cc
     serialization_internal.cc

From 3517c1cad3e7989626c69c9d00a03535f8499061 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 17:05:36 -0800
Subject: [PATCH 08/14] Fix clang-format warnings

---
 .../arrow/flight/client_cookie_middleware.cc  | 263 +++++++++---------
 1 file changed, 139 insertions(+), 124 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 81b7e3e110e..3939d1fd6a1 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -18,133 +18,148 @@
 #include "arrow/flight/client_cookie_middleware.h"
 
 // Platform-specific defines
-#include "arrow/flight/platform.h"
-
-#include "arrow/util/make_unique.h"
-#include "arrow/util/value_parsing.h"
-#include "arrow/util/string.h"
-#include "arrow/util/uri.h"
-
 #include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
 
+#include "arrow/flight/platform.h"
+#include "arrow/util/make_unique.h"
+#include "arrow/util/string.h"
+#include "arrow/util/uri.h"
+#include "arrow/util/value_parsing.h"
+
 namespace {
-  #ifdef _WIN32
-  #define strcasecmp stricmp
-  #endif
+#ifdef _WIN32
+#define strcasecmp stricmp
+#endif
+
+struct CaseInsensitiveComparator
+    : public std::binary_function<std::string, std::string, bool> {
+  bool operator()(const std::string& lhs, const std::string& rhs) const {
+    return strcasecmp(lhs.c_str(), rhs.c_str()) < 0;
+  }
+};
 
-  struct CaseInsensitiveComparator : public std::binary_function<std::string, std::string, bool> {
-    bool operator()(const std::string &lhs, const std::string &rhs) const {
-      return strcasecmp(lhs.c_str(), rhs.c_str()) < 0;
-    }
-  };
+// Parse a cookie header string beginning at the given start_pos and identify the name and
+// value of an attribute.
+//
+// @param cookie_header_value The value of the Set-Cookie header.
+// @param start_pos           An input/output parameter indicating the starting position
+// of the attribute.
+//                            It will store the position of the next attribute when the
+//                            function returns.
+// @param out_key             The name of the attribute.
+// @param out_value           The value of the attribute.
+//
+// @return true if an attribute is found.
+bool ParseCookieAttribute(std::string cookie_header_value,
+                          std::string::size_type& start_pos, std::string& out_key,
+                          std::string& out_value) {
+  std::string::size_type equals_pos = cookie_header_value.find('=', start_pos);
+  if (std::string::npos == equals_pos) {
+    // No cookie attribute.
+    return false;
+  }
 
-  // Parse a cookie header string beginning at the given start_pos and identify the name and value of an attribute.
-  //
-  // @param cookie_header_value The value of the Set-Cookie header.
-  // @param start_pos           An input/output parameter indicating the starting position of the attribute.
-  //                            It will store the position of the next attribute when the function returns.
-  // @param out_key             The name of the attribute.
-  // @param out_value           The value of the attribute.
-  //
-  // @return true if an attribute is found.
-  bool ParseCookieAttribute(std::string cookie_header_value, std::string::size_type& start_pos,
-    std::string& out_key, std::string& out_value) {
-      std::string::size_type equals_pos = cookie_header_value.find('=', start_pos);
-      if (std::string::npos == equals_pos) {
-        // No cookie attribute.
-        return false;
-      }
+  std::string::size_type semi_col_pos = cookie_header_value.find(';', equals_pos);
+  out_key =
+      arrow::internal::TrimString(cookie_header_value.substr(start_pos, equals_pos));
+  if (std::string::npos == semi_col_pos && semi_col_pos > equals_pos) {
+    out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos + 1));
+  } else {
+    out_value = arrow::internal::TrimString(
+        cookie_header_value.substr(equals_pos + 1, semi_col_pos - equals_pos));
+  }
 
-      std::string::size_type semi_col_pos = cookie_header_value.find(';', equals_pos);
-      out_key = arrow::internal::TrimString(cookie_header_value.substr(start_pos, equals_pos));
-      if (std::string::npos == semi_col_pos && semi_col_pos > equals_pos) {
-        out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos+1));
-      } else {
-        out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos+1, semi_col_pos - equals_pos));
-      }
+  // Key/Value may be URI-encoded.
+  out_key = arrow::internal::UriUnescape(arrow::util::string_view(out_key));
+  out_value = arrow::internal::UriUnescape(arrow::util::string_view(out_value));
 
-      // Key/Value may be URI-encoded.
-      out_key = arrow::internal::UriUnescape(arrow::util::string_view(out_key));
-      out_value = arrow::internal::UriUnescape(arrow::util::string_view(out_value));
+  // Strip outer quotes on the value.
+  if (out_value.size() >= 2 && out_value[0] == '"' &&
+      out_value[out_value.size() - 1] == '"') {
+    out_value = out_value.substr(1, out_value.size() - 2);
+  }
 
-      // Strip outer quotes on the value.
-      if (out_value.size() >= 2 && out_value[0] == '"' && out_value[out_value.size() - 1] == '"') {
-        out_value = out_value.substr(1, out_value.size() - 2);
-      }
+  // Update the start position for subsequent calls to this function.
+  start_pos = semi_col_pos + 1;
+  return true;
+}
 
-      // Update the start position for subsequent calls to this function.
-      start_pos = semi_col_pos + 1;
-      return true;
-  }
+struct Cookie {
+  static Cookie parse(const arrow::util::string_view& cookie_header_value) {
+    // Parse the cookie string. If the cookie has an expiration, record it.
+    // If the cookie has a max-age, calculate the current time + max_age and set that as
+    // the expiration.
+    Cookie cookie;
+    cookie.has_expiry_ = false;
+    std::string cookie_value_str(cookie_header_value);
+
+    // There should always be a first match which should be the name and value of the
+    // cookie.
+    std::string::size_type pos = 0;
+    if (!ParseCookieAttribute(cookie_value_str, pos, cookie.cookie_name_,
+                              cookie.cookie_value_)) {
+      // No cookie found. Mark the output cookie as expired.
+      cookie.has_expiry_ = true;
+      cookie.expiration_time_ = std::chrono::system_clock::now();
+    }
 
-  struct Cookie {
-    static Cookie parse(const arrow::util::string_view& cookie_header_value) {
-      // Parse the cookie string. If the cookie has an expiration, record it.
-      // If the cookie has a max-age, calculate the current time + max_age and set that as the expiration.
-      Cookie cookie;
-      cookie.has_expiry_ = false;
-      std::string cookie_value_str(cookie_header_value);
-
-      // There should always be a first match which should be the name and value of the cookie.
-      std::string::size_type pos = 0;
-      if (!ParseCookieAttribute(cookie_value_str, pos, cookie.cookie_name_, cookie.cookie_value_)) {
-        // No cookie found. Mark the output cookie as expired.
+    std::string cookie_attr_name;
+    std::string cookie_attr_value;
+    while (pos < cookie_value_str.size() &&
+           ParseCookieAttribute(cookie_value_str, pos, cookie_attr_name,
+                                cookie_attr_value)) {
+      if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name, "max-age")) {
+        // Note: max-age takes precedence over expires. We don't really care about other
+        // attributes and will arbitrarily take the first max-age. We can stop the loop
+        // here.
         cookie.has_expiry_ = true;
-        cookie.expiration_time_ = std::chrono::system_clock::now();
-      }
-
-      std::string cookie_attr_name;
-      std::string cookie_attr_value;
-      while (pos < cookie_value_str.size() &&
-          ParseCookieAttribute(cookie_value_str, pos, cookie_attr_name, cookie_attr_value)) {
-        if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name, "max-age")) {
-          // Note: max-age takes precedence over expires. We don't really care about other attributes and will
-          // arbitrarily take the first max-age. We can stop the loop here.
-          cookie.has_expiry_ = true;
-          int max_age = std::stoi(cookie_attr_value);
-          if (max_age <= 0) {
-            // Force expiration.
-            cookie.expiration_time_ = std::chrono::system_clock::now();
-          } else {
-            // Max-age is in seconds.
-            cookie.expiration_time_ = std::chrono::system_clock::now() + std::chrono::seconds(max_age);
-          }
-          break;
-        } else if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name, "expires")) {
-          cookie.has_expiry_ = true;
-          int64_t seconds = 0;
-          const char* COOKIE_EXPIRES_FORMAT = "%a, %d %b %Y %H:%M:%S GMT";
-          if (arrow::internal::ParseTimestampStrptime(cookie_attr_value.c_str(), cookie_attr_value.size(),
-              COOKIE_EXPIRES_FORMAT, false, true, arrow::TimeUnit::SECOND, &seconds)) {
-
-            cookie.expiration_time_ = std::chrono::system_clock::from_time_t(static_cast<time_t>(seconds));
-          }
+        int max_age = std::stoi(cookie_attr_value);
+        if (max_age <= 0) {
+          // Force expiration.
+          cookie.expiration_time_ = std::chrono::system_clock::now();
+        } else {
+          // Max-age is in seconds.
+          cookie.expiration_time_ =
+              std::chrono::system_clock::now() + std::chrono::seconds(max_age);
+        }
+        break;
+      } else if (arrow::internal::AsciiEqualsCaseInsensitive(cookie_attr_name,
+                                                             "expires")) {
+        cookie.has_expiry_ = true;
+        int64_t seconds = 0;
+        const char* COOKIE_EXPIRES_FORMAT = "%a, %d %b %Y %H:%M:%S GMT";
+        if (arrow::internal::ParseTimestampStrptime(
+                cookie_attr_value.c_str(), cookie_attr_value.size(),
+                COOKIE_EXPIRES_FORMAT, false, true, arrow::TimeUnit::SECOND, &seconds)) {
+          cookie.expiration_time_ =
+              std::chrono::system_clock::from_time_t(static_cast<time_t>(seconds));
         }
       }
-
-      return cookie;
     }
 
-    bool IsExpired() const {
-      // Check if current-time is less than creation time.
-      return has_expiry_ && expiration_time_ >= std::chrono::system_clock::now();
-    }
+    return cookie;
+  }
 
-    std::string AsCookieString() {
-      // Return the string for the cookie as it would appear in a Cookie header.
-      // Keys must be wrapped in quotes depending on if this is a v1 or v2 cookie.
-      return cookie_name_ + "=\"" + cookie_value_ + "\"";
-    }
+  bool IsExpired() const {
+    // Check if current-time is less than creation time.
+    return has_expiry_ && expiration_time_ >= std::chrono::system_clock::now();
+  }
 
-    std::string cookie_name_;
-    std::string cookie_value_;
-    std::chrono::time_point<std::chrono::system_clock> expiration_time_;
-    bool has_expiry_;
-  };
-} // end of anonymous namespace
+  std::string AsCookieString() {
+    // Return the string for the cookie as it would appear in a Cookie header.
+    // Keys must be wrapped in quotes depending on if this is a v1 or v2 cookie.
+    return cookie_name_ + "=\"" + cookie_value_ + "\"";
+  }
+
+  std::string cookie_name_;
+  std::string cookie_value_;
+  std::chrono::time_point<std::chrono::system_clock> expiration_time_;
+  bool has_expiry_;
+};
+}  // end of anonymous namespace
 
 namespace arrow {
 
@@ -154,18 +169,17 @@ using CookieCache = std::map<std::string, Cookie, CaseInsensitiveComparator>;
 
 class ClientCookieMiddlewareFactory::Impl {
  public:
-  void StartCall(const CallInfo& info,
-                            std::unique_ptr<ClientMiddleware>* middleware) {
+  void StartCall(const CallInfo& info, std::unique_ptr<ClientMiddleware>* middleware) {
     ARROW_UNUSED(info);
     *middleware = internal::make_unique<ClientCookieMiddleware>(*this);
   }
+
  private:
   class ClientCookieMiddleware : public ClientMiddleware {
    public:
     // This is public so that it can be used with make_unique.
-    explicit ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl) :
-     factory_impl_(factory_impl) {
-    }
+    explicit ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl)
+        : factory_impl_(factory_impl) {}
 
     void SendingHeaders(AddCallHeaders* outgoing_headers) override {
       const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
@@ -175,20 +189,21 @@ class ClientCookieMiddlewareFactory::Impl {
     }
 
     void ReceivedHeaders(const CallHeaders& incoming_headers) override {
-      const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& cookie_header_values =
-        incoming_headers.equal_range("Set-Cookie");
+      const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>&
+          cookie_header_values = incoming_headers.equal_range("Set-Cookie");
       factory_impl_.UpdateCachedCookies(cookie_header_values);
     }
 
-    void CallCompleted(const Status& status) override {
-    }
+    void CallCompleted(const Status& status) override {}
+
    private:
     ClientCookieMiddlewareFactory::Impl& factory_impl_;
   };
 
   // Retrieve the cached cookie values as a string.
   //
-  // @return a string that can be used in a Cookie header representing the cookies that have been cached.
+  // @return a string that can be used in a Cookie header representing the cookies that
+  // have been cached.
   std::string GetValidCookiesAsString() {
     const std::lock_guard<std::mutex> guard(mutex_);
 
@@ -209,15 +224,16 @@ class ClientCookieMiddlewareFactory::Impl {
   // Updates the cache of cookies with new Set-Cookie header values.
   //
   // @param header_values The range representing header values.
-  void UpdateCachedCookies(const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>& header_values) {
+  void UpdateCachedCookies(const std::pair<CallHeaders::const_iterator,
+                                           CallHeaders::const_iterator>& header_values) {
     const std::lock_guard<std::mutex> guard(mutex_);
 
     for (auto it = header_values.first; it != header_values.second; ++it) {
       const util::string_view& value = header_values.first->second;
       Cookie cookie = Cookie::parse(value);
 
-      // Cache cookies regardless of whether or not they are expired. The server may have explicitly
-      // sent a Set-Cookie to expire a cached cookie.
+      // Cache cookies regardless of whether or not they are expired. The server may have
+      // explicitly sent a Set-Cookie to expire a cached cookie.
       cookie_cache_.insert({cookie.cookie_name_, cookie});
     }
 
@@ -226,7 +242,7 @@ class ClientCookieMiddlewareFactory::Impl {
 
   // Removes cookies that are marked as expired from the cache.
   void DiscardExpiredCookies() {
-    for (auto it = cookie_cache_.begin(); it != cookie_cache_.end(); ) {
+    for (auto it = cookie_cache_.begin(); it != cookie_cache_.end();) {
       if (it->second.IsExpired()) {
         it = cookie_cache_.erase(it);
       } else {
@@ -240,9 +256,8 @@ class ClientCookieMiddlewareFactory::Impl {
   std::mutex mutex_;
 };
 
-ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory() :
- impl_(internal::make_unique<ClientCookieMiddlewareFactory::Impl>()) {
-}
+ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory()
+    : impl_(internal::make_unique<ClientCookieMiddlewareFactory::Impl>()) {}
 
 }  // namespace flight
 }  // namespace arrow

From 3e715f7b20d0e9ef23a1fbfc93fd27e4d30ed360 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 17:30:24 -0800
Subject: [PATCH 09/14] Fix mis-use of make_unique

---
 cpp/src/arrow/flight/client_cookie_middleware.cc | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 3939d1fd6a1..df3779914a6 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -24,7 +24,6 @@
 #include <string>
 
 #include "arrow/flight/platform.h"
-#include "arrow/util/make_unique.h"
 #include "arrow/util/string.h"
 #include "arrow/util/uri.h"
 #include "arrow/util/value_parsing.h"
@@ -171,13 +170,12 @@ class ClientCookieMiddlewareFactory::Impl {
  public:
   void StartCall(const CallInfo& info, std::unique_ptr<ClientMiddleware>* middleware) {
     ARROW_UNUSED(info);
-    *middleware = internal::make_unique<ClientCookieMiddleware>(*this);
+    *middleware = std::unique_ptr<ClientMiddleware>(new ClientCookieMiddleware(*this));
   }
 
  private:
   class ClientCookieMiddleware : public ClientMiddleware {
    public:
-    // This is public so that it can be used with make_unique.
     explicit ClientCookieMiddleware(ClientCookieMiddlewareFactory::Impl& factory_impl)
         : factory_impl_(factory_impl) {}
 
@@ -257,7 +255,7 @@ class ClientCookieMiddlewareFactory::Impl {
 };
 
 ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory()
-    : impl_(internal::make_unique<ClientCookieMiddlewareFactory::Impl>()) {}
+    : impl_(new ClientCookieMiddlewareFactory::Impl()) {}
 
 }  // namespace flight
 }  // namespace arrow

From 100c91f6d8d279a5952917a1ae9191235c236600 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sat, 14 Nov 2020 17:48:20 -0800
Subject: [PATCH 10/14] Fix missing function definitions

---
 cpp/src/arrow/flight/client_cookie_middleware.cc | 8 ++++++++
 cpp/src/arrow/flight/client_cookie_middleware.h  | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index df3779914a6..6269404a560 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -257,5 +257,13 @@ class ClientCookieMiddlewareFactory::Impl {
 ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory()
     : impl_(new ClientCookieMiddlewareFactory::Impl()) {}
 
+ClientCookieMiddlewareFactory::~ClientCookieMiddlewareFactory() {
+}
+
+void ClientCookieMiddlewareFactory::StartCall(const CallInfo& info,
+    std::unique_ptr<ClientMiddleware>* middleware) {
+  impl_->StartCall(info, middleware);
+}
+
 }  // namespace flight
 }  // namespace arrow
diff --git a/cpp/src/arrow/flight/client_cookie_middleware.h b/cpp/src/arrow/flight/client_cookie_middleware.h
index 608c35e11d9..5e3386bedbc 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.h
+++ b/cpp/src/arrow/flight/client_cookie_middleware.h
@@ -30,6 +30,9 @@ namespace flight {
 class ARROW_FLIGHT_EXPORT ClientCookieMiddlewareFactory : public ClientMiddlewareFactory {
  public:
   ClientCookieMiddlewareFactory();
+
+  ~ClientCookieMiddlewareFactory();
+
   void StartCall(const CallInfo& info,
                  std::unique_ptr<ClientMiddleware>* middleware) override;
 

From a1523a017a2ef36ff4e2297b7ecc1b501aeccd57 Mon Sep 17 00:00:00 2001
From: James Duong <jduong@dremio.com>
Date: Sun, 15 Nov 2020 09:41:07 -0800
Subject: [PATCH 11/14] More clang-format issues

---
 cpp/src/arrow/flight/client_cookie_middleware.cc | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 6269404a560..61c977000cc 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -257,11 +257,10 @@ class ClientCookieMiddlewareFactory::Impl {
 ClientCookieMiddlewareFactory::ClientCookieMiddlewareFactory()
     : impl_(new ClientCookieMiddlewareFactory::Impl()) {}
 
-ClientCookieMiddlewareFactory::~ClientCookieMiddlewareFactory() {
-}
+ClientCookieMiddlewareFactory::~ClientCookieMiddlewareFactory() {}
 
-void ClientCookieMiddlewareFactory::StartCall(const CallInfo& info,
-    std::unique_ptr<ClientMiddleware>* middleware) {
+void ClientCookieMiddlewareFactory::StartCall(
+    const CallInfo& info, std::unique_ptr<ClientMiddleware>* middleware) {
   impl_->StartCall(info, middleware);
 }
 

From 1a792ea48dd9f8846aa2225180a1e8cd43beb953 Mon Sep 17 00:00:00 2001
From: Lyndon Bauto <lyndonb@bitquilltech.com>
Date: Thu, 19 Nov 2020 19:15:33 -0800
Subject: [PATCH 12/14] [1] Added testing for cookies [2] Fixed a few bugs with
 parsing and logic of cookie handling

---
 .../arrow/flight/client_cookie_middleware.cc  |  40 ++--
 cpp/src/arrow/flight/flight_test.cc           | 219 ++++++++++++++++++
 2 files changed, 242 insertions(+), 17 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 61c977000cc..3cea4b3eba3 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -63,12 +63,16 @@ bool ParseCookieAttribute(std::string cookie_header_value,
 
   std::string::size_type semi_col_pos = cookie_header_value.find(';', equals_pos);
   out_key =
-      arrow::internal::TrimString(cookie_header_value.substr(start_pos, equals_pos));
+      arrow::internal::TrimString(
+          cookie_header_value.substr(start_pos, equals_pos - start_pos));
   if (std::string::npos == semi_col_pos && semi_col_pos > equals_pos) {
+    // Last item - set start pos to end
     out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos + 1));
+    start_pos = std::string::npos;
   } else {
     out_value = arrow::internal::TrimString(
-        cookie_header_value.substr(equals_pos + 1, semi_col_pos - equals_pos));
+        cookie_header_value.substr(equals_pos + 1, semi_col_pos - equals_pos - 1));
+    start_pos = semi_col_pos + 1;
   }
 
   // Key/Value may be URI-encoded.
@@ -82,7 +86,6 @@ bool ParseCookieAttribute(std::string cookie_header_value,
   }
 
   // Update the start position for subsequent calls to this function.
-  start_pos = semi_col_pos + 1;
   return true;
 }
 
@@ -144,7 +147,7 @@ struct Cookie {
 
   bool IsExpired() const {
     // Check if current-time is less than creation time.
-    return has_expiry_ && expiration_time_ >= std::chrono::system_clock::now();
+    return has_expiry_ && expiration_time_ <= std::chrono::system_clock::now();
   }
 
   std::string AsCookieString() {
@@ -182,13 +185,13 @@ class ClientCookieMiddlewareFactory::Impl {
     void SendingHeaders(AddCallHeaders* outgoing_headers) override {
       const std::string& cookie_string = factory_impl_.GetValidCookiesAsString();
       if (!cookie_string.empty()) {
-        outgoing_headers->AddHeader("Cookie", cookie_string);
+        outgoing_headers->AddHeader("cookie", cookie_string);
       }
     }
 
     void ReceivedHeaders(const CallHeaders& incoming_headers) override {
       const std::pair<CallHeaders::const_iterator, CallHeaders::const_iterator>&
-          cookie_header_values = incoming_headers.equal_range("Set-Cookie");
+          cookie_header_values = incoming_headers.equal_range("set-cookie");
       factory_impl_.UpdateCachedCookies(cookie_header_values);
     }
 
@@ -206,15 +209,13 @@ class ClientCookieMiddlewareFactory::Impl {
     const std::lock_guard<std::mutex> guard(mutex_);
 
     DiscardExpiredCookies();
-    std::string cookie_string;
-    bool first = true;
-    for (auto it = cookie_cache_.begin(); cookie_cache_.end() != it; ++it) {
-      if (first) {
-        first = false;
-      } else {
-        cookie_string += "; ";
-      }
-      cookie_string += it->second.AsCookieString();
+    if (cookie_cache_.empty()) {
+      return "";
+    }
+
+    std::string cookie_string = cookie_cache_.begin()->second.AsCookieString();
+    for (auto it = (++cookie_cache_.begin()); cookie_cache_.end() != it; ++it) {
+      cookie_string += "; " + it->second.AsCookieString();
     }
     return cookie_string;
   }
@@ -227,12 +228,17 @@ class ClientCookieMiddlewareFactory::Impl {
     const std::lock_guard<std::mutex> guard(mutex_);
 
     for (auto it = header_values.first; it != header_values.second; ++it) {
-      const util::string_view& value = header_values.first->second;
+      const util::string_view& value = it->second;
       Cookie cookie = Cookie::parse(value);
 
       // Cache cookies regardless of whether or not they are expired. The server may have
       // explicitly sent a Set-Cookie to expire a cached cookie.
-      cookie_cache_.insert({cookie.cookie_name_, cookie});
+      std::pair<CookieCache::iterator, bool> insertable = cookie_cache_.insert({cookie.cookie_name_, cookie});
+
+      // Force overwrite on insert collision.
+      if (!insertable.second) {
+        insertable.first->second = cookie;
+      }
     }
 
     DiscardExpiredCookies();
diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc
index a5fdfe8184e..c1c956f7372 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -46,6 +46,7 @@
 #include "arrow/flight/internal.h"
 #include "arrow/flight/middleware_internal.h"
 #include "arrow/flight/test_util.h"
+#include "arrow/flight/client_cookie_middleware.h"
 
 namespace pb = arrow::flight::protocol;
 
@@ -993,6 +994,178 @@ class TestErrorMiddleware : public ::testing::Test {
   std::unique_ptr<FlightServerBase> server_;
 };
 
+// This test has functions that allow adding and removing cookies from a list of cookies.
+// It also automatically adds cookies to an internal list of tracked cookies when they
+// are passed to the middleware. 
+class TestCookieMiddleware : public ::testing::Test {
+ public:
+  // Setup function creates middleware factory and starts it up.
+  void SetUp() {
+    factory_ = std::make_shared<ClientCookieMiddlewareFactory>();
+    CallInfo callInfo;
+    factory_->StartCall(callInfo, &middleware_);
+  }
+
+  // Function to add cookie to middleware.
+  void AddCookie(const std::string& cookie) {
+    CallHeaders call_headers;
+    call_headers.insert(
+      std::make_pair(arrow::util::string_view("set-cookie"), 
+      arrow::util::string_view(cookie)));
+    middleware_->ReceivedHeaders(call_headers);
+  }
+
+  // Function to get cookies from middleware.
+  std::string GetCookies() {
+    TestCallHeaders add_call_headers;
+    middleware_->SendingHeaders(&add_call_headers);
+    return add_call_headers.GetCookies();
+  }
+
+  // Function to trim whitespace from left and right sides of string.
+  std::string trim(std::string s) {
+    auto ltrim = [](std::string& s) {
+      s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) {
+          return !std::isspace(ch);
+      }));
+    };
+    auto rtrim = [](std::string& s) {
+      s.erase(std::find_if(s.rbegin(), s.rend(), [](unsigned char ch) {
+          return !std::isspace(ch);
+      }).base(), s.end());
+    };
+    ltrim(s);
+    rtrim(s);
+    return s;
+  };
+
+  // Function to take a list of cookies and split them into a vector of individual 
+  // cookies.
+  std::vector<std::string> SplitCookies(const std::string& cookies) {
+    std::vector<std::string> split_cookies;
+    std::string::size_type pos1 = 0;
+    std::string::size_type pos2 = 0;
+    while ((pos2 = cookies.find(';', pos1)) != std::string::npos) {
+      split_cookies.push_back(trim(cookies.substr(pos1, pos2 - pos1)));
+      pos1 = pos2 + 1;
+    }
+    if (pos1 < cookies.size()) {
+      split_cookies.push_back(trim(cookies.substr(pos1)));
+    }
+    std::sort(split_cookies.begin(), split_cookies.end());
+    return split_cookies;
+  }
+
+  // Function to check if expected cookies and actual cookies are equal.
+  void CheckEqual() {
+    std::sort(expected_cookies.begin(), expected_cookies.end());
+    std::vector<std::string> split_actual_cookies = SplitCookies(GetCookies());
+    EXPECT_EQ(expected_cookies, split_actual_cookies);
+  }
+
+  // Function to add incoming cookies to middleware and validate them.
+  void AddAndValidate(const std::string& incoming_cookie) {
+    AddCookie(incoming_cookie);
+    AddExpected(incoming_cookie);
+    CheckEqual();
+  }
+
+  // Function to add an expired cookie to middleware and validate that it was removed.
+  void AddExpiredAndValidate(const std::string& incoming_cookie) {
+    AddCookie(incoming_cookie);
+    CheckEqual();
+  }
+
+  // Function to convert format so that it is key="val" and not key=val.
+  void ConvertFormat(const std::string& incoming_cookie, std::string& outgoing_cookie) {
+    // Grab the first key value pair from the incoming cookie string.
+    std::string::size_type pos = 0;
+    const std::string key_val_pair =
+        trim(((pos = incoming_cookie.find(';')) != std::string::npos) ? 
+            incoming_cookie.substr(0, pos) : incoming_cookie.substr(0));
+
+    // Split the key value pair into the key and value.
+    pos = key_val_pair.find('=');
+    ASSERT_NE(std::string::npos, pos);
+    std::string key = key_val_pair.substr(0, pos);
+    std::string val = key_val_pair.substr(pos + 1);
+
+    // Add key/value pair to outgoing cookie, need to add double quotes around the value
+    // if there isn't any there already.
+    outgoing_cookie = key + "=" + 
+        (((val[0] == '\"') && (val[val.size() - 1] == '\"')) ? val : "\"" + val + "\"");
+  }
+
+  // Function to find cookie matching a given key.
+  std::vector<std::string>::iterator FindCookieByKey(const std::string& key) {
+    auto collision = [key](const std::string& str1) {
+      const std::string::size_type pos = str1.find('=');
+      const std::string old_key = str1.substr(0, pos);
+      return key == old_key;
+    };
+    return std::find_if(expected_cookies.begin(), expected_cookies.end(), collision);
+  }
+
+  // Function to add new cookie or replace existing cookie.
+  void AddOrReplace(const std::string& new_cookie) {
+    // Get position of '=' to get the key.
+    const std::string::size_type pos = new_cookie.find('=');
+    ASSERT_NE(std::string::npos, pos);
+
+    // Get iterator of vector using key. If it exists, replace it, else insert normally.
+    std::vector<std::string>::iterator it = FindCookieByKey(new_cookie.substr(0, pos));
+    if (it != expected_cookies.end()) {
+      *it = new_cookie;
+    } else {
+      expected_cookies.push_back(new_cookie);
+    }
+  }
+
+  // Function to add a cookie to the list of expected cookies.
+  void AddExpected(const std::string& incoming_cookie) {
+    std::string new_cookie;
+    ConvertFormat(incoming_cookie, new_cookie);
+    AddOrReplace(new_cookie);
+  }
+
+  // Function to set a cookie as expired.
+  void SetExpired(const std::string& cookie) {
+    std::string::size_type pos = cookie.find('=');
+    ASSERT_NE(std::string::npos, pos);
+    std::vector<std::string>::iterator it = FindCookieByKey(cookie.substr(0, pos));
+    if (it != expected_cookies.end()) {
+      it = expected_cookies.erase(it);
+    }
+  }
+
+ protected:
+
+  // Class to allow testing of the call headers.
+  class TestCallHeaders : public AddCallHeaders {
+  public:
+    TestCallHeaders() {}
+    ~TestCallHeaders() {}
+
+    // Function to add cookie header.
+    void AddHeader(const std::string& key, const std::string& value) {
+      ASSERT_EQ(key, "cookie");
+      outbound_cookie = value;
+    }
+
+    // Function to get outgoing cookie.
+    std::string GetCookies() {
+      return outbound_cookie;
+    }
+
+  private:
+    std::string outbound_cookie;
+  };
+
+  std::vector<std::string> expected_cookies;
+  std::unique_ptr<ClientMiddleware> middleware_;
+  std::shared_ptr<ClientCookieMiddlewareFactory> factory_;
+};
+
 TEST_F(TestErrorMiddleware, TestMetadata) {
   Action action;
   std::unique_ptr<ResultStream> stream;
@@ -2172,5 +2345,51 @@ TEST_F(TestPropagatingMiddleware, DoPut) {
   ValidateStatus(status, FlightMethod::DoPut);
 }
 
+TEST_F(TestCookieMiddleware, BasicParsing) {
+  AddAndValidate("id1=1; foo=bar;");
+  AddAndValidate("id1=1; foo=bar");
+  AddAndValidate("id2=2;");
+  AddAndValidate("id4=\"4\"");
+  AddAndValidate("id5=5; foo=bar; baz=buz;");
+}
+
+TEST_F(TestCookieMiddleware, Overwrite) {
+  AddAndValidate("id0=0");
+  AddAndValidate("id0=1");
+  AddAndValidate("id1=0");
+  AddAndValidate("id1=1");
+  AddAndValidate("id1=1");
+  AddAndValidate("id1=10");
+  AddAndValidate("id=3");
+  AddAndValidate("id=0");
+  AddAndValidate("id=0");
+}
+
+TEST_F(TestCookieMiddleware, MaxAge) {
+  AddExpiredAndValidate("id0=0; max-age=0;");
+  AddExpiredAndValidate("id1=0; max-age=-1;");
+  AddExpiredAndValidate("id2=0; max-age=0");
+  AddExpiredAndValidate("id3=0; max-age=-1");
+  AddAndValidate("id4=0; max-age=1");
+  AddAndValidate("id5=0; max-age=1");
+  SetExpired("id4=0");
+  AddExpiredAndValidate("id4=0; max-age=0");
+  SetExpired("id5=0");
+  AddExpiredAndValidate("id5=0; max-age=0");
+}
+
+TEST_F(TestCookieMiddleware, Expires) {
+  AddExpiredAndValidate("id0=0; expires=0, 0 0 0 0:0:0 GMT;");
+  AddExpiredAndValidate("id0=0; expires=0, 0 0 0 0:0:0 GMT");
+  AddExpiredAndValidate("id0=0; expires=Fri, 22 Dec 2017 22:15:36 GMT;");
+  AddExpiredAndValidate("id0=0; expires=Fri, 22 Dec 2017 22:15:36 GMT");
+  AddAndValidate("id0=0; expires=Fri, 22 Dec 5000 22:15:36 GMT;");
+  AddAndValidate("id1=0; expires=Fri, 22 Dec 5000 22:15:36 GMT");
+  SetExpired("id0=0");
+  AddExpiredAndValidate("id0=0; expires=Fri, 22 Dec 2017 22:15:36 GMT;");
+  SetExpired("id1=0");
+  AddExpiredAndValidate("id1=0; expires=Fri, 22 Dec 2017 22:15:36 GMT");
+}
+
 }  // namespace flight
 }  // namespace arrow

From 249d59e364025d23e6a61796e904ed27754fdf3a Mon Sep 17 00:00:00 2001
From: Lyndon Bauto <lyndonb@bitquilltech.com>
Date: Fri, 20 Nov 2020 10:04:04 -0800
Subject: [PATCH 13/14] [1] Fixed some linting issues

---
 .../arrow/flight/client_cookie_middleware.cc  |  8 ++---
 cpp/src/arrow/flight/flight_test.cc           | 34 +++++++++----------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/cpp/src/arrow/flight/client_cookie_middleware.cc b/cpp/src/arrow/flight/client_cookie_middleware.cc
index 3cea4b3eba3..11b2a9e4146 100644
--- a/cpp/src/arrow/flight/client_cookie_middleware.cc
+++ b/cpp/src/arrow/flight/client_cookie_middleware.cc
@@ -62,9 +62,8 @@ bool ParseCookieAttribute(std::string cookie_header_value,
   }
 
   std::string::size_type semi_col_pos = cookie_header_value.find(';', equals_pos);
-  out_key =
-      arrow::internal::TrimString(
-          cookie_header_value.substr(start_pos, equals_pos - start_pos));
+  out_key = arrow::internal::TrimString(
+      cookie_header_value.substr(start_pos, equals_pos - start_pos));
   if (std::string::npos == semi_col_pos && semi_col_pos > equals_pos) {
     // Last item - set start pos to end
     out_value = arrow::internal::TrimString(cookie_header_value.substr(equals_pos + 1));
@@ -233,7 +232,8 @@ class ClientCookieMiddlewareFactory::Impl {
 
       // Cache cookies regardless of whether or not they are expired. The server may have
       // explicitly sent a Set-Cookie to expire a cached cookie.
-      std::pair<CookieCache::iterator, bool> insertable = cookie_cache_.insert({cookie.cookie_name_, cookie});
+      std::pair<CookieCache::iterator, bool> insertable =
+          cookie_cache_.insert({cookie.cookie_name_, cookie});
 
       // Force overwrite on insert collision.
       if (!insertable.second) {
diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc
index c1c956f7372..8e06a1752c5 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -43,10 +43,10 @@
 #error "gRPC headers should not be in public API"
 #endif
 
+#include "arrow/flight/client_cookie_middleware.h"
 #include "arrow/flight/internal.h"
 #include "arrow/flight/middleware_internal.h"
 #include "arrow/flight/test_util.h"
-#include "arrow/flight/client_cookie_middleware.h"
 
 namespace pb = arrow::flight::protocol;
 
@@ -996,7 +996,7 @@ class TestErrorMiddleware : public ::testing::Test {
 
 // This test has functions that allow adding and removing cookies from a list of cookies.
 // It also automatically adds cookies to an internal list of tracked cookies when they
-// are passed to the middleware. 
+// are passed to the middleware.
 class TestCookieMiddleware : public ::testing::Test {
  public:
   // Setup function creates middleware factory and starts it up.
@@ -1009,9 +1009,8 @@ class TestCookieMiddleware : public ::testing::Test {
   // Function to add cookie to middleware.
   void AddCookie(const std::string& cookie) {
     CallHeaders call_headers;
-    call_headers.insert(
-      std::make_pair(arrow::util::string_view("set-cookie"), 
-      arrow::util::string_view(cookie)));
+    call_headers.insert(std::make_pair(arrow::util::string_view("set-cookie"),
+                                       arrow::util::string_view(cookie)));
     middleware_->ReceivedHeaders(call_headers);
   }
 
@@ -1026,20 +1025,21 @@ class TestCookieMiddleware : public ::testing::Test {
   std::string trim(std::string s) {
     auto ltrim = [](std::string& s) {
       s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) {
-          return !std::isspace(ch);
-      }));
+                return !std::isspace(ch);
+             }));
     };
     auto rtrim = [](std::string& s) {
-      s.erase(std::find_if(s.rbegin(), s.rend(), [](unsigned char ch) {
-          return !std::isspace(ch);
-      }).base(), s.end());
+      s.erase(std::find_if(s.rbegin(), s.rend(),
+                           [](unsigned char ch) { return !std::isspace(ch); })
+                  .base(),
+              s.end());
     };
     ltrim(s);
     rtrim(s);
     return s;
   };
 
-  // Function to take a list of cookies and split them into a vector of individual 
+  // Function to take a list of cookies and split them into a vector of individual
   // cookies.
   std::vector<std::string> SplitCookies(const std::string& cookies) {
     std::vector<std::string> split_cookies;
@@ -1092,7 +1092,8 @@ class TestCookieMiddleware : public ::testing::Test {
 
     // Add key/value pair to outgoing cookie, need to add double quotes around the value
     // if there isn't any there already.
-    outgoing_cookie = key + "=" + 
+    outgoing_cookie =
+        key + "=" +
         (((val[0] == '\"') && (val[val.size() - 1] == '\"')) ? val : "\"" + val + "\"");
   }
 
@@ -1139,10 +1140,9 @@ class TestCookieMiddleware : public ::testing::Test {
   }
 
  protected:
-
   // Class to allow testing of the call headers.
   class TestCallHeaders : public AddCallHeaders {
-  public:
+   public:
     TestCallHeaders() {}
     ~TestCallHeaders() {}
 
@@ -1153,11 +1153,9 @@ class TestCookieMiddleware : public ::testing::Test {
     }
 
     // Function to get outgoing cookie.
-    std::string GetCookies() {
-      return outbound_cookie;
-    }
+    std::string GetCookies() { return outbound_cookie; }
 
-  private:
+   private:
     std::string outbound_cookie;
   };
 

From 835cd33ca286f9707a886daa247ef801f7fa88db Mon Sep 17 00:00:00 2001
From: Lyndon Bauto <lyndonb@bitquilltech.com>
Date: Fri, 20 Nov 2020 10:33:09 -0800
Subject: [PATCH 14/14] [1] Removing custom trim function for arrow trim
 function

---
 cpp/src/arrow/flight/flight_test.cc | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc
index ed899861d67..5251cf55797 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -38,6 +38,7 @@
 #include "arrow/testing/util.h"
 #include "arrow/util/logging.h"
 #include "arrow/util/make_unique.h"
+#include "arrow/util/string.h"
 
 #ifdef GRPCPP_GRPCPP_H
 #error "gRPC headers should not be in public API"
@@ -1038,24 +1039,6 @@ class TestCookieMiddleware : public ::testing::Test {
     return add_call_headers.GetCookies();
   }
 
-  // Function to trim whitespace from left and right sides of string.
-  std::string trim(std::string s) {
-    auto ltrim = [](std::string& s) {
-      s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) {
-                return !std::isspace(ch);
-             }));
-    };
-    auto rtrim = [](std::string& s) {
-      s.erase(std::find_if(s.rbegin(), s.rend(),
-                           [](unsigned char ch) { return !std::isspace(ch); })
-                  .base(),
-              s.end());
-    };
-    ltrim(s);
-    rtrim(s);
-    return s;
-  };
-
   // Function to take a list of cookies and split them into a vector of individual
   // cookies.
   std::vector<std::string> SplitCookies(const std::string& cookies) {
@@ -1063,11 +1046,12 @@ class TestCookieMiddleware : public ::testing::Test {
     std::string::size_type pos1 = 0;
     std::string::size_type pos2 = 0;
     while ((pos2 = cookies.find(';', pos1)) != std::string::npos) {
-      split_cookies.push_back(trim(cookies.substr(pos1, pos2 - pos1)));
+      split_cookies.push_back(
+          arrow::internal::TrimString(cookies.substr(pos1, pos2 - pos1)));
       pos1 = pos2 + 1;
     }
     if (pos1 < cookies.size()) {
-      split_cookies.push_back(trim(cookies.substr(pos1)));
+      split_cookies.push_back(arrow::internal::TrimString(cookies.substr(pos1)));
     }
     std::sort(split_cookies.begin(), split_cookies.end());
     return split_cookies;