Skip to content

Commit 5272a68

Browse files
authored
GH-48966: [C++] Fix cookie duplication in the Flight SQL ODBC driver and the Flight Client (#48967)
### Rationale for this change The bug breaks a Flight SQL server that refreshens the auth token when cookie authentication is enabled ### What changes are included in this PR? 1. In the ODBC layer, removed the code that adds a 2nd ClientCookieMiddlewareFactory in the client options (the 1st one is registered in `BuildFlightClientOptions`). This fixes the issue of the duplicate header cookie fields. 2. In the flight client layer, uses the case-insensitive equality comparator instead of the case-insensitive less-than comparator for the cookies cache which is an unordered map. This fixes the issue of duplicate cookie keys. ### Are these changes tested? Manually on Windows, and CI ### Are there any user-facing changes? No * GitHub Issue: #48966 Authored-by: jianfengmao <jianfengmao@deephaven.io> Signed-off-by: David Li <li.davidm96@gmail.com>
1 parent 8010794 commit 5272a68

File tree

3 files changed

+12
-4
lines changed

3 files changed

+12
-4
lines changed

cpp/src/arrow/flight/cookie_internal.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ size_t CaseInsensitiveHash::operator()(const std::string& key) const {
6464
return std::hash<std::string>{}(upper_string);
6565
}
6666

67+
bool CaseInsensitiveEqual::operator()(const std::string& lhs,
68+
const std::string& rhs) const {
69+
return strcasecmp(lhs.c_str(), rhs.c_str()) == 0;
70+
}
71+
6772
Cookie Cookie::Parse(std::string_view cookie_header_value) {
6873
// Parse the cookie string. If the cookie has an expiration, record it.
6974
// If the cookie has a max-age, calculate the current time + max_age and set that as

cpp/src/arrow/flight/cookie_internal.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ class ARROW_FLIGHT_EXPORT CaseInsensitiveComparator {
4141
bool operator()(const std::string& t1, const std::string& t2) const;
4242
};
4343

44+
/// \brief Case insensitive equality comparator for use by unordered cookie map.
45+
class ARROW_FLIGHT_EXPORT CaseInsensitiveEqual {
46+
public:
47+
bool operator()(const std::string& lhs, const std::string& rhs) const;
48+
};
49+
4450
/// \brief Case insensitive hasher for use by cookie caching map. Cookies are not
4551
/// case-sensitive.
4652
class ARROW_FLIGHT_EXPORT CaseInsensitiveHash {
@@ -117,7 +123,7 @@ class ARROW_FLIGHT_EXPORT CookieCache {
117123

118124
// Mutex must be used to protect cookie cache.
119125
std::mutex mutex_;
120-
std::unordered_map<std::string, Cookie, CaseInsensitiveHash, CaseInsensitiveComparator>
126+
std::unordered_map<std::string, Cookie, CaseInsensitiveHash, CaseInsensitiveEqual>
121127
cookies;
122128
};
123129

cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_connection.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,6 @@ void FlightSqlConnection::Connect(const ConnPropertyMap& properties,
157157
client_options_ =
158158
BuildFlightClientOptions(properties, missing_attr, flight_ssl_configs);
159159

160-
const std::shared_ptr<ClientMiddlewareFactory>& cookie_factory = GetCookieFactory();
161-
client_options_.middleware.push_back(cookie_factory);
162-
163160
std::unique_ptr<FlightClient> flight_client;
164161
ThrowIfNotOK(FlightClient::Connect(location, client_options_).Value(&flight_client));
165162
PopulateMetadataSettings(properties);

0 commit comments

Comments
 (0)