Do not expand errorpage %codes injected by X509 certificates

src/clients/FtpClient.cc

@@ -78,7 +78,7 @@ Ftp::ErrorDetail::brief() const
 }
 
 SBuf
-Ftp::ErrorDetail::verbose(const HttpRequest::Pointer &) const
+Ftp::ErrorDetail::verbose(const ErrorTemplateCompiler &) const
 {
     return ToSBuf("FTP reply with completion code ", completionCode);
 }

src/clients/FtpClient.h

@@ -32,7 +32,7 @@ class ErrorDetail: public ::ErrorDetail {
 
     /* ErrorDetail API */
     SBuf brief() const override;
-    SBuf verbose(const HttpRequestPointer &) const override;
+    SBuf verbose(const ErrorTemplateCompiler &) const override;
 
 private:
     int completionCode; ///< FTP reply completion code

src/error/Detail.cc

@@ -22,7 +22,7 @@ class NamedErrorDetail: public ErrorDetail
 
     /* ErrorDetail API */
     SBuf brief() const override { return name; }
-    SBuf verbose(const HttpRequestPointer &) const override { return name; }
+    SBuf verbose(const ErrorTemplateCompiler &) const override { return name; }
 
 private:
     /// distinguishes us from all other NamedErrorDetail objects

src/error/Detail.h

@@ -21,16 +21,18 @@ class ErrorDetail: public RefCountable
 {
 public:
     using Pointer = ErrorDetailPointer;
+    using ErrorTemplateCompiler = ErrorState;
 
     ~ErrorDetail() override {}
 
     /// \returns a single "token" summarizing available details
     /// suitable as an access.log field and similar output processed by programs
     virtual SBuf brief() const = 0;
 
-    /// \returns all available details; may be customized for the given request
+    /// \returns all available details
     /// suitable for error pages and other output meant for human consumption
-    virtual SBuf verbose(const HttpRequestPointer &) const = 0;
+    /// Supports configurable templates populated by the given compiler.
+    virtual SBuf verbose(const ErrorTemplateCompiler &) const = 0;
 
     // Duplicate details for the same error typically happen when we update some
     // error storage (e.g., ALE) twice from the same detail source (e.g., the

src/error/ExceptionErrorDetail.h

@@ -31,7 +31,7 @@ class ExceptionErrorDetail: public ErrorDetail
         return ToSBuf("exception=", asHex(exceptionId));
     }
 
-    SBuf verbose(const HttpRequestPointer &) const override {
+    SBuf verbose(const ErrorTemplateCompiler &) const override {
         return ToSBuf("Exception (ID=", asHex(exceptionId), ')');
     }
 

src/error/SysErrorDetail.cc

@@ -24,7 +24,7 @@ SysErrorDetail::brief() const
 }
 
 SBuf
-SysErrorDetail::verbose(const HttpRequestPointer &) const
+SysErrorDetail::verbose(const ErrorTemplateCompiler &) const
 {
     return SBuf(xstrerr(errorNo));
 }

src/error/SysErrorDetail.h

@@ -30,7 +30,7 @@ class SysErrorDetail: public ErrorDetail
 
     /* ErrorDetail API */
     SBuf brief() const override;
-    SBuf verbose(const HttpRequestPointer &) const override;
+    SBuf verbose(const ErrorTemplateCompiler &) const override;
 
 private:
     // hidden by NewIfAny() to avoid creating SysErrorDetail from zero errno

src/error/forward.h

@@ -92,6 +92,11 @@ class Error;
 class ErrorDetail;
 class ErrorState;
 
+namespace ErrorPage {
+class PercentCodeCompiler;
+class Build;
+} // namespace ErrorPage
+
 typedef RefCount<ErrorDetail> ErrorDetailPointer;
 
 #endif /* SQUID_SRC_ERROR_FORWARD_H */

src/errorpage.cc

@@ -102,16 +102,6 @@ class ErrorDynamicPageInfo {
 
 namespace ErrorPage {
 
-/// state and parameters shared by several ErrorState::compile*() methods
-class Build
-{
-public:
-    SBuf output; ///< compilation result
-    const char *input = nullptr; ///< template bytes that need to be compiled
-    bool building_deny_info_url = false; ///< whether we compile deny_info URI
-    bool allowRecursion = false; ///< whether top-level compile() calls are OK
-};
-
 /// pretty-prints error page/deny_info building error
 class BuildErrorPrinter
 {
@@ -1002,9 +992,7 @@ ErrorState::compileLegacyCode(Build &build)
         if (!build.allowRecursion)
             p = "%D";  // if recursion is not allowed, do not convert
         else if (detail) {
-            auto rawDetail = detail->verbose(request);
-            // XXX: Performance regression. c_str() reallocates
-            const auto compiledDetail = compileBody(rawDetail.c_str(), false);
+            const auto compiledDetail = detail->verbose(*this);
             mb.append(compiledDetail.rawContent(), compiledDetail.length());
             do_quote = 0;
         }
@@ -1456,23 +1444,55 @@ ErrorState::compile(const char *input, bool building_deny_info_url, bool allowRe
     build.allowRecursion = allowRecursion;
     build.input = input;
 
+    compile(build);
+    Assure(!*build.input); // compiled the whole template
+
+    return build.output;
+}
+
+SBuf
+ErrorState::compileDetail(const char * const format, const ErrorPage::PercentCodeCompiler * const secondaryCompiler) const
+{
+    Assure(format);
+
+    Build build;
+    build.input = format;
+    build.secondaryCompiler = secondaryCompiler; // may be nil
+
+    compile(build);
+    Assure(!*build.input); // compiled the whole template
+
+    return build.output;
+}
+
+/// Replaces all %code sequences found in build.input.
+/// Appends processed input to build.output.
+/// Consumes processed build.input.
+void
+ErrorState::compile(Build &build) const
+{
+    Assure(build.input);
+
+    // TODO: Instead of violating const-correctness with const_cast<ErrorState*>
+    // below, adjust compile*() methods to avoid ErrorState modifications.
+
     auto blockStart = build.input;
     while (const auto letter = *build.input) {
         if (letter == '%') {
             build.output.append(blockStart, build.input - blockStart);
-            compileLegacyCode(build);
+            if (!build.secondaryCompiler || !build.secondaryCompiler->compilePercentCode(build))
+                const_cast<ErrorState*>(this)->compileLegacyCode(build);
             blockStart = build.input;
         }
         else if (letter == '@' && LogformatMagic.cmp(build.input, LogformatMagic.length()) == 0) {
             build.output.append(blockStart, build.input - blockStart);
-            compileLogformatCode(build);
+            const_cast<ErrorState*>(this)->compileLogformatCode(build);
             blockStart = build.input;
         } else {
             ++build.input;
         }
     }
     build.output.append(blockStart, build.input - blockStart);
-    return build.output;
 }
 
 /// react to a compile() error

src/errorpage.h

@@ -11,6 +11,7 @@
 #ifndef SQUID_SRC_ERRORPAGE_H
 #define SQUID_SRC_ERRORPAGE_H
 
+#include "base/TypeTraits.h"
 #include "cbdata.h"
 #include "comm/forward.h"
 #include "error/Detail.h"
@@ -78,18 +79,14 @@ class MemBuf;
 class StoreEntry;
 class wordlist;
 
-namespace ErrorPage {
-
-class Build;
-
-} // namespace ErrorPage
-
 /// \ingroup ErrorPageAPI
 class ErrorState
 {
     CBDATA_CLASS(ErrorState);
 
 public:
+    using Build = ErrorPage::Build;
+
     /// creates an error of type other than ERR_RELAY_REMOTE
     ErrorState(err_type type, Http::StatusCode, HttpRequest * request, const AccessLogEntryPointer &al);
     ErrorState() = delete; // not implemented.
@@ -113,12 +110,15 @@ class ErrorState
     /// ensures that a future BuildHttpReply() is likely to succeed
     void validate();
 
+    /// Replaces all %codes in the given ErrorDetail `format` template.
+    /// \param compiler is an optional handler for caller-specific template %code sequences
+    /// \sa compile()
+    SBuf compileDetail(const char *format, const ErrorPage::PercentCodeCompiler *compiler) const;
+
     /// the source of the error template (for reporting purposes)
     SBuf inputLocation;
 
 private:
-    typedef ErrorPage::Build Build;
-
     /// initializations shared by public constructors
     ErrorState(err_type, const AccessLogEntryPointer &);
 
@@ -141,8 +141,11 @@ class ErrorState
     /// \param building_deny_info_url  whether input is a deny_info URL parameter
     /// \param allowRecursion  whether to compile %codes which produce %codes
     /// \returns the given input with all %codes substituted
+    /// \sa compileDetail()
     SBuf compile(const char *input, bool building_deny_info_url, bool allowRecursion);
 
+    void compile(Build &build) const;
+
     /// React to a compile() error, throwing if buildContext allows.
     /// \param msg description of what went wrong
     /// \param errorLocation approximate start of the problematic input
@@ -336,6 +339,41 @@ class TemplateFile
     err_type templateCode; ///< The internal code for this template.
 };
 
+namespace ErrorPage {
+
+/// An API for recognizing and replacing a single %code sequence.
+class PercentCodeCompiler: public Interface {
+public:
+    /// Either recognizes a single %code sequence at the beginning of
+    /// `build.input` and appends its replacement to `build.output` OR does not
+    /// modify `build`.
+    /// \returns true when the leading %code was recognized
+    /// \prec `build.input` starts with a `%` character.
+    virtual bool compilePercentCode(Build &) const = 0;
+};
+
+/// State and parameters shared by several
+/// PercentCodeCompiler::compilePercentCode() and ErrorState::compile*() methods
+/// that convert an errorpage template fragment (or equivalent) into an error
+/// response body fragment. This conversion replaces legacy errorpage %code
+/// sequences, logformat %code sequences, and, in some cases,
+/// ErrorDetail-dependent %code sequences.
+class Build
+{
+public:
+    SBuf output; ///< compilation result
+    const char *input; ///< template bytes that need to be compiled; never nil
+
+    /// Handles context-dependent %code sequences unsupported by ErrorState.
+    /// Compiler object lifetime must exceed this Build object lifetime.
+    const PercentCodeCompiler *secondaryCompiler = nullptr;
+
+    bool building_deny_info_url = false; ///< whether we compile deny_info URI
+    bool allowRecursion = false; ///< whether top-level compile() calls are OK
+};
+
+} // namespace ErrorPage
+
 /**
  * Parses the Accept-Language header value and return one language item on
  * each call.

src/mk-string-arrays.awk

@@ -25,8 +25,11 @@ BEGIN {
         nspath = ""
 }
 
-# when namespace is encountered store it
+# Remember the name of the last namespace encountered before the enum.
+# XXX: We should remember the name(s) of the namespace(s) surrounding the enum
+# instead. TODO: Replace this C++ parsing hack with a command-line parameter.
 /^namespace *[a-zA-Z]+/	{
+	if (type) next
 	nspath = tolower($2) "/"		# nested folder
 	namespace = $2				# code namespace reconstruct
 	next

src/security/ErrorDetail.cc

@@ -531,33 +531,19 @@ Security::ErrorDetail::brief() const
 }
 
 SBuf
-Security::ErrorDetail::verbose(const HttpRequestPointer &request) const
+Security::ErrorDetail::verbose(const ErrorTemplateCompiler &compiler) const
 {
     std::optional<SBuf> customFormat;
 #if USE_OPENSSL
-    if (const auto errorDetail = Ssl::ErrorDetailsManager::GetInstance().findDetail(error_no, request)) {
+    if (const auto errorDetail = Ssl::ErrorDetailsManager::GetInstance().findDetail(error_no, compiler.request)) {
         detailEntry = *errorDetail;
         customFormat = detailEntry->detail;
     } else {
         detailEntry.reset();
     }
-#else
-    (void)request;
 #endif
     auto format = customFormat ? customFormat->c_str() : "SSL handshake error (%err_name)";
-
-    SBufStream os;
-    assert(format);
-    auto remainder = format;
-    while (auto p = strchr(remainder, '%')) {
-        os.write(remainder, p - remainder);
-        const auto formattingCodeLen = convertErrorCodeToDescription(++p, os);
-        if (!formattingCodeLen)
-            os << '%';
-        remainder = p + formattingCodeLen;
-    }
-    os << remainder;
-    return os.buf();
+    return compiler.compileDetail(format, this); // calls our compilePercentCode() as needed
 }
 
 /// textual representation of the subject of the broken certificate
@@ -739,6 +725,24 @@ Security::ErrorDetail::printErrorLibError(std::ostream &os) const
         os << "[No Error]";
 }
 
+bool
+Security::ErrorDetail::compilePercentCode(ErrorPage::Build &build) const
+{
+    Assure(build.input);
+    Assure(*build.input == '%');
+    const auto codeNameStart = build.input + 1;
+    SBufStream os;
+    if (const auto codeNameLength = convertErrorCodeToDescription(codeNameStart, os)) {
+        Assure(build.input < codeNameStart + codeNameLength); // making parsing progress
+        Assure(codeNameStart + codeNameLength <= build.input + strlen(build.input)); // still within bounds
+        build.input = codeNameStart + codeNameLength;
+        build.output.append(os.buf());
+        return true;
+    }
+
+    return false; // including bare/trailing '%' cases
+}
+
 /**
  * Converts the code to a string value. Supported formatting codes are:
  *
@@ -783,7 +787,6 @@ Security::ErrorDetail::convertErrorCodeToDescription(const char * const code, st
         }
     }
 
-    // TODO: Support logformat %codes.
     return 0;
 }
 

src/security/ErrorDetail.h

@@ -11,6 +11,7 @@
 
 #include "base/RefCount.h"
 #include "error/Detail.h"
+#include "errorpage.h"
 #include "http/forward.h"
 #include "security/forward.h"
 #include "SquidString.h"
@@ -36,7 +37,7 @@ namespace Security {
 /// * for certificate validation errors: validation failure reason
 /// * for non-validation errors: TLS library-reported error(s)
 /// * for non-validation errors: system call errno(3)
-class ErrorDetail: public ::ErrorDetail
+class ErrorDetail: public ::ErrorDetail, public ErrorPage::PercentCodeCompiler
 {
     MEMPROXY_CLASS(Security::ErrorDetail);
 
@@ -64,7 +65,7 @@ class ErrorDetail: public ::ErrorDetail
 
     /* ErrorDetail API */
     SBuf brief() const override;
-    SBuf verbose(const HttpRequestPointer &) const override;
+    SBuf verbose(const ErrorTemplateCompiler &) const override;
 
     /// \returns error category; \see ErrorCode
     ErrorCode errorNo() const { return error_no; }
@@ -87,6 +88,9 @@ class ErrorDetail: public ::ErrorDetail
 private:
     ErrorDetail(ErrorCode err, int aSysErrorNo);
 
+    /* ErrorPage::PercentCodeCompiler API */
+    bool compilePercentCode(ErrorPage::Build &) const override;
+
     /* methods for formatting error details using admin-configurable %codes */
     void printSubject(std::ostream &os) const;
     void printCaName(std::ostream &os) const;