Fix key equality comparison in LookupTable map (#1379)

An std::unordered_map with case-insensitive keys must use a
case-insensitive key equality comparison. lookupTable_t used (the
default) std::equal_to<SBuf> comparison which is case-sensitive.

The full extent of this bug effects is unknown, but Squid was
mishandling Cache-Control and Surrogate-Control directives with
non-canonical (i.e. other than all-lower-case) spelling. Similar
problems affected WWW-Authenticate Digest parameter names, but the
related code remains inconsistent, with both case-sensitive and
case-insensitive checks applied to some of the key names in
Auth::Digest::Config::decode().

Also removed a similarly buggy (and, technically, unused) "typical use"
example from the CaseInsensitiveSBufHash class description. C++
documentation and Squid code are better sources of usage examples when
it comes to STL-used concepts like hash function objects.

This minimal fix excludes LookupTable class polishing.

The bug was introduced in 2015 commit 81ab22b.

diff --git a/src/base/LookupTable.h b/src/base/LookupTable.h
index 2cf6023e6783ed87543a139f374f85f54fa37d24..e410960915288d6991bda3f266ffff6de7f77231 100644 (file)
--- a/src/base/LookupTable.h
+++ b/src/base/LookupTable.h
@@ -70,7 +70,7 @@ public:
     }
 
 private:
-    typedef std::unordered_map<const SBuf, EnumType, Hasher> lookupTable_t;
+    using lookupTable_t = std::unordered_map<const SBuf, EnumType, Hasher, CaseInsensitiveSBufEqual>;
     lookupTable_t lookupTable;
     EnumType invalidValue;
 };

diff --git a/src/sbuf/Algorithms.h b/src/sbuf/Algorithms.h
index 64c475c2c695eac57c9893aeff6a77a58d615e2a..866c522994027dfdd7f345ae16aa601a0c361989 100644 (file)
--- a/src/sbuf/Algorithms.h
+++ b/src/sbuf/Algorithms.h
@@ -112,18 +112,25 @@ struct hash<SBuf>
 };
 }
 
-/** hash functor for SBufs, meant so support case-insensitive std::unordered_map
- *
- * Typical use:
- * \code
- * auto m = std::unordered_map<SBuf, ValueType, CaseInsensitiveSBufHash>();
- * \endcode
- */
+/// hash functor for case-insensitive SBufs
+/// \sa std::hash<SBuf>
 class CaseInsensitiveSBufHash
 {
 public:
     std::size_t operator()(const SBuf &) const noexcept;
 };
 
+/// equality functor for case-insensitive SBufs
+/// \sa std::equal_to<SBuf>
+class CaseInsensitiveSBufEqual
+{
+public:
+    bool operator()(const SBuf &lhs, const SBuf &rhs) const
+    {
+        // Optimization: Do not iterate strings of different lengths.
+        return lhs.length() == rhs.length() && (lhs.compare(rhs, caseInsensitive) == 0);
+    }
+};
+
 #endif /* SQUID_SBUFALGOS_H_ */
 

diff --git a/src/tests/testLookupTable.cc b/src/tests/testLookupTable.cc
index 337fb0d6e078dea1fe00cf2460c129a655a2f6aa..c194954617b5e99dbc35e972cd00fc4213e75993 100644 (file)
--- a/src/tests/testLookupTable.cc
+++ b/src/tests/testLookupTable.cc
@@ -57,6 +57,11 @@ TestLookupTable::testLookupTableLookup()
     CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("six")), ENUM_6);
     CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("seven")), ENUM_7);
 
+    // element found despite a different key spelling
+    CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("One")), ENUM_1);
+    CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("fOUr")), ENUM_4);
+    CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("seveN")), ENUM_7);
+
     // element not found
     CPPUNIT_ASSERT_EQUAL(lt.lookup(SBuf("eleventy")), ENUM_INVALID);
 }