Browse Source

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.
Tatsuhiro Tsujikawa 13 years ago
parent
commit
b7dbcea58b
3 changed files with 42 additions and 16 deletions
  1. 34 16
      src/Request.cc
  2. 2 0
      src/util.h
  3. 6 0
      test/RequestTest.cc

+ 34 - 16
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);
 }

+ 2 - 0
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);

+ 6 - 0
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()