Check scheme part in redirected URI more strictly.

The previous implementation mistakenly sees '/abspath?uri=http://foo'
as an absolute URI because it contains '://', but actually it is not.
This change checks scheme part is constructed in the allowed character
set described in RFC 3986 Section 3.1.

3 files changed, 42 insertions, 16 deletions

diff --git a/src/Request.cc b/src/Request.cc
index 82f7f4e7..b3ab8a21 100644
--- a/src/Request.cc
+++ b/src/Request.cc
@@ -114,23 +114,41 @@ bool Request::redirectUri(const std::string& uri) {
     redirectedUri = getProtocol();
     redirectedUri += ":";
     redirectedUri += uri;
-  } else if(uri.find("://") == std::string::npos) {
-    // rfc2616 requires absolute URI should be provided by Location header
-    // field, but some servers don't obey this rule.
-    // UPDATE: draft-ietf-httpbis-p2-semantics-18 now allows this.
-    uri::UriStruct rus(us_);
-    rus.query.clear();
-    rus.file.clear();
-    size_t offset = 0;
-    if(uri[0] == '/') {
-      // abosulute path
-      rus.dir.clear();
-      offset = 1;
-    }
-    redirectedUri = uri::construct(rus);
-    redirectedUri.append(uri.begin()+offset, uri.end());
   } else {
-    redirectedUri = uri;
+    std::string::size_type schemeEnd = uri.find("://");
+    bool absUri;
+    if(schemeEnd == std::string::npos) {
+      absUri = false;
+    } else {
+      absUri = true;
+      // Check that scheme is acceptable one.
+      for(size_t i = 0; i < schemeEnd; ++i) {
+        char c = uri[i];
+        if(!util::isAlpha(c) && !util::isDigit(c) &&
+           c != '+' && c != '-' && c != '.') {
+          absUri = false;
+          break;
+        }
+      }
+    }
+    if(absUri) {
+      redirectedUri = uri;
+    } else {
+      // rfc2616 requires absolute URI should be provided by Location header
+      // field, but some servers don't obey this rule.
+      // UPDATE: draft-ietf-httpbis-p2-semantics-18 now allows this.
+      uri::UriStruct rus(us_);
+      rus.query.clear();
+      rus.file.clear();
+      size_t offset = 0;
+      if(uri[0] == '/') {
+        // abosulute path
+        rus.dir.clear();
+        offset = 1;
+      }
+      redirectedUri = uri::construct(rus);
+      redirectedUri.append(uri.begin()+offset, uri.end());
+    }
   }
   return parseUri(redirectedUri);
 }
diff --git a/src/util.h b/src/util.h
index 4d2ceb43..80de4d29 100644
--- a/src/util.h
+++ b/src/util.h
@@ -358,6 +358,8 @@ bool isNumber(InputIterator first, InputIterator last)
   return true;
 }
 
+bool isAlpha(const char c);
+
 bool isDigit(const char c);
 
 bool isHexDigit(const char c);
diff --git a/test/RequestTest.cc b/test/RequestTest.cc
index d21af8e7..651b4e52 100644
--- a/test/RequestTest.cc
+++ b/test/RequestTest.cc
@@ -142,6 +142,12 @@ void RequestTest::testRedirectUri()
   CPPUNIT_ASSERT(req.redirectUri("//host/to/file"));
   CPPUNIT_ASSERT_EQUAL(std::string("http://host/to/file"), req.getCurrentUri());
   CPPUNIT_ASSERT_EQUAL(5, req.getRedirectCount());
+
+  // http:// in query part
+  CPPUNIT_ASSERT(req.redirectUri("/abspath?uri=http://foo"));
+  CPPUNIT_ASSERT_EQUAL(std::string("http://host/abspath?uri=http://foo"),
+                       req.getCurrentUri());
+  CPPUNIT_ASSERT_EQUAL(6, req.getRedirectCount());
 }
 
 void RequestTest::testRedirectUri2()