Address Alex's comments.

- Use bool instead of int for urlIsRelative.
- Document what leads to a NULL return in urlMakeAbsolute.
- Declare variables closer to where they're used.
- Fix indentation.

diff --git a/src/protos.h b/src/protos.h
index 48f16947043791ab2277ee74ac2f8a413080ae3d..b18b0b14499770bd4aac7dac3eca653f46fc10e4 100644 (file)
--- a/src/protos.h
+++ b/src/protos.h
@@ -638,8 +638,8 @@ SQUIDCEXTERN protocol_t urlParseProtocol(const char *, const char *e = NULL);
 SQUIDCEXTERN void urlInitialize(void);
 SQUIDCEXTERN HttpRequest *urlParse(const HttpRequestMethod&, char *, HttpRequest *request = NULL);
 SQUIDCEXTERN const char *urlCanonical(HttpRequest *);
-SQUIDCEXTERN int urlIsRelative(const char *);
-SQUIDCEXTERN const char *urlMakeAbsolute(const HttpRequest *, const char *);
+SQUIDCEXTERN bool urlIsRelative(const char *);
+SQUIDCEXTERN char *urlMakeAbsolute(const HttpRequest *, const char *);
 SQUIDCEXTERN char *urlRInternal(const char *host, u_short port, const char *dir, const char *name);
 SQUIDCEXTERN char *urlInternal(const char *dir, const char *name);
 SQUIDCEXTERN int matchDomainName(const char *host, const char *domain);

diff --git a/src/url.cc b/src/url.cc
index 71746c2035c5a4e67e220809092723ab1930f4cb..6c06690a464e89559f291b01a5653649a1878110 100644 (file)
--- a/src/url.cc
+++ b/src/url.cc
@@ -538,24 +538,24 @@ urlCanonicalClean(const HttpRequest * request)
  * RFC 2396, Section 5 (Page 17) implies that in a relative URL, a '/' will
  * appear before a ':'.
  */
-int
+bool
 urlIsRelative(const char *url)
 {
     const char *p;
 
     if (url == NULL) {
-        return (0);
+        return (false);
     }
     if (*url == '\0') {
-        return (0);
+        return (false);
     }
 
     for (p = url; *p != '\0' && *p != ':' && *p != '/'; p++);
 
     if (*p == ':') {
-        return (0);
+        return (false);
     }
-    return (1);
+    return (true);
 }
 
 /*
@@ -564,64 +564,66 @@ urlIsRelative(const char *url)
  *
  * It is assumed that you have already ensured that the URL is relative.
  *
- * If NULL is returned, you should use the original URL unchanged.
+ * If NULL is returned it is an indication that the method in use in the
+ * request does not distinguish between relative and absolute and you should
+ * use the url unchanged.
  */
-const char *
+char *
 urlMakeAbsolute(const HttpRequest * req, const char *relUrl)
 {
-    char *urlbuf;
-    const char *path, *last_slash;
-    size_t urllen, pathlen;
 
     if (req->method.id() == METHOD_CONNECT) {
         return (NULL);
     }
     
-    urlbuf = (char *)xmalloc(MAX_URL * sizeof(char));
+    char *urlbuf = (char *)xmalloc(MAX_URL * sizeof(char));
     
     if (req->protocol == PROTO_URN) {
         snprintf(urlbuf, MAX_URL, "urn:%s", req->urlpath.buf());
         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);
-               }
-           }
-       }
+    size_t urllen;
+
+    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 {
+        const char *path = req->urlpath.buf();
+        const char *last_slash = strrchr(path, '/');
+
+        if (last_slash == NULL) {
+               urlbuf[urllen++] = '/';
+            strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1);
+        } else {
+            last_slash++;
+            size_t 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);
 }