Fix printing Security::ErrorDetail (#1129)

Squid dumps RefCount pointer details instead of the intended TLS error
details like "SQUID_TLS_ERR_ACCEPT+TLS_LIB_ERR=14094418":

    ERROR: failure while accepting a TLS connection...: 0x564017165ef0*1

We overload operator "<<" for ErrorDetail::Pointer to report errors, but
the compiler picks our generic operator "<<" overload for RefCount<C>
instead. I believe this happens because the actual type of the
being-printed handshakeResult.errorDetail object (i.e.
Security::IoResult::ErrorDetailPointer) is not ErrorDetail::Pointer but
Security::ErrorDetail::Pointer; there is no overload for the latter.

This simple solution "works" but it does not solve the underlying
problem: Other ErrorDetail kids (existing and future) and other similar
class hierarchies using RefCount pointers (e.g., Http::Message) are or
will be mishandled because of missing kid-specific overloads.

To properly address this XXX, we probably should add traits/interfaces
to all RefCountable classes that want their Pointers to be printed
specially (e.g., ErrorDetail) _and_ teach our generic operator "<<"
overload for RefCount<C> to honor those. Also, an InstanceId data member
might be recognized/printed by default instead of the memory address; it
may even be added to RefCountable.

diff --git a/src/error/Detail.h b/src/error/Detail.h
index f8549d409503fed93e9fc31be68760b22c6bcc27..f0c1c55373bfe70032b861435e917eb391051a3d 100644 (file)
--- a/src/error/Detail.h
+++ b/src/error/Detail.h
@@ -40,7 +40,10 @@ ErrorDetail::Pointer MakeNamedErrorDetail(const char *name);
 /// dump the given ErrorDetail (for debugging)
 std::ostream &operator <<(std::ostream &os, const ErrorDetail &);
 
-/// dump the given ErrorDetail pointer which may be nil (for debugging)
+// XXX: Every ErrorDetail child, especially those declaring their own Pointer
+// types should overload this operator. The compiler will not find this overload
+// for child pointers. See Security::ErrorDetail overload for an example.
+/// dump the given ErrorDetail via a possibly nil pointer (for debugging)
 std::ostream &operator <<(std::ostream &os, const ErrorDetail::Pointer &);
 
 #endif /* _SQUID_SRC_ERROR_DETAIL_H */

diff --git a/src/security/ErrorDetail.h b/src/security/ErrorDetail.h
index 668c83a6ff010eaee8bd43f53b9e6653d7d6de39..6610711a04d8ac4ae7ee202318ccf862791fc95f 100644 (file)
--- a/src/security/ErrorDetail.h
+++ b/src/security/ErrorDetail.h
@@ -126,6 +126,16 @@ ErrorCode ErrorCodeFromName(const char *name);
 /// \param prefixRawCode whether to prefix raw codes with "SSL_ERR="
 const char *ErrorNameFromCode(ErrorCode err, bool prefixRawCode = false);
 
+} // namespace Security
+
+/// Dump the given Security::ErrorDetail via a possibly nil pointer (for
+/// debugging). Unfortunately, without this, compilers pick generic RefCount<T>
+/// operator "<<" overload (with T=Security::ErrorDetail) instead of the
+/// overload provided by the parent ErrorDetail class (that we call here).
+inline std::ostream &
+operator <<(std::ostream &os, const Security::ErrorDetail::Pointer &p)
+{
+    return operator <<(os, ::ErrorDetail::Pointer(p));
 }
 
 #endif

diff --git a/src/tests/stub_liberror.cc b/src/tests/stub_liberror.cc
index 51bbcb2bbaf70c7509e6c1b59f887d7b1547ba3c..0709805281250fa0309f78220d76e2ba30940374 100644 (file)
--- a/src/tests/stub_liberror.cc
+++ b/src/tests/stub_liberror.cc
@@ -10,6 +10,7 @@ const char * err_type_str[ERR_MAX] = {};
 void Error::update(const Error &) STUB_NOP
 
 std::ostream &operator <<(std::ostream &os, const Error &) STUB_RETVAL(os)
+std::ostream &operator <<(std::ostream &os, const ErrorDetail::Pointer &) STUB_RETVAL(os)
 
 ErrorDetail::Pointer MakeNamedErrorDetail(const char *) STUB_RETVAL(ErrorDetail::Pointer())