Fixed URI scheme case-sensitivity treatment broken since r14802.

A parsed value for the AnyP::UriScheme image constructor parameter was
stored without toLower() canonicalization for known protocols (e.g.,
Squid would store "HTTP" instead of "http" after successfully parsing
"HTTP://EXAMPLE.COM/" in urlParseFinish()). Without that
canonicalization step, Squid violated various HTTP caching rules related
to URI comparison (and served fewer hits) when dealing with absolute
URLs containing non-lowercase HTTP scheme.

According to my limited tests, URL-based ACLs are not affected by this
bug, but I have not investigated how URL-based ACL code differs from
caching code when it comes to stored URL access and whether some ACLs
are actually affected in some environments.

diff --git a/src/anyp/UriScheme.cc b/src/anyp/UriScheme.cc
index 6e2e86ca9dd6253e18dc5798c7e668db8e60793f..2782051b996cb1923866b59331caa41430cb96dd 100644 (file)
--- a/src/anyp/UriScheme.cc
+++ b/src/anyp/UriScheme.cc
@@ -11,24 +11,41 @@
 #include "squid.h"
 #include "anyp/UriScheme.h"
 
+AnyP::UriScheme::LowercaseSchemeNames AnyP::UriScheme::LowercaseSchemeNames_;
+
 AnyP::UriScheme::UriScheme(AnyP::ProtocolType const aScheme, const char *img) :
     theScheme_(aScheme)
 {
-    if (img)
-        // image could be provided explicitly (case-sensitive)
-        image_ = img;
-
-    else if (theScheme_ == AnyP::PROTO_UNKNOWN)
-        // image could be actually unknown and not provided
-        image_ = "(unknown)";
-
-    else if (theScheme_ > AnyP::PROTO_NONE && theScheme_ < AnyP::PROTO_MAX) {
-        // image could be implied by a registered transfer protocol
-        // which use upper-case labels, so down-case for scheme image
-        image_ = AnyP::ProtocolType_str[theScheme_];
-        image_.toLower();
+    // RFC 3986 section 3.1: schemes are case-insensitive.
+
+    // To improve diagnostic, remember exactly how an unsupported scheme looks like.
+    // XXX: Object users may rely on toLower() canonicalization that we refuse to provide.
+    if (img && theScheme_ == AnyP::PROTO_UNKNOWN)
+         image_ = img;
+
+    // XXX: A broken caller supplies an image of an absent scheme?
+    // XXX: We assume that the caller is using a lower-case image.
+    else if (img && theScheme_ == AnyP::PROTO_NONE)
+         image_ = img;
+
+    else if (theScheme_ > AnyP::PROTO_NONE && theScheme_ < AnyP::PROTO_MAX)
+        image_ = LowercaseSchemeNames_.at(theScheme_);
+    // else, the image remains empty (e.g., "://example.com/")
+    // hopefully, theScheme_ is PROTO_NONE here
+}
+
+void
+AnyP::UriScheme::Init()
+{
+    if (LowercaseSchemeNames_.empty()) {
+        LowercaseSchemeNames_.reserve(sizeof(SBuf) * AnyP::PROTO_MAX);
+        // TODO: use base/EnumIterator.h if possible
+        for (int i = AnyP::PROTO_NONE; i < AnyP::PROTO_MAX; ++i) {
+            SBuf image(ProtocolType_str[i]);
+            image.toLower();
+            LowercaseSchemeNames_.emplace_back(image);
+        }
     }
-    // else, image is an empty string ("://example.com/")
 }
 
 unsigned short

diff --git a/src/anyp/UriScheme.h b/src/anyp/UriScheme.h
index 1ad4f2a3c9cef3cb7a09d227e194722ddd1ccc44..f0e0a6f43331fd31b9069b57eaece6edf876d6e0 100644 (file)
--- a/src/anyp/UriScheme.h
+++ b/src/anyp/UriScheme.h
@@ -23,7 +23,10 @@ namespace AnyP
 class UriScheme
 {
 public:
+    typedef std::vector<SBuf> LowercaseSchemeNames;
+
     UriScheme() : theScheme_(AnyP::PROTO_NONE) {}
+    /// \param img Explicit scheme representation for unknown/none schemes.
     UriScheme(AnyP::ProtocolType const aScheme, const char *img = nullptr);
     UriScheme(const AnyP::UriScheme &o) : theScheme_(o.theScheme_), image_(o.image_) {}
     UriScheme(AnyP::UriScheme &&) = default;
@@ -47,7 +50,14 @@ public:
 
     unsigned short defaultPort() const;
 
+    /// initializes down-cased protocol scheme names array
+    static void Init();
+
 private:
+    /// optimization: stores down-cased protocol scheme names, copied from
+    /// AnyP::ProtocolType_str
+    static LowercaseSchemeNames LowercaseSchemeNames_;
+
     /// This is a typecode pointer into the enum/registry of protocols handled.
     AnyP::ProtocolType theScheme_;
 

diff --git a/src/main.cc b/src/main.cc
index 7e064f717b32d699ab6f94e647a40ce91fa35b2d..46e81df48356e5bb2b60335cde5506c3b3221391 100644 (file)
--- a/src/main.cc
+++ b/src/main.cc
@@ -12,6 +12,7 @@
 #include "AccessLogEntry.h"
 #include "acl/Acl.h"
 #include "acl/Asn.h"
+#include "anyp/UriScheme.h"
 #include "auth/Config.h"
 #include "auth/Gadgets.h"
 #include "AuthReg.h"
@@ -1499,6 +1500,8 @@ SquidMain(int argc, char **argv)
 
         Mem::Init();
 
+        AnyP::UriScheme::Init();
+
         storeFsInit();      /* required for config parsing */
 
         /* TODO: call the FS::Clean() in shutdown to do Fs cleanups */

diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc
index b8fe51f1b55c7bd441625767b69369fa36fc770b..35d9d99b68dfe964d467ddbbdeba8633e5f5841b 100644 (file)
--- a/src/tests/testHttpRequest.cc
+++ b/src/tests/testHttpRequest.cc
@@ -31,6 +31,7 @@ void
 testHttpRequest::setUp()
 {
     Mem::Init();
+    AnyP::UriScheme::Init();
     httpHeaderInitModule();
 }
 

diff --git a/src/tests/testURL.cc b/src/tests/testURL.cc
index d88c7b13d1cf3ea13ddd49c4fcbf8d5e3cd556b1..1f2226539960a13ea60022e03f76b92a5e581955 100644 (file)
--- a/src/tests/testURL.cc
+++ b/src/tests/testURL.cc
@@ -25,6 +25,7 @@ void
 testURL::setUp()
 {
     Mem::Init();
+    AnyP::UriScheme::Init();
 }
 
 /*