From 133ade7a5d9fad39d39a42ecb7fe15f7a1b17390 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli <5175948+kinkie@users.noreply.github.com> Date: Tue, 3 Oct 2023 22:37:13 +0000 Subject: [PATCH] Maintenance: use SBufs for ErrorDetailEntry data fields (#1451) Also fixed (re)configuration crashes when an error-details.txt entry is missing a required "detail" or "descr" field. --- src/HttpHeader.h | 20 ++++ src/HttpHeaderTools.cc | 11 ++ src/security/ErrorDetail.cc | 204 ++++++++++++++++++++-------------- src/security/ErrorDetail.h | 25 +++-- src/ssl/ErrorDetail.cc | 7 +- src/ssl/ErrorDetail.h | 7 +- src/ssl/ErrorDetailManager.cc | 112 ++++++++----------- src/ssl/ErrorDetailManager.h | 39 +++---- src/ssl/support.cc | 4 +- src/tests/stub_HttpHeader.cc | 1 + 10 files changed, 245 insertions(+), 185 deletions(-) diff --git a/src/HttpHeader.h b/src/HttpHeader.h index 7657749398b..b9d87627923 100644 --- a/src/HttpHeader.h +++ b/src/HttpHeader.h @@ -199,6 +199,26 @@ class HttpHeader int httpHeaderParseQuotedString(const char *start, const int len, String *val); +namespace Http { + +/** + * Parses an HTTP quoted-string sequence (RFC 9110, Section 5.6.4). + * + * \param a brief human-friendly description of the string being parsed + * \param start input buffer (an opening double-quote is expected at *start) + * \param length is the number of characters in the given buffer + * + * \returns string contents with open/closing quotes stripped and any quoted-pairs decoded + * + * Avoid this slow function on performance-sensitive code paths. + * TODO: Replace with an efficient, SBuf-friendly implementation. + * + * \sa httpHeaderParseQuotedString() for a String-friendly function. + */ +SBuf SlowlyParseQuotedString(const char *description, const char *start, size_t length); + +} + /// quotes string using RFC 7230 quoted-string rules SBuf httpHeaderQuoteString(const char *raw); diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc index 938f9289624..1e6c5253523 100644 --- a/src/HttpHeaderTools.cc +++ b/src/HttpHeaderTools.cc @@ -27,6 +27,8 @@ #include "HttpHeaderTools.h" #include "HttpRequest.h" #include "MemBuf.h" +#include "sbuf/Stream.h" +#include "sbuf/StringConvert.h" #include "SquidConfig.h" #include "Store.h" #include "StrList.h" @@ -233,6 +235,15 @@ httpHeaderParseQuotedString(const char *start, const int len, String *val) return 1; } +SBuf +Http::SlowlyParseQuotedString(const char * const description, const char * const start, const size_t length) +{ + String s; + if (!httpHeaderParseQuotedString(start, length, &s)) + throw TextException(ToSBuf("Cannot parse ", description, " as a quoted string"), Here()); + return StringToSBuf(s); +} + SBuf httpHeaderQuoteString(const char *raw) { diff --git a/src/security/ErrorDetail.cc b/src/security/ErrorDetail.cc index 9279ed84211..8788d7d896b 100644 --- a/src/security/ErrorDetail.cc +++ b/src/security/ErrorDetail.cc @@ -26,6 +26,7 @@ #endif #endif #include +#include namespace Security { @@ -498,15 +499,17 @@ Security::ErrorDetail::setPeerCertificate(const CertPointer &cert) SBuf Security::ErrorDetail::brief() const { - SBuf buf(err_code()); // TODO: Upgrade err_code()/etc. to return SBuf. + SBufStream os; + + err_code(os); if (lib_error_no) { #if USE_OPENSSL // TODO: Log ERR_error_string_n() instead, despite length, whitespace? // Example: `error:1408F09C:SSL routines:ssl3_get_record:http request`. - buf.append(ToSBuf("+TLS_LIB_ERR=", std::hex, std::uppercase, lib_error_no)); + os << "+TLS_LIB_ERR=" << std::hex << std::uppercase << lib_error_no << std::nouppercase << std::dec; #elif USE_GNUTLS - buf.append(ToSBuf("+", gnutls_strerror_name(lib_error_no))); + os << '+' << gnutls_strerror_name(lib_error_no); #endif } @@ -514,185 +517,217 @@ Security::ErrorDetail::brief() const // TODO: Consider logging long but human-friendly names (e.g., // SSL_ERROR_SYSCALL). if (ioErrorNo) - buf.append(ToSBuf("+TLS_IO_ERR=", ioErrorNo)); + os << "+TLS_IO_ERR=" << ioErrorNo; #endif if (sysErrorNo) { - buf.append('+'); - buf.append(SysErrorDetail::Brief(sysErrorNo)); + os << '+' << SysErrorDetail::Brief(sysErrorNo); } if (broken_cert) - buf.append("+broken_cert"); + os << "+broken_cert"; - return buf; + return os.buf(); } SBuf Security::ErrorDetail::verbose(const HttpRequestPointer &request) const { - char const *format = nullptr; + std::optional customFormat; #if USE_OPENSSL - if (Ssl::ErrorDetailsManager::GetInstance().getErrorDetail(error_no, request, detailEntry)) - format = detailEntry.detail.termedBuf(); + if (const auto errorDetail = Ssl::ErrorDetailsManager::GetInstance().findDetail(error_no, request)) + customFormat = errorDetail->detail; #else (void)request; #endif - if (!format) - format = "SSL handshake error (%err_name)"; + auto format = customFormat ? customFormat->c_str() : "SSL handshake error (%err_name)"; - SBuf errDetailStr; + SBufStream os; assert(format); auto remainder = format; while (auto p = strchr(remainder, '%')) { - errDetailStr.append(remainder, p - remainder); - char const *converted = nullptr; - const auto formattingCodeLen = convert(++p, &converted); - if (formattingCodeLen) - errDetailStr.append(converted); - else - errDetailStr.append("%"); + os.write(remainder, p - remainder); + const auto formattingCodeLen = convert(++p, os); + if (!formattingCodeLen) + os << '%'; remainder = p + formattingCodeLen; } - errDetailStr.append(remainder, strlen(remainder)); - return errDetailStr; + os << remainder; + return os.buf(); } /// textual representation of the subject of the broken certificate -const char * -Security::ErrorDetail::subject() const +void +Security::ErrorDetail::subject(std::ostream &os) const { if (broken_cert) { auto buf = SubjectName(*broken_cert); if (!buf.isEmpty()) { + // TODO: Convert html_quote() into an std::ostream manipulator. // quote to avoid possible html code injection through // certificate subject - return html_quote(buf.c_str()); + os << html_quote(buf.c_str()); + return; } } - return "[Not available]"; + os << "[Not available]"; } #if USE_OPENSSL -/// helper function to collect CNs using Ssl::matchX509CommonNames() -static int -copy_cn(void *check_data, ASN1_STRING *cn_data) +/// a helper class to print CNs extracted using Ssl::matchX509CommonNames() +class CommonNamesPrinter { - const auto str = static_cast(check_data); - if (!str) // no data? abort - return 0; - if (cn_data && cn_data->length) { - if (str->size() > 0) - str->append(", "); - str->append(reinterpret_cast(cn_data->data), cn_data->length); - } +public: + explicit CommonNamesPrinter(std::ostream &os): os_(os) {} + + /// Ssl::matchX509CommonNames() visitor that reports the given name (if any) + static int PrintName(void *, ASN1_STRING *); + + /// whether any names have been printed so far + bool printed = false; + +private: + void printName(const ASN1_STRING *); + + std::ostream &os_; ///< destination for printed names +}; + +int +CommonNamesPrinter::PrintName(void * const printer, ASN1_STRING * const name) +{ + assert(printer); + static_cast(printer)->printName(name); return 1; } + +/// prints an HTML-quoted version of the given common name (as a part of the +/// printed names list) +void +CommonNamesPrinter::printName(const ASN1_STRING * const name) +{ + if (name && name->length) { + if (printed) + os_ << ", "; + + // TODO: Convert html_quote() into an std::ostream manipulator accepting (buf, n). + SBuf buf(reinterpret_cast(name->data), name->length); + os_ << html_quote(buf.c_str()); + + printed = true; + } +} #endif // USE_OPENSSL /// a list of the broken certificates CN and alternate names -const char * -Security::ErrorDetail::cn() const +void +Security::ErrorDetail::cn(std::ostream &os) const { #if USE_OPENSSL if (broken_cert.get()) { - static String tmpStr; - tmpStr.clean(); - Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn); - if (tmpStr.size()) { - // quote to avoid possible HTML code injection through - // certificate subject - return html_quote(tmpStr.termedBuf()); - } + CommonNamesPrinter printer(os); + Ssl::matchX509CommonNames(broken_cert.get(), &printer, printer.PrintName); + if (printer.printed) + return; } #endif // USE_OPENSSL - return "[Not available]"; + os << "[Not available]"; } /// the issuer of the broken certificate -const char * -Security::ErrorDetail::ca_name() const +void +Security::ErrorDetail::ca_name(std::ostream &os) const { if (broken_cert) { auto buf = IssuerName(*broken_cert); if (!buf.isEmpty()) { // quote to avoid possible html code injection through // certificate issuer subject - return html_quote(buf.c_str()); + os << html_quote(buf.c_str()); + return; } } - return "[Not available]"; + os << "[Not available]"; } /// textual representation of the "not before" field of the broken certificate -const char * -Security::ErrorDetail::notbefore() const +void +Security::ErrorDetail::notbefore(std::ostream &os) const { #if USE_OPENSSL if (broken_cert.get()) { if (const auto tm = X509_getm_notBefore(broken_cert.get())) { + // TODO: Add and use an ASN1_TIME printing operator instead. static char tmpBuffer[256]; // A temporary buffer Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer)); - return tmpBuffer; + os << tmpBuffer; + return; } } #endif // USE_OPENSSL - return "[Not available]"; + os << "[Not available]"; } /// textual representation of the "not after" field of the broken certificate -const char * -Security::ErrorDetail::notafter() const +void +Security::ErrorDetail::notafter(std::ostream &os) const { #if USE_OPENSSL if (broken_cert.get()) { if (const auto tm = X509_getm_notAfter(broken_cert.get())) { + // XXX: Reduce code duplication. static char tmpBuffer[256]; // A temporary buffer Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer)); - return tmpBuffer; + os << tmpBuffer; + return; } } #endif // USE_OPENSSL - return "[Not available]"; + os << "[Not available]"; } /// textual representation of error_no -const char * -Security::ErrorDetail::err_code() const +void +Security::ErrorDetail::err_code(std::ostream &os) const { #if USE_OPENSSL // try detailEntry first because it is faster - if (const char *err = detailEntry.name.termedBuf()) - return err; + if (detailEntry) { + os << detailEntry->name; + return; + } #endif - - return ErrorNameFromCode(error_no); + os << ErrorNameFromCode(error_no); } /// short description of error_no -const char * -Security::ErrorDetail::err_descr() const +void +Security::ErrorDetail::err_descr(std::ostream &os) const { - if (!error_no) - return "[No Error]"; + if (!error_no) { + os << "[No Error]"; + return; + } + #if USE_OPENSSL - if (const char *err = detailEntry.descr.termedBuf()) - return err; + if (detailEntry) { + os << detailEntry->descr; + return; + } #endif - return "[Not available]"; + + os << "[Not available]"; } /// textual representation of lib_error_no -const char * -Security::ErrorDetail::err_lib_error() const +void +Security::ErrorDetail::err_lib_error(std::ostream &os) const { if (errReason.size() > 0) - return errReason.termedBuf(); + os << errReason; else if (lib_error_no) - return ErrorString(lib_error_no); + os << ErrorString(lib_error_no); else - return "[No Error]"; - return "[Not available]"; + os << "[No Error]"; } /** @@ -714,9 +749,9 @@ Security::ErrorDetail::err_lib_error() const \retval 0 for unsupported codes */ size_t -Security::ErrorDetail::convert(const char *code, const char **value) const +Security::ErrorDetail::convert(const char * const code, std::ostream &os) const { - typedef const char *(ErrorDetail::*PartDescriber)() const; + using PartDescriber = void (ErrorDetail::*)(std::ostream &os) const; static const std::map PartDescriberByCode = { {"ssl_subject", &ErrorDetail::subject}, {"ssl_ca_name", &ErrorDetail::ca_name}, @@ -728,17 +763,18 @@ Security::ErrorDetail::convert(const char *code, const char **value) const {"ssl_lib_error", &ErrorDetail::err_lib_error} }; + // We can refactor the map to find matches without looping, but that + // requires a "starts with" comparison function -- `code` length is unknown. for (const auto &pair: PartDescriberByCode) { const auto len = strlen(pair.first); if (strncmp(code, pair.first, len) == 0) { const auto method = pair.second; - *value = (this->*method)(); + (this->*method)(os); return len; } } // TODO: Support logformat %codes. - *value = ""; // unused with zero return return 0; } diff --git a/src/security/ErrorDetail.h b/src/security/ErrorDetail.h index a400be4bd92..cc5389fa06e 100644 --- a/src/security/ErrorDetail.h +++ b/src/security/ErrorDetail.h @@ -19,6 +19,10 @@ #include "ssl/ErrorDetailManager.h" #endif +#if USE_OPENSSL +#include +#endif + namespace Security { /// Details a TLS-related error. Two kinds of errors can be detailed: @@ -80,16 +84,17 @@ class ErrorDetail: public ::ErrorDetail private: ErrorDetail(ErrorCode err, int aSysErrorNo); + // TODO: Rename to printCamelCaseDetailName()? /* methods for formatting error details using admin-configurable %codes */ - const char *subject() const; - const char *ca_name() const; - const char *cn() const; - const char *notbefore() const; - const char *notafter() const; - const char *err_code() const; - const char *err_descr() const; - const char *err_lib_error() const; - size_t convert(const char *code, const char **value) const; + void subject(std::ostream &os) const; + void ca_name(std::ostream &os) const; + void cn(std::ostream &os) const; + void notbefore(std::ostream &os) const; + void notafter(std::ostream &os) const; + void err_code(std::ostream &os) const; + void err_descr(std::ostream &os) const; + void err_lib_error(std::ostream &os) const; + size_t convert(const char *code, std::ostream &os) const; CertPointer peer_cert; ///< A pointer to the peer certificate CertPointer broken_cert; ///< A pointer to the broken certificate (peer or intermediate) @@ -111,7 +116,7 @@ class ErrorDetail: public ::ErrorDetail int ioErrorNo = 0; using ErrorDetailEntry = Ssl::ErrorDetailEntry; - mutable ErrorDetailEntry detailEntry; + mutable std::optional detailEntry; #else // other TLS libraries do not use custom ErrorDetail members #endif diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc index 542ee246ea6..f441d57cc43 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -9,6 +9,7 @@ #include "squid.h" #include "errorpage.h" #include "fatal.h" +#include "sbuf/SBuf.h" #include "ssl/ErrorDetail.h" #include "ssl/ErrorDetailManager.h" @@ -154,9 +155,11 @@ Ssl::ErrorIsOptional(const char *name) return false; } -const char * +std::optional Ssl::GetErrorDescr(Security::ErrorCode value) { - return ErrorDetailsManager::GetInstance().getDefaultErrorDescr(value); + if (const auto detail = ErrorDetailsManager::GetInstance().findDefaultDetail(value)) + return detail->descr; + return std::nullopt; } diff --git a/src/ssl/ErrorDetail.h b/src/ssl/ErrorDetail.h index 994846ad2b1..cb870ce4cf6 100644 --- a/src/ssl/ErrorDetail.h +++ b/src/ssl/ErrorDetail.h @@ -11,6 +11,8 @@ #include "security/ErrorDetail.h" +#include + // TODO: Remove Security::X wrappers and move the remaining configurable error // details (i.e. templates/error-details.txt) code to src/security/ErrorDetail. @@ -38,8 +40,9 @@ GetErrorName(const Security::ErrorCode code, const bool prefixRawCode = false) return Security::ErrorNameFromCode(code, prefixRawCode); } -/// A short description of the TLS error "value" -const char *GetErrorDescr(Security::ErrorCode value); +/// a short description of the given TLS error known to Squid (or, if the error +/// is unknown, nothing) +std::optional GetErrorDescr(Security::ErrorCode); /// \return true if the TLS error is optional and may not be supported by current squid version bool ErrorIsOptional(const char *name); diff --git a/src/ssl/ErrorDetailManager.cc b/src/ssl/ErrorDetailManager.cc index bfbc3d33223..e2049aff933 100644 --- a/src/ssl/ErrorDetailManager.cc +++ b/src/ssl/ErrorDetailManager.cc @@ -13,6 +13,8 @@ #include "errorpage.h" #include "http/ContentLengthInterpreter.h" #include "mime_header.h" +#include "sbuf/Stream.h" +#include "sbuf/StringConvert.h" void Ssl::errorDetailInitialize() { @@ -24,6 +26,25 @@ void Ssl::errorDetailClean() Ssl::ErrorDetailsManager::Shutdown(); } +/// ErrorDetailEntry constructor helper that extracts a quoted HTTP field value +static SBuf +SlowlyParseQuotedField(const char * const description, const HttpHeader &parser, const char * const fieldName) +{ + String fieldValue; + if (!parser.hasNamed(fieldName, strlen(fieldName), &fieldValue)) + throw TextException(ToSBuf("Missing ", description), Here()); + return Http::SlowlyParseQuotedString(description, fieldValue.termedBuf(), fieldValue.size()); +} + +Ssl::ErrorDetailEntry::ErrorDetailEntry(const SBuf &aName, const HttpHeader &fields): + name(aName), + detail(SlowlyParseQuotedField("error 'detail' field", fields, "detail")), + descr(SlowlyParseQuotedField("error 'descr' field", fields, "descr")) +{ + // TODO: Warn about and report extra/unrecognized error detail fields. + // TODO: Validate formatting %codes inside parsed quoted field values. +} + namespace Ssl { @@ -42,40 +63,11 @@ class ErrorDetailFile : public TemplateFile }// namespace Ssl /******************/ -bool -Ssl::ErrorDetailsList::getRecord(Security::ErrorCode value, ErrorDetailEntry &entry) +const Ssl::ErrorDetailEntry * +Ssl::ErrorDetailsList::findRecord(Security::ErrorCode value) const { const ErrorDetails::const_iterator it = theList.find(value); - if (it != theList.end()) { - entry.error_no = it->second.error_no; - entry.name = it->second.name; - entry.detail = it->second.detail; - entry.descr = it->second.descr; - return true; - } - return false; -} - -const char * -Ssl::ErrorDetailsList::getErrorDescr(Security::ErrorCode value) -{ - const ErrorDetails::const_iterator it = theList.find(value); - if (it != theList.end()) { - return it->second.descr.termedBuf(); - } - - return nullptr; -} - -const char * -Ssl::ErrorDetailsList::getErrorDetail(Security::ErrorCode value) -{ - const ErrorDetails::const_iterator it = theList.find(value); - if (it != theList.end()) { - return it->second.detail.termedBuf(); - } - - return nullptr; + return it != theList.end() ? &it->second : nullptr; } Ssl::ErrorDetailsManager *Ssl::ErrorDetailsManager::TheDetailsManager = nullptr; @@ -102,7 +94,8 @@ Ssl::ErrorDetailsManager::ErrorDetailsManager() detailTmpl.loadDefault(); } -Ssl::ErrorDetailsList::Pointer Ssl::ErrorDetailsManager::getCachedDetails(const char *lang) +Ssl::ErrorDetailsList::Pointer +Ssl::ErrorDetailsManager::getCachedDetails(const char * const lang) const { Cache::iterator it; it = cache.find(lang); @@ -114,7 +107,8 @@ Ssl::ErrorDetailsList::Pointer Ssl::ErrorDetailsManager::getCachedDetails(const return nullptr; } -void Ssl::ErrorDetailsManager::cacheDetails(ErrorDetailsList::Pointer &errorDetails) +void +Ssl::ErrorDetailsManager::cacheDetails(const ErrorDetailsList::Pointer &errorDetails) const { const char *lang = errorDetails->errLanguage.termedBuf(); assert(lang); @@ -122,8 +116,8 @@ void Ssl::ErrorDetailsManager::cacheDetails(ErrorDetailsList::Pointer &errorDeta cache[lang] = errorDetails; } -bool -Ssl::ErrorDetailsManager::getErrorDetail(Security::ErrorCode value, const HttpRequest::Pointer &request, ErrorDetailEntry &entry) +const Ssl::ErrorDetailEntry * +Ssl::ErrorDetailsManager::findDetail(const Security::ErrorCode value, const HttpRequest::Pointer &request) const { #if USE_ERR_LOCALES String hdr; @@ -149,32 +143,21 @@ Ssl::ErrorDetailsManager::getErrorDetail(Security::ErrorCode value, const HttpRe } } - if (errDetails != nullptr && errDetails->getRecord(value, entry)) - return true; + assert(errDetails); + if (const auto entry = errDetails->findRecord(value)) + return entry; } #else (void)request; #endif - // else try the default - if (theDefaultErrorDetails->getRecord(value, entry)) { - debugs(83, 8, "Found default details record for error: " << GetErrorName(value)); - return true; - } - - return false; + return findDefaultDetail(value); } -const char * -Ssl::ErrorDetailsManager::getDefaultErrorDescr(Security::ErrorCode value) +const Ssl::ErrorDetailEntry * +Ssl::ErrorDetailsManager::findDefaultDetail(const Security::ErrorCode value) const { - return theDefaultErrorDetails->getErrorDescr(value); -} - -const char * -Ssl::ErrorDetailsManager::getDefaultErrorDetail(Security::ErrorCode value) -{ - return theDefaultErrorDetails->getErrorDetail(value); + return theDefaultErrorDetails->findRecord(value); } // Use HttpHeaders parser to parse error-details.txt files @@ -226,22 +209,19 @@ Ssl::ErrorDetailFile::parse() Security::ErrorCode ssl_error = Ssl::GetErrorCode(errorName.termedBuf()); if (ssl_error != SSL_ERROR_NONE) { - if (theDetails->getErrorDetail(ssl_error)) { + if (theDetails->findRecord(ssl_error)) { debugs(83, DBG_IMPORTANT, "WARNING: duplicate entry: " << errorName); return false; } - ErrorDetailEntry &entry = theDetails->theList[ssl_error]; - entry.error_no = ssl_error; - entry.name = errorName; - String tmp = parser.getByName("detail"); - const int detailsParseOk = httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.detail); - tmp = parser.getByName("descr"); - const int descrParseOk = httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.descr); - // TODO: Validate "descr" and "detail" field values. - - if (!detailsParseOk || !descrParseOk) { - debugs(83, DBG_IMPORTANT, "WARNING: missing important field for detail error: " << errorName); + try { + theDetails->theList.try_emplace(ssl_error, StringToSBuf(errorName), parser); + } + catch (...) { + // TODO: Reject the whole file on this and surrounding problems instead of + // keeping/using just the previously parsed entries while telling the admin + // that we "failed to find or read error text file error-details.txt". + debugs(83, DBG_IMPORTANT, "ERROR: Ignoring bad " << errorName << " detail entry: " << CurrentException); return false; } diff --git a/src/ssl/ErrorDetailManager.h b/src/ssl/ErrorDetailManager.h index a712858728a..efef66d38c7 100644 --- a/src/ssl/ErrorDetailManager.h +++ b/src/ssl/ErrorDetailManager.h @@ -11,6 +11,7 @@ #include "base/RefCount.h" #include "HttpRequest.h" +#include "sbuf/SBuf.h" #include "SquidString.h" #include "ssl/support.h" @@ -25,10 +26,12 @@ namespace Ssl class ErrorDetailEntry { public: - Security::ErrorCode error_no = 0; ///< TLS error; \see Security::ErrorCode - String name; ///< a name for the error - String detail; ///< for error page %D macro expansion; may contain macros - String descr; ///< short error description (for use in debug messages or error pages) + /// extracts quoted detail and descr fields from the given header + ErrorDetailEntry(const SBuf &aName, const HttpHeader &); + + SBuf name; ///< a name for the error + SBuf detail; ///< for error page %D macro expansion; may contain macros + SBuf descr; ///< short error description (for use in debug messages or error pages) }; /** @@ -39,13 +42,10 @@ class ErrorDetailsList : public RefCountable { public: typedef RefCount Pointer; - /** - * Retrieves the error details for a given error to "entry" object - * \return true on success, false otherwise - */ - bool getRecord(Security::ErrorCode value, ErrorDetailEntry &entry); - const char *getErrorDescr(Security::ErrorCode value); ///< an error description for an error if exist in list. - const char *getErrorDetail(Security::ErrorCode value); ///< an error details for an error if exist in list. + + /// looks up metadata details for a given error (or nil); returned pointer + /// is invalidated by any non-constant operation on the list object + const ErrorDetailEntry *findRecord(Security::ErrorCode) const; String errLanguage; ///< The language of the error-details.txt template, if any typedef std::map ErrorDetails; @@ -70,21 +70,22 @@ class ErrorDetailsManager * the default error details. * \param value the error code * \param request the current HTTP request. - * \param entry where to store error details - * \return true on success, false otherwise */ - bool getErrorDetail(Security::ErrorCode value, const HttpRequest::Pointer &request, ErrorDetailEntry &entry); - const char *getDefaultErrorDescr(Security::ErrorCode value); ///< the default error description for a given error - const char *getDefaultErrorDetail(Security::ErrorCode value); ///< the default error details for a given error + const ErrorDetailEntry *findDetail(Security::ErrorCode value, const HttpRequest::Pointer &request) const; + + /// Default error details for the given TLS error known to Squid (or, if the + /// error is unknown, nil). Use findDetail() instead when the error is tied + /// to a specific request. + const ErrorDetailEntry *findDefaultDetail(Security::ErrorCode) const; private: /// Return cached error details list for a given language if exist - ErrorDetailsList::Pointer getCachedDetails(const char *lang); + ErrorDetailsList::Pointer getCachedDetails(const char *lang) const; /// cache the given error details list. - void cacheDetails(ErrorDetailsList::Pointer &errorDetails); + void cacheDetails(const ErrorDetailsList::Pointer &errorDetails) const; typedef std::map Cache; - Cache cache; ///< the error details list cache + mutable Cache cache; ///< the error details list cache ErrorDetailsList::Pointer theDefaultErrorDetails; ///< the default error details list /// An instance of ErrorDetailsManager to be used by squid (ssl/ErrorDetails.*) diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 89411e4720f..753d491e6c0 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -340,8 +340,8 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) } else // remember another error number errs->push_back_unique(Security::CertError(error_no, broken_cert, depth)); - if (const char *err_descr = Ssl::GetErrorDescr(error_no)) - debugs(83, 5, err_descr << ": " << *peer_cert); + if (const auto description = Ssl::GetErrorDescr(error_no)) + debugs(83, 5, *description << ": " << *peer_cert); else debugs(83, DBG_IMPORTANT, "ERROR: SSL unknown certificate error " << error_no << " in " << *peer_cert); diff --git a/src/tests/stub_HttpHeader.cc b/src/tests/stub_HttpHeader.cc index ad8b184508d..fa9fad48b8b 100644 --- a/src/tests/stub_HttpHeader.cc +++ b/src/tests/stub_HttpHeader.cc @@ -83,6 +83,7 @@ bool HttpHeader::Isolate(const char **, size_t, const char **, const char **) ST bool HttpHeader::needUpdate(const HttpHeader *) const STUB_RETVAL(false) bool HttpHeader::skipUpdateHeader(const Http::HdrType) const STUB_RETVAL(false) int httpHeaderParseQuotedString(const char *, const int, String *) STUB_RETVAL(-1) +SBuf Http::SlowlyParseQuotedString(const char *, const char *, size_t) STUB_RETVAL(SBuf()) SBuf httpHeaderQuoteString(const char *) STUB_RETVAL(SBuf()) void httpHeaderCalcMask(HttpHeaderMask *, Http::HdrType [], size_t) STUB void httpHeaderInitModule() STUB