From b83225fcc3c6aa5655884cbf9e7391afe38b4392 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Fri, 9 Oct 2020 17:53:53 +0200
Subject: qDebug: Avoid implicit QVariant conversion

This commit restricts operator<<(QDebug lhs, QVariant rhs) to only work
if rhs is actually of type QVariant (instead of any type convertible to
QVariant). This is especially important as
a) we check in QMetaType whether (slightly simplified) QDebug{} <<
   std::declval<T>() is valid, and if so, register a function which
   simply uses the operator.
b) In QVariant, we ask the metatype system for the contained types
   registered debug function and then use it.
If a type now does not have its own operator<< for QDebug, but is
implicitly convertible to QVariant containing itself, this would lead to
an infinite recursion, when trying to use qDebug with that type. The
registered function in a) would just convert the type to QVariant, and
then ask the QVariant to print itself.
Disallowing implicit conversions in qDebug in general was considered
(i.e. adding template<typename T> operator<<(T) = delete in QDebug ),
but discarded as it breaks too much code relying on conversions.

Fixes: QTBUG-87122
Change-Id: Ib709297670cbc6cc307efd0dfd8e5b0279df9414
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/kernel/qvariant.cpp             | 12 ++++++------
 src/corelib/kernel/qvariant.h               |  7 +++++--
 tests/auto/corelib/io/qdebug/tst_qdebug.cpp | 15 +++++++++++++++
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index ad52921cb7..05293afccc 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -2396,16 +2396,16 @@ bool QVariant::isNull() const
 }
 
 #ifndef QT_NO_DEBUG_STREAM
-QDebug operator<<(QDebug dbg, const QVariant &v)
+QDebug QVariant::qdebugHelper(QDebug dbg) const
 {
     QDebugStateSaver saver(dbg);
-    const uint typeId = v.d.typeId();
+    const uint typeId = d.typeId();
     dbg.nospace() << "QVariant(";
     if (typeId != QMetaType::UnknownType) {
-        dbg << v.d.type().name() << ", ";
-        bool streamed = v.d.type().debugStream(dbg, v.d.storage());
-        if (!streamed && v.canConvert<QString>())
-            dbg << v.toString();
+        dbg << d.type().name() << ", ";
+        bool streamed = d.type().debugStream(dbg, d.storage());
+        if (!streamed && canConvert<QString>())
+            dbg << toString();
     } else {
         dbg << "Invalid";
     }
diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index 907094d25c..9df9c53802 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -494,7 +494,11 @@ private:
     friend inline bool operator!=(const QVariant &a, const QVariant &b)
     { return !a.equals(b); }
 #ifndef QT_NO_DEBUG_STREAM
-    friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &);
+    template <typename T>
+    friend auto operator<<(const QDebug &debug, const T &variant) -> std::enable_if_t<std::is_same_v<T, QVariant>, QDebug> {
+        return  variant.qdebugHelper(debug);
+    }
+    QDebug qdebugHelper(QDebug) const;
 #endif
     template<typename T>
     friend inline T qvariant_cast(const QVariant &);
@@ -595,7 +599,6 @@ template<> inline QVariant qvariant_cast<QVariant>(const QVariant &v)
 #endif
 
 #ifndef QT_NO_DEBUG_STREAM
-Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &);
 Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant::Type);
 #endif
 
diff --git a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
index 3dbda13643..949882b578 100644
--- a/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
+++ b/tests/auto/corelib/io/qdebug/tst_qdebug.cpp
@@ -34,6 +34,7 @@
 
 #include <QtConcurrentRun>
 #include <QFutureSynchronizer>
+#include <QVariant>
 
 static_assert(QTypeTraits::has_ostream_operator_v<QDebug, int>);
 static_assert(QTypeTraits::has_ostream_operator_v<QDebug, QList<int>>);
@@ -42,6 +43,11 @@ struct NonStreamable {};
 static_assert(!QTypeTraits::has_ostream_operator_v<QDebug, NonStreamable>);
 static_assert(!QTypeTraits::has_ostream_operator_v<QDebug, QList<NonStreamable>>);
 static_assert(!QTypeTraits::has_ostream_operator_v<QDebug, QMap<int, NonStreamable>>);
+struct ConvertsToQVariant {
+    operator QVariant() {return QVariant::fromValue(*this);};
+};
+static_assert(!QTypeTraits::has_ostream_operator_v<QDebug, ConvertsToQVariant>);
+
 
 class tst_QDebug: public QObject
 {
@@ -75,6 +81,7 @@ private slots:
     void defaultMessagehandler() const;
     void threadSafety() const;
     void toString() const;
+    void noQVariantEndlessRecursion() const;
 };
 
 void tst_QDebug::assignment() const
@@ -770,6 +777,14 @@ void tst_QDebug::toString() const
     }
 }
 
+void tst_QDebug::noQVariantEndlessRecursion() const
+{
+    ConvertsToQVariant conv;
+    QVariant var = QVariant::fromValue(conv);
+    QTest::ignoreMessage(QtDebugMsg, "QVariant(ConvertsToQVariant, )");
+    qDebug() << var;
+}
+
 // Should compile: instentiation of unrelated operator<< should not cause cause compilation
 // error in QDebug operators (QTBUG-47375)
 class TestClassA {};
-- 
cgit v1.2.3