Add error replies to bluez peripheral adaptor functions

These are Qt functions that BlueZ invokes over DBus, and in some
circumstances an error reply is needed.

Task-number: QTBUG-107511
Change-Id: If325621d12600990e86fe07681aabfba8adc9056
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

6 files changed, 108 insertions, 36 deletions

diff --git a/src/bluetooth/bluez/bluezperipheralobjects.cpp b/src/bluetooth/bluez/bluezperipheralobjects.cpp
index 877b17ca..5c45da57 100644
--- a/src/bluetooth/bluez/bluezperipheralobjects.cpp
+++ b/src/bluetooth/bluez/bluezperipheralobjects.cpp
@@ -17,13 +17,18 @@ Q_DECLARE_LOGGING_CATEGORY(QT_BT_BLUEZ)
 
 using namespace Qt::StringLiterals;
 
-static constexpr QLatin1String characteristicPathTemplate{"%1/char%2"};
-static constexpr QLatin1String descriptorPathTemplate{"%1/desc%2"};
-static constexpr QLatin1String servicePathTemplate{"%1/service%2"};
+static constexpr auto characteristicPathTemplate{"%1/char%2"_L1};
+static constexpr auto descriptorPathTemplate{"%1/desc%2"_L1};
+static constexpr auto servicePathTemplate{"%1/service%2"_L1};
+
+static constexpr auto bluezServiceInterface{"org.bluez.GattService1"_L1};
+static constexpr auto bluezCharacteristicInterface{"org.bluez.GattCharacteristic1"_L1};
+static constexpr auto bluezDescriptorInterface{"org.bluez.GattDescriptor1"_L1};
+
+static constexpr auto bluezErrorInvalidValueLength("org.bluez.Error.InvalidValueLength"_L1);
+// Bluetooth Core v5.3, 3.2.9, Vol 3, Part F
+static constexpr int maximumAttributeLength{512};
 
-static constexpr QLatin1String bluezServiceInterface("org.bluez.GattService1");
-static constexpr QLatin1String bluezCharacteristicInterface("org.bluez.GattCharacteristic1");
-static constexpr QLatin1String bluezDescriptorInterface("org.bluez.GattDescriptor1");
 
 QtBluezPeripheralGattObject::QtBluezPeripheralGattObject(const QString& objectPath,
            const QString& uuid, QLowEnergyHandle handle, QObject* parent)
@@ -77,9 +82,15 @@ QtBluezPeripheralDescriptor::QtBluezPeripheralDescriptor(
                  descriptorData.uuid().toString(QUuid::WithoutBraces), handle, parent),
       m_adaptor(new OrgBluezGattDescriptor1Adaptor(this)),
       m_characteristicPath(characteristicPath),
-      m_value(descriptorData.value()),
       m_characteristicHandle(characteristicHandle)
 {
+    if (descriptorData.value().size() > maximumAttributeLength) {
+        qCWarning(QT_BT_BLUEZ) << "Descriptor value is too large, cropping it to"
+                               << maximumAttributeLength;
+        m_value = descriptorData.value().sliced(0, maximumAttributeLength);
+    } else {
+        m_value = descriptorData.value();
+    }
     initializeFlags(descriptorData);
 }
 
@@ -97,7 +108,7 @@ InterfaceList QtBluezPeripheralDescriptor::properties() const
 
 // org.bluez.GattDescriptor1
 // This function is invoked when remote device reads the value
-QByteArray QtBluezPeripheralDescriptor::ReadValue(const QVariantMap &options)
+QByteArray QtBluezPeripheralDescriptor::ReadValue(const QVariantMap &options, QString& error)
 {
     accessEvent(options);
     // Offset is set by Bluez when the value size is more than MTU size.
@@ -107,6 +118,12 @@ QByteArray QtBluezPeripheralDescriptor::ReadValue(const QVariantMap &options)
     const quint16 offset = options.value("offset"_L1).toUInt();
     const quint16 mtu = options.value("mtu"_L1).toUInt();
 
+    if (offset > m_value.length() - 1) {
+        qCWarning(QT_BT_BLUEZ) << "Invalid offset" << offset << ", value len:" << m_value.length();
+        error = bluezErrorInvalidValueLength;
+        return {};
+    }
+
     if (offset > 0)
         return m_value.mid(offset, mtu);
     else
@@ -115,17 +132,27 @@ QByteArray QtBluezPeripheralDescriptor::ReadValue(const QVariantMap &options)
 
 // org.bluez.GattDescriptor1
 // This function is invoked when remote device writes a value
-void QtBluezPeripheralDescriptor::WriteValue(const QByteArray &value, const QVariantMap &options)
+QString QtBluezPeripheralDescriptor::WriteValue(const QByteArray &value,
+                                                const QVariantMap &options)
 {
     accessEvent(options);
 
+    if (value.size() > maximumAttributeLength) {
+        qCWarning(QT_BT_BLUEZ) << "Descriptor value is too large:" << value.size();
+        return bluezErrorInvalidValueLength;
+    }
     m_value = value;
     emit valueUpdatedByRemote(m_characteristicHandle, handle, value);
+    return {};
 }
 
 // This function is called when the value has been updated locally (server-side)
 bool QtBluezPeripheralDescriptor::localValueUpdate(const QByteArray& value)
 {
+    if (value.size() > maximumAttributeLength) {
+        qCWarning(QT_BT_BLUEZ) << "Descriptor value is too large:" << value.size();
+        return false;
+    }
     m_value = value;
     return true;
 }
@@ -165,8 +192,10 @@ QtBluezPeripheralCharacteristic::QtBluezPeripheralCharacteristic(
                  characteristicData.uuid().toString(QUuid::WithoutBraces), handle, parent),
       m_adaptor(new OrgBluezGattCharacteristic1Adaptor(this)),
       m_servicePath(servicePath),
-      m_minimumValueLength(characteristicData.minimumValueLength()),
-      m_maximumValueLength(characteristicData.maximumValueLength())
+      m_minimumValueLength(std::min(characteristicData.minimumValueLength(),
+                                    maximumAttributeLength)),
+      m_maximumValueLength(std::min(characteristicData.maximumValueLength(),
+                                    maximumAttributeLength))
 {
     initializeFlags(characteristicData);
     initializeValue(characteristicData.value());
@@ -186,7 +215,7 @@ InterfaceList QtBluezPeripheralCharacteristic::properties() const
 
 // org.bluez.GattCharacteristic1
 // This function is invoked when remote device reads the value
-QByteArray QtBluezPeripheralCharacteristic::ReadValue(const QVariantMap &options)
+QByteArray QtBluezPeripheralCharacteristic::ReadValue(const QVariantMap &options, QString& error)
 {
     accessEvent(options);
     // Offset is set by Bluez when the value size is more than MTU size.
@@ -196,6 +225,12 @@ QByteArray QtBluezPeripheralCharacteristic::ReadValue(const QVariantMap &options
     const quint16 offset = options.value("offset"_L1).toUInt();
     const quint16 mtu = options.value("mtu"_L1).toUInt();
 
+    if (offset > m_value.length() - 1) {
+        qCWarning(QT_BT_BLUEZ) << "Invalid offset" << offset << ", value len:" << m_value.length();
+        error = bluezErrorInvalidValueLength;
+        return {};
+    }
+
     if (offset > 0)
         return m_value.mid(offset, mtu);
     else
@@ -204,7 +239,8 @@ QByteArray QtBluezPeripheralCharacteristic::ReadValue(const QVariantMap &options
 
 // org.bluez.GattCharacteristic1
 // This function is invoked when remote device writes a value
-void QtBluezPeripheralCharacteristic::WriteValue(const QByteArray &value, const QVariantMap &options)
+QString QtBluezPeripheralCharacteristic::WriteValue(const QByteArray &value,
+                                                    const QVariantMap &options)
 {
     accessEvent(options);
 
@@ -212,10 +248,11 @@ void QtBluezPeripheralCharacteristic::WriteValue(const QByteArray &value, const
         qCWarning(QT_BT_BLUEZ) << "Characteristic value has invalid length" << value.size()
                                << "min:" << m_minimumValueLength
                                << "max:" << m_maximumValueLength;
-        return;
+        return bluezErrorInvalidValueLength;
     }
     m_value = value;
     emit valueUpdatedByRemote(handle, value);
+    return {};
 }
 
 // This function is called when the value has been updated locally (server-side)
diff --git a/src/bluetooth/bluez/bluezperipheralobjects_p.h b/src/bluetooth/bluez/bluezperipheralobjects_p.h
index 69187b9c..dbc9c4eb 100644
--- a/src/bluetooth/bluez/bluezperipheralobjects_p.h
+++ b/src/bluetooth/bluez/bluezperipheralobjects_p.h
@@ -49,7 +49,7 @@ public:
 public:
     // DBus object path
     QString objectPath;
-    // UUID or the gatt object
+    // UUID of the gatt object
     QString uuid;
     // QtBluetooth internal handle and reference to the application
     // to read and write values towards the Qt API
@@ -80,12 +80,12 @@ public:
     InterfaceList properties() const final;
 
     // org.bluez.GattDescriptor1
-    // This function is invoked when remote device reads the value
-    Q_INVOKABLE QByteArray ReadValue(const QVariantMap &options);
+    // This function is invoked when remote device reads the value. Sets error if any
+    Q_INVOKABLE QByteArray ReadValue(const QVariantMap &options, QString &error);
 
     // org.bluez.GattDescriptor1
-    // This function is invoked when remote device writes a value
-    Q_INVOKABLE void WriteValue(const QByteArray &value, const QVariantMap &options);
+    // This function is invoked when remote device writes a value. Returns Bluez DBus error if any
+    Q_INVOKABLE QString WriteValue(const QByteArray &value, const QVariantMap &options);
 
     // Call this function when value has been updated locally (server/user application side)
     bool localValueUpdate(const QByteArray& value);
@@ -117,12 +117,12 @@ public:
     InterfaceList properties() const final;
 
     // org.bluez.GattCharacteristic1
-    // This function is invoked when remote device reads the value
-    Q_INVOKABLE QByteArray ReadValue(const QVariantMap &options);
+    // This function is invoked when remote device reads the value. Sets error if any
+    Q_INVOKABLE QByteArray ReadValue(const QVariantMap &options, QString& error);
 
     // org.bluez.GattCharacteristic1
-    // This function is invoked when remote device writes a value
-    Q_INVOKABLE void WriteValue(const QByteArray &value, const QVariantMap &options);
+    // This function is invoked when remote device writes a value. Returns Bluez DBus error if any
+    Q_INVOKABLE QString WriteValue(const QByteArray &value, const QVariantMap &options);
 
     // org.bluez.GattCharacteristic1
     // These are called when remote client enables or disables NTF/IND
diff --git a/src/bluetooth/bluez/gattcharacteristic1adaptor.cpp b/src/bluetooth/bluez/gattcharacteristic1adaptor.cpp
index 7e060197..d6eb0a1c 100644
--- a/src/bluetooth/bluez/gattcharacteristic1adaptor.cpp
+++ b/src/bluetooth/bluez/gattcharacteristic1adaptor.cpp
@@ -63,11 +63,19 @@ QByteArray OrgBluezGattCharacteristic1Adaptor::value() const
     return qvariant_cast< QByteArray >(parent()->property("Value"));
 }
 
-QByteArray OrgBluezGattCharacteristic1Adaptor::ReadValue(const QVariantMap &options)
+QByteArray OrgBluezGattCharacteristic1Adaptor::ReadValue(const QVariantMap &options,
+                                                         const QDBusMessage& msg)
 {
     // handle method call org.bluez.GattCharacteristic1.ReadValue
     QByteArray value;
-    QMetaObject::invokeMethod(parent(), "ReadValue", Q_RETURN_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
+    QString error;
+    QMetaObject::invokeMethod(parent(), "ReadValue", Q_RETURN_ARG(QByteArray, value),
+                              Q_ARG(QVariantMap, options), Q_ARG(QString&, error));
+    if (!error.isEmpty()) {
+        // Reply with error if needed
+        auto reply = msg.createErrorReply(error, {});
+        QDBusConnection::systemBus().send(reply);
+    }
     return value;
 }
 
@@ -83,8 +91,18 @@ void OrgBluezGattCharacteristic1Adaptor::StopNotify()
     QMetaObject::invokeMethod(parent(), "StopNotify");
 }
 
-void OrgBluezGattCharacteristic1Adaptor::WriteValue(const QByteArray &value, const QVariantMap &options)
+void OrgBluezGattCharacteristic1Adaptor::WriteValue(const QByteArray &value,
+                                                    const QVariantMap &options,
+                                                    const QDBusMessage& msg)
 {
     // handle method call org.bluez.GattCharacteristic1.WriteValue
-    QMetaObject::invokeMethod(parent(), "WriteValue", Q_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
+    QString error;
+    QMetaObject::invokeMethod(parent(), "WriteValue", Q_RETURN_ARG(QString, error),
+                              Q_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
+
+    if (!error.isEmpty()) {
+        // Reply with error if needed
+        auto reply = msg.createErrorReply(error, {});
+        QDBusConnection::systemBus().send(reply);
+    }
 }
diff --git a/src/bluetooth/bluez/gattcharacteristic1adaptor_p.h b/src/bluetooth/bluez/gattcharacteristic1adaptor_p.h
index d22208b0..7f80e8f0 100644
--- a/src/bluetooth/bluez/gattcharacteristic1adaptor_p.h
+++ b/src/bluetooth/bluez/gattcharacteristic1adaptor_p.h
@@ -77,10 +77,10 @@ public: // PROPERTIES
     QByteArray value() const;
 
 public Q_SLOTS: // METHODS
-    QByteArray ReadValue(const QVariantMap &options);
+    QByteArray ReadValue(const QVariantMap &options, const QDBusMessage &msg);
     void StartNotify();
     void StopNotify();
-    void WriteValue(const QByteArray &value, const QVariantMap &options);
+    void WriteValue(const QByteArray &value, const QVariantMap &options, const QDBusMessage& msg);
 Q_SIGNALS: // SIGNALS
 };
 
diff --git a/src/bluetooth/bluez/gattdescriptor1adaptor.cpp b/src/bluetooth/bluez/gattdescriptor1adaptor.cpp
index 6f2ea125..b7177384 100644
--- a/src/bluetooth/bluez/gattdescriptor1adaptor.cpp
+++ b/src/bluetooth/bluez/gattdescriptor1adaptor.cpp
@@ -51,17 +51,34 @@ QByteArray OrgBluezGattDescriptor1Adaptor::value() const
     return qvariant_cast< QByteArray >(parent()->property("Value"));
 }
 
-QByteArray OrgBluezGattDescriptor1Adaptor::ReadValue(const QVariantMap &options)
+QByteArray OrgBluezGattDescriptor1Adaptor::ReadValue(const QVariantMap &options,
+                                                     const QDBusMessage& msg)
 {
     // handle method call org.bluez.GattDescriptor1.ReadValue
     QByteArray value;
-    QMetaObject::invokeMethod(parent(), "ReadValue", Q_RETURN_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
+    QString error;
+    QMetaObject::invokeMethod(parent(), "ReadValue", Q_RETURN_ARG(QByteArray, value),
+                              Q_ARG(QVariantMap, options), Q_ARG(QString&, error));
+    if (!error.isEmpty()) {
+        // Reply with error if needed
+        auto reply = msg.createErrorReply(error, {});
+        QDBusConnection::systemBus().send(reply);
+    }
     return value;
 }
 
-void OrgBluezGattDescriptor1Adaptor::WriteValue(const QByteArray &value, const QVariantMap &options)
+void OrgBluezGattDescriptor1Adaptor::WriteValue(const QByteArray &value,
+                                                const QVariantMap &options,
+                                                const QDBusMessage& msg)
 {
     // handle method call org.bluez.GattDescriptor1.WriteValue
-    QMetaObject::invokeMethod(parent(), "WriteValue", Q_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
-}
+    QString error;
+    QMetaObject::invokeMethod(parent(), "WriteValue", Q_RETURN_ARG(QString, error),
+                              Q_ARG(QByteArray, value), Q_ARG(QVariantMap, options));
 
+    if (!error.isEmpty()) {
+        // Reply with error if needed
+        auto reply = msg.createErrorReply(error, {});
+        QDBusConnection::systemBus().send(reply);
+    }
+}
diff --git a/src/bluetooth/bluez/gattdescriptor1adaptor_p.h b/src/bluetooth/bluez/gattdescriptor1adaptor_p.h
index 945392e9..a7eacb99 100644
--- a/src/bluetooth/bluez/gattdescriptor1adaptor_p.h
+++ b/src/bluetooth/bluez/gattdescriptor1adaptor_p.h
@@ -67,8 +67,8 @@ public: // PROPERTIES
     QByteArray value() const;
 
 public Q_SLOTS: // METHODS
-    QByteArray ReadValue(const QVariantMap &options);
-    void WriteValue(const QByteArray &value, const QVariantMap &options);
+    QByteArray ReadValue(const QVariantMap &options, const QDBusMessage &msg);
+    void WriteValue(const QByteArray &value, const QVariantMap &options, const QDBusMessage& msg);
 Q_SIGNALS: // SIGNALS
 };