From 62644650c09e1ef6b9c2420bc91d83a38f6a34e5 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Sun, 5 Jun 2022 02:02:45 +0200 Subject: [PATCH] Some fixes in Coap * Fixed an indexOutOfRange warning when creating coap requests because the first addOption() call was calling m_options.insert(1) on an empty list. * Old code was appending multiple UDP datagrams to a single big Coap message, however, Coap is specified to only send s single datagram per message. The datagram length specifies the payload size. * some boolean member variables weren't initialized which resulted in occational wrong flags. * Parsing had issues with determining the option length in some occations and also would crash when receiving coap messages without any options or payload. To get rid of the complex and erraneous index calculations, the entire package parsing is now using a DataStream. (This makes it work with Shelly devices) --- libnymea/coap/coap.cpp | 11 ++-- libnymea/coap/coappdu.cpp | 125 ++++++++++++++++++++++++-------------- libnymea/coap/coappdu.h | 4 ++ libnymea/coap/coapreply.h | 4 +- 4 files changed, 89 insertions(+), 55 deletions(-) diff --git a/libnymea/coap/coap.cpp b/libnymea/coap/coap.cpp index b30f37ec..404e434c 100644 --- a/libnymea/coap/coap.cpp +++ b/libnymea/coap/coap.cpp @@ -801,16 +801,15 @@ void Coap::hostLookupFinished(const QHostInfo &hostInfo) void Coap::onReadyRead() { QHostAddress hostAddress; - QByteArray data; quint16 port; while (m_socket->hasPendingDatagrams()) { - data.resize(m_socket->pendingDatagramSize()); - m_socket->readDatagram(data.data(), data.size(), &hostAddress, &port); + QByteArray datagram(m_socket->pendingDatagramSize(), 0); + m_socket->readDatagram(datagram.data(), datagram.size(), &hostAddress, &port); + qCDebug(dcCoap()) << "Datagram received from:" << hostAddress << ":" << datagram; + CoapPdu pdu(datagram); + processResponse(pdu, hostAddress, port); } - - CoapPdu pdu(data); - processResponse(pdu, hostAddress, port); } void Coap::onReplyTimeout() diff --git a/libnymea/coap/coappdu.cpp b/libnymea/coap/coappdu.cpp index c1f82011..d02abc90 100644 --- a/libnymea/coap/coappdu.cpp +++ b/libnymea/coap/coappdu.cpp @@ -163,6 +163,10 @@ #include #include +#include +#include + +Q_DECLARE_LOGGING_CATEGORY(dcCoap) /*! Constructs a CoapPdu with the given \a parent. */ CoapPdu::CoapPdu(QObject *parent) : @@ -370,7 +374,7 @@ void CoapPdu::addOption(const CoapOption::Option &option, const QByteArray &data CoapOption o; o.setOption(option); o.setData(data); - m_options.insert(index + 1, o); + m_options.insert(qMin(m_options.length(), index + 1), o); } /*! Returns the block of this \l{CoapPdu}. */ @@ -504,70 +508,97 @@ QByteArray CoapPdu::pack() const void CoapPdu::unpack(const QByteArray &data) { - // create a CoapPDU if (data.length() < 4) { m_error = InvalidPduSizeError; } - quint8 *rawData = (quint8 *)data.data(); - setVersion((rawData[0] & 0xc0) >> 6); - setMessageType(static_cast((rawData[0] & 0x30) >> 4)); - quint8 tokenLength = (rawData[0] & 0xf); + QDataStream stream(data); + quint8 flags; + stream >> flags; + setVersion((flags & 0xc0) >> 6); +// qCDebug(dcCoap()) << "Version:" << m_version; + + setMessageType(static_cast((flags & 0x30) >> 4)); +// qCDebug(dcCoap()) << "Message Type:" << messageType(); + + quint8 tokenLength = flags & 0x0f; +// qCDebug(dcCoap()) << "Token length:" << tokenLength; if (tokenLength > 8) { + qCWarning(dcCoap()) << "Inavalid token length" << tokenLength; m_error = InvalidTokenError; + return; } - setToken(QByteArray((const char *)rawData + 4, tokenLength)); - setStatusCode(static_cast(rawData[1])); - setMessageId((qint16)data.mid(2,2).toHex().toUInt(0,16)); + quint8 reqRspCode; + stream >> reqRspCode; + setStatusCode(static_cast(reqRspCode)); +// qCDebug(dcCoap()) << "Req/Rsp code:" << statusCode(); - // parse options - int index = 4 + tokenLength; - quint8 optionByte = rawData[index]; - quint16 delta = 0; - while (QByteArray::number(optionByte, 16) != "ff" && optionByte != 0) { - quint16 optionNumber = ((optionByte & 0xf0) >> 4); + quint16 messageId; + stream >> messageId; + setMessageId(messageId); +// qCDebug(dcCoap()) << "Message ID:" << messageId; + char tokenData[tokenLength]; + if (stream.readRawData(tokenData, tokenLength) != tokenLength) { + qCWarning(dcCoap()) << "Token data not complete."; + m_error = InvalidTokenError; + return; + } + QByteArray token(tokenData, tokenLength); + setToken(token); +// qCDebug(dcCoap()) << "Token:" << token.toHex(); - // check option delta - if (optionNumber < 13) { - delta += optionNumber; - } else if (optionNumber == 13) { - // extended 8 bit option delta - delta += (quint8)(rawData[index + 1] + 13); - index += 1; - } else if (optionNumber == 14) { - // extended 16 bit option delta - delta += ((rawData[index + 1] << 8) | rawData[index + 2]) + 269; - index += 2; - } else if (optionNumber == 15) { - m_error = InvalidOptionDeltaError; + + while (!stream.atEnd()) { + quint8 optionByte; + stream >> optionByte; +// qCDebug(dcCoap()) << "OptionByte:" << optionByte; + + if (optionByte == 0xff) { + char payloadData[65507]; // Max UDP datagram size + int payloadLength = stream.readRawData(payloadData, 65507); + if (payloadLength > 0) { + setPayload(QByteArray(payloadData, payloadLength)); + } + return; + } + + quint16 optionDelta; + optionDelta = (optionByte & 0xf0) >> 4; +// qCDebug(dcCoap()) << "Option delta:" << optionDelta; + quint16 optionLength = (optionByte & 0x0f); +// qCDebug(dcCoap()) << "Option length:" << optionLength; + + if (optionDelta == 13) { + quint8 optionDeltaExtended; + stream >> optionDeltaExtended; + optionDelta = optionDeltaExtended + 13; +// qCDebug(dcCoap()).nospace() << "Extended option delta (8 bit): " << optionDelta << " (" << optionDeltaExtended << " + 13)"; + } else if (optionDelta == 14) { + quint16 optionDeltaExtended; + stream >> optionDeltaExtended; + optionDelta = optionDeltaExtended + 269; +// qCDebug(dcCoap()).nospace() << "Extended option delta (16 bit): " << optionDelta << " (" << optionDeltaExtended << " + 269)"; } - // check option length - quint16 optionLength = (optionByte & 0xf); if (optionLength == 13) { - // extended 8 bit option length - optionLength = (quint8)(rawData[index + 1] - 13); - index += 1; + quint8 optionLengthExtended; + stream >> optionLengthExtended; + optionLength = optionLengthExtended + 13; +// qCDebug(dcCoap()).nospace() << "Extended option length (8 bit): " << optionLength << " (" << optionLengthExtended << " + 13)"; } else if (optionLength == 14) { - // extended 16 bit option delta - optionLength = ((rawData[index + 1] << 8) | rawData[index + 2]) - 269; - index += 2; - } else if (optionLength == 15) { - m_error = InvalidOptionLengthError; + quint16 optionLengthExtended; + stream >> optionLengthExtended; + optionLength = optionLengthExtended + 269; +// qCDebug(dcCoap()).nospace() << "Extended option kength (16 bit): " << optionDelta << " (" << optionLengthExtended << " + 269)"; } - QByteArray optionData = QByteArray((const char *)rawData + index + 1, optionLength); - addOption(static_cast(delta), optionData); + char optionData[optionLength]; + stream.readRawData(optionData, optionLength); +// qCDebug(dcCoap()) << "Option data:" << QByteArray(optionData, optionLength); - index += optionLength + 1; - optionByte = rawData[index]; - - if (QByteArray::number(optionByte, 16) == "ff") { - setPayload(data.right(data.length() - index - 1)); - break; - } + addOption(static_cast(optionDelta), QByteArray(optionData, optionLength)); } } diff --git a/libnymea/coap/coappdu.h b/libnymea/coap/coappdu.h index 1df14645..1d633a7a 100644 --- a/libnymea/coap/coappdu.h +++ b/libnymea/coap/coappdu.h @@ -68,6 +68,7 @@ public: Acknowledgement = 0x02, Reset = 0x03 }; + Q_ENUM(MessageType) // Methods: https://tools.ietf.org/html/rfc7252#section-5.8 // Respond codes: https://tools.ietf.org/html/rfc7252#section-12.1.2 @@ -101,6 +102,7 @@ public: GatewayTimeout = 0xa4, // 5.04 ProxyingNotSupported = 0xa5 // 5.05 }; + Q_ENUM(StatusCode) // https://tools.ietf.org/html/rfc7252#section-12.3 enum ContentType { @@ -111,6 +113,7 @@ public: ApplicationExi = 47, ApplicationJson = 50 }; + Q_ENUM(ContentType) enum Error { NoError, @@ -120,6 +123,7 @@ public: InvalidOptionLengthError, UnknownOptionError }; + Q_ENUM(Error) CoapPdu(QObject *parent = 0); CoapPdu(const QByteArray &data, QObject *parent = 0); diff --git a/libnymea/coap/coapreply.h b/libnymea/coap/coapreply.h index ac734822..0a5a92c7 100644 --- a/libnymea/coap/coapreply.h +++ b/libnymea/coap/coapreply.h @@ -132,8 +132,8 @@ private: int m_messageId; QByteArray m_messageToken; - bool m_observation; - bool m_observationEnable; + bool m_observation = false; + bool m_observationEnable = false; signals: void timeout();