From: Benno Rice <benno@squid-cache.org>
Date: Wed, 3 Sep 2008 01:00:39 +0000 (+1000)
Subject: Further cleanup of urlAbsolute and friends.
X-Git-Tag: SQUID_3_1_0_1~49^2~62^2~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=71051277538ad1caed28ad93ccbc0e230e5f3fd6;p=thirdparty%2Fsquid.git

Further cleanup of urlAbsolute and friends.

- Split urlAbsolute into urlIsRelative and urlMakeAbsolute.
- Make urlIsRelative compliant with the RFC as to what defines a relative URL.
- Rework purgeEntriesByHeader to be a little easier on the eyes.
---

diff --git a/src/Server.cc b/src/Server.cc
index 3c5f0ba3e1..ffae195b2d 100644
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -406,19 +406,30 @@ purgeEntriesByHeader(const HttpRequest *req, const char *reqUrl, HttpMsg *rep, h
 {
     const char *hdrUrl, *absUrl;
     
-    if ((hdrUrl = rep->header.getStr(hdr)) != NULL) {
-	    absUrl = urlMakeAbsolute(req, hdrUrl);
-	    if (absUrl != NULL) {
-	        hdrUrl = absUrl;
-	    }
-	    if (absUrl != NULL) { // if the URL was relative, it is by nature the same host
-                purgeEntriesByUrl(hdrUrl);
-	    } else if (sameUrlHosts(reqUrl, hdrUrl)) { // prevent purging DoS, per RFC 2616 13.10, second last paragraph
-                purgeEntriesByUrl(hdrUrl);
-        }
+    absUrl = NULL;
+    hdrUrl = rep->header.getStr(hdr);
+    if (hdrUrl == NULL) {
+        return;
+    }
+    
+    /*
+     * If the URL is relative, make it absolute so we can find it.
+     * If it's absolute, make sure the host parts match to avoid DOS attacks
+     * as per RFC 2616 13.10.
+     */
+    if (urlIsRelative(hdrUrl)) {
+        absUrl = urlMakeAbsolute(req, hdrUrl);
         if (absUrl != NULL) {
-            safe_free(absUrl);
+            hdrUrl = absUrl;
         }
+    } else if (!sameUrlHosts(reqUrl, hdrUrl)) {
+        return;
+    }
+    
+    purgeEntriesByUrl(hdrUrl);
+    
+    if (absUrl != NULL) {
+        safe_free(absUrl);
     }
 }
 
diff --git a/src/url.cc b/src/url.cc
index aa231a93f8..71746c2035 100644
--- a/src/url.cc
+++ b/src/url.cc
@@ -535,8 +535,8 @@ urlCanonicalClean(const HttpRequest * request)
 /*
  * Test if a URL is relative.
  *
- * RFC 1808 says that colons can show up in 'fragments' or 'queries'.
- * Fragments come after a '#' and queries come after '?'.
+ * RFC 2396, Section 5 (Page 17) implies that in a relative URL, a '/' will
+ * appear before a ':'.
  */
 int
 urlIsRelative(const char *url)
@@ -550,7 +550,7 @@ urlIsRelative(const char *url)
         return (0);
     }
 
-    for (p = url; *p != '\0' && *p != ':' && *p != '#' && *p != '?'; p++);
+    for (p = url; *p != '\0' && *p != ':' && *p != '/'; p++);
 
     if (*p == ':') {
         return (0);
@@ -559,9 +559,12 @@ urlIsRelative(const char *url)
 }
 
 /*
- * Take a potentially relative URL.  If the URL is _not_ relative, return NULL.
- * If the URL is relative, generate an absolute URL based on the provided
+ * Convert a relative URL to an absolute URL using the context of a given
  * request.
+ *
+ * It is assumed that you have already ensured that the URL is relative.
+ *
+ * If NULL is returned, you should use the original URL unchanged.
  */
 const char *
 urlMakeAbsolute(const HttpRequest * req, const char *relUrl)
@@ -573,52 +576,52 @@ urlMakeAbsolute(const HttpRequest * req, const char *relUrl)
     if (req->method.id() == METHOD_CONNECT) {
         return (NULL);
     }
-    if (!urlIsRelative(relUrl)) {
-        return (NULL);
-    }
+    
     urlbuf = (char *)xmalloc(MAX_URL * sizeof(char));
+    
     if (req->protocol == PROTO_URN) {
         snprintf(urlbuf, MAX_URL, "urn:%s", req->urlpath.buf());
-    } else {
-    	if (req->port != urlDefaultPort(req->protocol)) {
-    	    urllen = snprintf(urlbuf, MAX_URL, "%s://%s%s%s:%d",
-        		ProtocolStr[req->protocol],
-        		req->login,
-        		*req->login ? "@" : null_string,
-        		req->GetHost(),
-        		req->port
-    	    );
-    	} else {
-    	    urllen = snprintf(urlbuf, MAX_URL, "%s://%s%s%s",
-        		ProtocolStr[req->protocol],
-        		req->login,
-        		*req->login ? "@" : null_string,
-        		req->GetHost()
-    	    );	    
-    	}
-	
-    	if (relUrl[0] == '/') {
-    	    strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
-    	} else {
-    	    path = req->urlpath.buf();
-    	    last_slash = strrchr(path, '/');
-    	    if (last_slash == NULL) {
-        		urlbuf[urllen++] = '/';
-        		strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
-    	    } else {
-        		last_slash++;
-        		pathlen = last_slash - path;
-        		if (pathlen > MAX_URL - urllen - 1) {
-        		    pathlen = MAX_URL - urllen - 1;
-        		}
-        		strncpy(&urlbuf[urllen], path, pathlen);
-        		urllen += pathlen;
-        		if (urllen + 1 < MAX_URL) {
-        		    strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
-        		}
-    	    }
-    	}
+        return (urlbuf);
     }
+        
+	if (req->port != urlDefaultPort(req->protocol)) {
+	    urllen = snprintf(urlbuf, MAX_URL, "%s://%s%s%s:%d",
+    		ProtocolStr[req->protocol],
+    		req->login,
+    		*req->login ? "@" : null_string,
+    		req->GetHost(),
+    		req->port
+	    );
+	} else {
+	    urllen = snprintf(urlbuf, MAX_URL, "%s://%s%s%s",
+    		ProtocolStr[req->protocol],
+    		req->login,
+    		*req->login ? "@" : null_string,
+    		req->GetHost()
+	    );	    
+	}
+
+	if (relUrl[0] == '/') {
+	    strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
+	} else {
+	    path = req->urlpath.buf();
+	    last_slash = strrchr(path, '/');
+	    if (last_slash == NULL) {
+    		urlbuf[urllen++] = '/';
+    		strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
+	    } else {
+    		last_slash++;
+    		pathlen = last_slash - path;
+    		if (pathlen > MAX_URL - urllen - 1) {
+    		    pathlen = MAX_URL - urllen - 1;
+    		}
+    		strncpy(&urlbuf[urllen], path, pathlen);
+    		urllen += pathlen;
+    		if (urllen + 1 < MAX_URL) {
+    		    strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
+    		}
+	    }
+	}
 
     return (urlbuf);
 }