[message] clarify partial read behavior of Read() vs ReadByte() (o…

…penthread#10719)

This commit updates and clarifies the behavior of `Message::ReadByte()`
and `Message::Read()` overloads regarding partial reads.

- `ReadByte()` will read the available bytes and return the actual
  number of bytes read if fewer bytes are available in the message
  than the requested read length. This behavior remains unchanged.
  The documentation is updated to emphasize this behavior.
- `Read()` methods return `kErrorParse` if the requested length cannot
  be read. This is the existing behavior which remains unchanged.
  Previously, `Read()` methods would still perform a partial read and
  populate the buffer/object with as many bytes that could be read,
  even in case of failure and returning `kErrorParse`. This behavior
  has been changed in this commit so the method will skip
  reading/copying bytes if the full length cannot be read. This
  aligns the documentation and behavior with how the `Read()` methods
  are used and intended to be used within the OT stack.

src/core/common/message.cpp

@@ -660,15 +660,21 @@ uint16_t Message::ReadBytes(const OffsetRange &aOffsetRange, void *aBuf) const
 
 Error Message::Read(uint16_t aOffset, void *aBuf, uint16_t aLength) const
 {
-    return (ReadBytes(aOffset, aBuf, aLength) == aLength) ? kErrorNone : kErrorParse;
+    Error error = kErrorNone;
+
+    VerifyOrExit(aOffset + aLength <= GetLength(), error = kErrorParse);
+    ReadBytes(aOffset, aBuf, aLength);
+
+exit:
+    return error;
 }
 
 Error Message::Read(const OffsetRange &aOffsetRange, void *aBuf, uint16_t aLength) const
 {
     Error error = kErrorNone;
 
     VerifyOrExit(aOffsetRange.Contains(aLength), error = kErrorParse);
-    VerifyOrExit(ReadBytes(aOffsetRange.GetOffset(), aBuf, aLength) == aLength, error = kErrorParse);
+    error = Read(aOffsetRange.GetOffset(), aBuf, aLength);
 
 exit:
     return error;

src/core/common/message.hpp

@@ -762,6 +762,12 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads bytes from the message.
      *
+     * The provided buffer @p aBuf MUST contain at least @p aLength bytes.
+     *
+     * If there are fewer bytes available in the message than the requested @p aLength, the available bytes are read
+     * and copied into @p aBuf. This method returns the actual number of bytes successfully read from the message and
+     * written into @p aBuf.
+     *
      * @param[in]  aOffset  Byte offset within the message to begin reading.
      * @param[out] aBuf     A pointer to a data buffer to copy the read bytes into.
      * @param[in]  aLength  Number of bytes to read.
@@ -774,6 +780,10 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads bytes from the message.
      *
+     * If there are fewer bytes available in the message than the provided length in @p aOffsetRange, the available
+     * bytes are read and copied into @p aBuf. This method returns the actual number of bytes successfully read from
+     * the message and written into @p aBuf.
+     *
      * @param[in]  aOffsetRange  The offset range in the message to read bytes from.
      * @param[out] aBuf          A pointer to a data buffer to copy the read bytes into.
      *
@@ -785,9 +795,6 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads a given number of bytes from the message.
      *
-     * If there are fewer bytes available in the message than the requested read length, the available bytes will be
-     * read and copied into @p aBuf. In this case `kErrorParse` will be returned.
-     *
      * @param[in]  aOffset  Byte offset within the message to begin reading.
      * @param[out] aBuf     A pointer to a data buffer to copy the read bytes into.
      * @param[in]  aLength  Number of bytes to read.
@@ -801,9 +808,6 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads a given number of bytes from the message.
      *
-     * If there are fewer bytes available in the message or @p aOffsetRange than the requested @p aLength, the
-     * available bytes are read and copied into @p aBuf. In this case `kErrorParse` will be returned.
-     *
      * @param[in]  aOffsetRange  The offset range in the message to read from.
      * @param[out] aBuf          A pointer to a data buffer to copy the read bytes into.
      * @param[in]  aLength       Number of bytes to read.
@@ -817,10 +821,6 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads an object from the message.
      *
-     * If there are fewer bytes available in the message than the requested object size, the available bytes will be
-     * read and copied into @p aObject (@p aObject will be read partially). In this case `kErrorParse` will
-     * be returned.
-     *
      * @tparam     ObjectType   The object type to read from the message.
      *
      * @param[in]  aOffset      Byte offset within the message to begin reading.
@@ -840,10 +840,6 @@ class Message : public otMessage, public Buffer, public GetProvider<Message>
     /**
      * Reads an object from the message.
      *
-     * If there are fewer bytes available in the message or @p aOffsetRange than the requested object size, the
-     * available bytes will be read and copied into @p aObject (@p aObject will be read partially). In this case
-     * `kErrorParse` will be returned.
-     *
      * @tparam     ObjectType   The object type to read from the message.
      *
      * @param[in]  aOffsetRange  The offset range in the message to read from.

tests/unit/test_message.cpp

@@ -157,6 +157,32 @@ void TestMessage(void)
             VerifyOrQuit(!message->CompareBytes(offset, readBuffer, length));
             VerifyOrQuit(message->CompareBytes(offset, readBuffer, readLength));
         }
+
+        // Verify `Read()` behavior when requested read length goes beyond available bytes in the message.
+
+        for (uint16_t length = kMaxSize - offset + 1; length <= kMaxSize + 1; length++)
+        {
+            Error error;
+
+            memset(readBuffer, 0, sizeof(readBuffer));
+
+            error = message->Read(offset, readBuffer, length);
+
+            if (length < kMaxSize - offset)
+            {
+                uint16_t readLength = kMaxSize - offset;
+
+                SuccessOrQuit(error);
+                VerifyOrQuit(memcmp(readBuffer, &writeBuffer[offset], readLength) == 0);
+                VerifyOrQuit(memcmp(&readBuffer[readLength], zeroBuffer, kMaxSize - readLength) == 0,
+                             "read after length");
+            }
+            else
+            {
+                VerifyOrQuit(error == kErrorParse);
+                VerifyOrQuit(memcmp(readBuffer, zeroBuffer, sizeof(readBuffer)) == 0, "Read() updated buffer on error");
+            }
+        }
     }
 
     VerifyOrQuit(message->GetLength() == kMaxSize);