diff --git a/src/clients/FtpClient.cc b/src/clients/FtpClient.cc index c9db1ea7e61..6243a7071a8 100644 --- a/src/clients/FtpClient.cc +++ b/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); } diff --git a/src/clients/FtpClient.h b/src/clients/FtpClient.h index 600247fb7d0..8f8ffc4e7c2 100644 --- a/src/clients/FtpClient.h +++ b/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 diff --git a/src/error/Detail.cc b/src/error/Detail.cc index 17e0879b3b2..1cd8fcbfdae 100644 --- a/src/error/Detail.cc +++ b/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 diff --git a/src/error/Detail.h b/src/error/Detail.h index 9a6c3b23fed..4caaf106129 100644 --- a/src/error/Detail.h +++ b/src/error/Detail.h @@ -21,6 +21,7 @@ class ErrorDetail: public RefCountable { public: using Pointer = ErrorDetailPointer; + using ErrorTemplateCompiler = ErrorState; ~ErrorDetail() override {} @@ -28,9 +29,10 @@ class ErrorDetail: public RefCountable /// 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 diff --git a/src/error/ExceptionErrorDetail.h b/src/error/ExceptionErrorDetail.h index eb105f59838..1763a8abb75 100644 --- a/src/error/ExceptionErrorDetail.h +++ b/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), ')'); } diff --git a/src/error/SysErrorDetail.cc b/src/error/SysErrorDetail.cc index 260f521552a..a39fb85c0fe 100644 --- a/src/error/SysErrorDetail.cc +++ b/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)); } diff --git a/src/error/SysErrorDetail.h b/src/error/SysErrorDetail.h index 829b1bac601..3c94f572e21 100644 --- a/src/error/SysErrorDetail.h +++ b/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 diff --git a/src/error/forward.h b/src/error/forward.h index e66bd492dfe..0769a4d328e 100644 --- a/src/error/forward.h +++ b/src/error/forward.h @@ -92,6 +92,11 @@ class Error; class ErrorDetail; class ErrorState; +namespace ErrorPage { +class PercentCodeCompiler; +class Build; +} // namespace ErrorPage + typedef RefCount ErrorDetailPointer; #endif /* SQUID_SRC_ERROR_FORWARD_H */ diff --git a/src/errorpage.cc b/src/errorpage.cc index 2569d3b3ca5..352d83d515f 100644 --- a/src/errorpage.cc +++ b/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 + // 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(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(this)->compileLogformatCode(build); blockStart = build.input; } else { ++build.input; } } build.output.append(blockStart, build.input - blockStart); - return build.output; } /// react to a compile() error diff --git a/src/errorpage.h b/src/errorpage.h index 350484fe504..90983c82db9 100644 --- a/src/errorpage.h +++ b/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. diff --git a/src/mk-string-arrays.awk b/src/mk-string-arrays.awk index c9695b1683a..6bc1fa9ccfd 100644 --- a/src/mk-string-arrays.awk +++ b/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 diff --git a/src/security/ErrorDetail.cc b/src/security/ErrorDetail.cc index afe9fd2e0eb..4a67763bb77 100644 --- a/src/security/ErrorDetail.cc +++ b/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 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; } diff --git a/src/security/ErrorDetail.h b/src/security/ErrorDetail.h index b027ca80b34..a690689a2e4 100644 --- a/src/security/ErrorDetail.h +++ b/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; diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index bb236a3bc6e..f7d57d8b144 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -1103,9 +1103,18 @@ Ftp::Server::writeErrorReply(const HttpReply *reply, const int scode) if (request->error) mb.appendf("%i-%s\r\n", scode, errorPageName(request->error.category)); - for (const auto &detail: request->error.details) { - mb.appendf("%i-Error-Detail-Brief: " SQUIDSBUFPH "\r\n", scode, SQUIDSBUFPRINT(detail->brief())); - mb.appendf("%i-Error-Detail-Verbose: " SQUIDSBUFPH "\r\n", scode, SQUIDSBUFPRINT(detail->verbose(request))); + if (!request->error.details.empty()) { + // Addressing TODO at the end of this method should remove this temporary hack. + // XXX: ErrorState currently requires non-constant HttpRequest and HttpReply. + const auto err = std::make_unique( + const_cast(request), + const_cast(reply), + pipeline.front()->http->al); + + for (const auto &detail: request->error.details) { + mb.appendf("%i-Error-Detail-Brief: " SQUIDSBUFPH "\r\n", scode, SQUIDSBUFPRINT(detail->brief())); + mb.appendf("%i-Error-Detail-Verbose: " SQUIDSBUFPH "\r\n", scode, SQUIDSBUFPRINT(detail->verbose(*err))); + } } #if USE_ADAPTATION diff --git a/src/tests/stub_liberror.cc b/src/tests/stub_liberror.cc index 2d7bd98bc0a..d2caf9204df 100644 --- a/src/tests/stub_liberror.cc +++ b/src/tests/stub_liberror.cc @@ -29,6 +29,6 @@ ErrorDetail::Pointer MakeNamedErrorDetail(const char *) STUB_RETVAL(ErrorDetail: #include "error/SysErrorDetail.h" SBuf SysErrorDetail::Brief(int) STUB_RETVAL(SBuf()) SBuf SysErrorDetail::brief() const STUB_RETVAL(SBuf()) -SBuf SysErrorDetail::verbose(const HttpRequestPointer &) const STUB_RETVAL(SBuf()) +SBuf SysErrorDetail::verbose(const ErrorTemplateCompiler &) const STUB_RETVAL(SBuf()) std::ostream &operator <<(std::ostream &os, ReportSysError) STUB_RETVAL(os) diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index 86989daab7f..cfd2eaee76e 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -70,8 +70,9 @@ Security::ErrorDetail::ErrorDetail(ErrorCode, int, int) STUB Security::ErrorDetail::ErrorDetail(ErrorCode, LibErrorCode, int) STUB #endif void Security::ErrorDetail::setPeerCertificate(const CertPointer &) STUB -SBuf Security::ErrorDetail::verbose(const HttpRequestPointer &) const STUB_RETVAL(SBuf()) +SBuf Security::ErrorDetail::verbose(const ErrorTemplateCompiler &) const STUB_RETVAL(SBuf()) SBuf Security::ErrorDetail::brief() const STUB_RETVAL(SBuf()) +bool Security::ErrorDetail::compilePercentCode(ErrorPage::Build &) const STUB_RETVAL(false) Security::ErrorCode Security::ErrorCodeFromName(const char *) STUB_RETVAL(0) const char *Security::ErrorNameFromCode(ErrorCode, bool) STUB_RETVAL("")