Correct deny_info tokens again.

url_presentable was not just enabling URL-encoding.
It restricts the set of tokens to those which produce data usable in
deny_info URLs.

Renamed to building_deny_info_url for clarity of use and added back in the
restrictions on adding protocol dumps and non-existent page names into the
URL.

diff --git a/src/errorpage.cc b/src/errorpage.cc
index ac5caf45c809772310b62da64664107c1da97a1f..558edd7e82a5c506ab046e7b82c76d5a29f7a9ce 100644 (file)
--- a/src/errorpage.cc
+++ b/src/errorpage.cc
@@ -595,7 +595,7 @@ ErrorState::Dump(MemBuf * mb)
 #define CVT_BUF_SZ 512
 
 const char *
-ErrorState::Convert(char token, bool url_presentable)
+ErrorState::Convert(char token, bool building_deny_info_url)
 {
     static MemBuf mb;
     const char *p = NULL;      /* takes priority over mb if set */
@@ -615,10 +615,12 @@ ErrorState::Convert(char token, bool url_presentable)
         break;
 
     case 'B':
+        if (building_deny_info_url) break;
         p = request ? ftpUrlWith2f(request) : "[no URL]";
         break;
 
     case 'c':
+        if (building_deny_info_url) break;
         p = errorPageName(type);
         break;
 
@@ -634,6 +636,7 @@ ErrorState::Convert(char token, bool url_presentable)
         break;
 
     case 'f':
+        if (building_deny_info_url) break;
         /* FTP REQUEST LINE */
         if (ftp.request)
             p = ftp.request;
@@ -642,6 +645,7 @@ ErrorState::Convert(char token, bool url_presentable)
         break;
 
     case 'F':
+        if (building_deny_info_url) break;
         /* FTP REPLY LINE */
         if (ftp.request)
             p = ftp.reply;
@@ -650,6 +654,7 @@ ErrorState::Convert(char token, bool url_presentable)
         break;
 
     case 'g':
+        if (building_deny_info_url) break;
         /* FTP SERVER MESSAGE */
         if (ftp.server_msg)
             wordlistCat(ftp.server_msg, &mb);
@@ -669,7 +674,7 @@ ErrorState::Convert(char token, bool url_presentable)
                 p = request->hier.host;
             else
                 p = request->GetHost();
-        } else if (!url_presentable)
+        } else if (!building_deny_info_url)
             p = "[unknown host]";
         break;
 
@@ -680,44 +685,47 @@ ErrorState::Convert(char token, bool url_presentable)
     case 'I':
         if (request && request->hier.host[0] != '\0') // if non-empty string
             mb.Printf("%s", request->hier.host);
-        else if (!url_presentable)
+        else if (!building_deny_info_url)
             p = "[unknown]";
         break;
 
     case 'l':
-        if (url_presentable) break;
+        if (building_deny_info_url) break;
         mb.append(error_stylesheet.content(), error_stylesheet.contentSize());
         do_quote = 0;
         break;
 
     case 'L':
-        if (url_presentable) break;
+        if (building_deny_info_url) break;
         if (Config.errHtmlText) {
             mb.Printf("%s", Config.errHtmlText);
             do_quote = 0;
-        } else if (!url_presentable)
+        } else
             p = "[not available]";
         break;
 
     case 'm':
-        if (url_presentable) break;
+        if (building_deny_info_url) break;
         p = auth_user_request->denyMessage("[not available]");
         break;
 
     case 'M':
-        p = request ? RequestMethodStr(request->method) : "[unknown method]";
+        if (request)
+            p = RequestMethodStr(request->method);
+        else if (!building_deny_info_url)
+            p= "[unknown method]";
         break;
 
     case 'o':
         p = request ? request->extacl_message.termedBuf() : external_acl_message;
-        if (!p)
+        if (!p && !building_deny_info_url)
             p = "[not available]";
         break;
 
     case 'p':
         if (request) {
             mb.Printf("%d", (int) request->port);
-        } else if (!url_presentable) {
+        } else if (!building_deny_info_url) {
             p = "[unknown port]";
         }
         break;
@@ -725,13 +733,13 @@ ErrorState::Convert(char token, bool url_presentable)
     case 'P':
         if (request) {
             p = ProtocolStr[request->protocol];
-        } else if (!url_presentable) {
+        } else if (!building_deny_info_url) {
             p = "[unknown protocol]";
         }
         break;
 
     case 'R':
-        if (url_presentable) {
+        if (building_deny_info_url) {
             p = (request->urlpath.size() != 0 ? request->urlpath.termedBuf() : "/");
             break;
         }
@@ -760,7 +768,7 @@ ErrorState::Convert(char token, bool url_presentable)
 
     case 's':
         /* for backward compat we make %s show the full URL. Drop this in some future release. */
-        if (url_presentable) {
+        if (building_deny_info_url) {
             p = request ? urlCanonical(request) : url;
             debugs(0,0, "WARNING: deny_info now accepts coded tags. Use %u to get the full URL instead of %s");
         } else
@@ -768,7 +776,7 @@ ErrorState::Convert(char token, bool url_presentable)
         break;
 
     case 'S':
-        if (url_presentable) {
+        if (building_deny_info_url) {
             p = visible_appname_string;
             break;
         }
@@ -803,42 +811,48 @@ ErrorState::Convert(char token, bool url_presentable)
             p = urlCanonicalFakeHttps(request);
         else if (url)
             p = url;
-        else if (!url_presentable)
+        else if (!building_deny_info_url)
             p = "[no URL]";
         break;
 
     case 'u':
-        p = request ? urlCanonical(request) : url ? url : "[no URL]";
+        if (request)
+            p = urlCanonical(request);
+        else if (url)
+            p = url;
+        else if (!building_deny_info_url)
+            p = "[no URL]";
         break;
 
     case 'w':
         if (Config.adminEmail)
             mb.Printf("%s", Config.adminEmail);
-        else if (!url_presentable)
+        else if (!building_deny_info_url)
             p = "[unknown]";
         break;
 
     case 'W':
+        if (building_deny_info_url) break;
         if (Config.adminEmail && Config.onoff.emailErrData)
             Dump(&mb);
         no_urlescape = 1;
         break;
 
     case 'z':
-        if (url_presentable) break;
+        if (building_deny_info_url) break;
         if (dnsError.size() > 0)
             p = dnsError.termedBuf();
         else if (ftp.cwd_msg)
             p = ftp.cwd_msg;
-        else if (!url_presentable)
+        else
             p = "[unknown]";
         break;
 
     case 'Z':
-        if (url_presentable) break;
+        if (building_deny_info_url) break;
         if (err_msg)
             p = err_msg;
-        else if (!url_presentable)
+        else
             p = "[unknown]";
         break;
 
@@ -862,7 +876,7 @@ ErrorState::Convert(char token, bool url_presentable)
     if (do_quote)
         p = html_quote(p);
 
-    if (url_presentable && !no_urlescape)
+    if (building_deny_info_url && !no_urlescape)
         p = rfc1738_escape_part(p);
 
     return p;

diff --git a/src/errorpage.h b/src/errorpage.h
index 4c6a89303edf092f9f5ef7286961df6a9e9a9c6a..785458efd34f6870ac41b1c324c6f79d3b5f56f0 100644 (file)
--- a/src/errorpage.h
+++ b/src/errorpage.h
@@ -110,10 +110,10 @@ private:
      * Several of the codes produce blocks of non-URL compatible results.
      * When processing the deny_info location URL they will be skipped.
      *
-     * \param token             The token following % which need to be converted
-     * \param url_presentable   URL-encode the the output for deny_info redirect
+     * \param token                    The token following % which need to be converted
+     * \param building_deny_info_url   Perform special deny_info actions, such as URL-encoding and token skipping.
      */
-    const char *Convert(char token, bool url_presentable);
+    const char *Convert(char token, bool building_deny_info_url);
 
     /**
      * CacheManager / Debug dump of the ErrorState object.