From 2c5854a7ec6c4a0430c0d04482a8b858bbecdaeb Mon Sep 17 00:00:00 2001
From: Andrew Johnson <anj@anl.gov>
Date: Tue, 21 Jul 2026 16:00:00 -0500
Subject: [PATCH] Add a max string len param to deserializeString()

Defaults to INT32_MAX if not provided, but several places in
pvAccess can set limits, e.g. MAX_CHANNEL_NAME_LENGTH, and
this is the best way to harden them against huge values.

Document that SerializeHelper::readSize() returns -1 for NULL.
---
 src/misc/pv/serializeHelper.h | 24 ++++++++----
 src/misc/serializeHelper.cpp  | 72 ++++++++++++++++-------------------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/src/misc/pv/serializeHelper.h b/src/misc/pv/serializeHelper.h
index 5ced0d6..57f74e6 100644
--- a/src/misc/pv/serializeHelper.h
+++ b/src/misc/pv/serializeHelper.h
@@ -50,7 +50,7 @@ namespace epics {
              *
              * @param[in] buffer deserialization buffer.
              * @param[in] control the DeserializableControl.
-             * @returns array size.
+             * @returns array size, or (size_t) -1.
              */
             static std::size_t readSize(ByteBuffer* buffer,
                     DeserializableControl* control);
@@ -80,20 +80,22 @@ namespace epics {
 
             /**
              * std::string deserialization helper method.
-             * TODO This method cannot return "null", but Java implementation
-             * could have serialized "null" value as well. We need to decide
-             * how to deserialize "null".
              *
              * @param[in] buffer deserialization buffer
              * @param[in] control control
+             * @param[in] max length limit
              * @returns deserialized string
              *
              * @todo This method cannot return "null", but Java implementation
-             * could have serialized "null" value as well. We need to decide
-             * how to deserialize "null".
+             * could have serialized "null" value as well. This implementation
+             * returns an empty string in that event.
+             *
+             * @since UNRELEASED Added the max argument.
+             *        Test for the DESERIALIZE_STRING_HAS_LIMIT macro to detect.
+             *        Retroactive default is 2 GiB
              */
             static std::string deserializeString(ByteBuffer* buffer,
-                    DeserializableControl* control);
+                    DeserializableControl* control, std::size_t max = 0x7fffffff);
 
         private:
             SerializeHelper() {};
@@ -112,4 +114,12 @@ namespace epics {
     }
 }
 
+/** Feature test macro for max argument
+ *
+ *  of SerializeHelper::deserializeString()
+ *
+ *  @since UNRELEASED added
+ */
+#define DESERIALIZE_STRING_HAS_LIMIT
+
 #endif /* SERIALIZEHELPER_H_ */
diff --git a/src/misc/serializeHelper.cpp b/src/misc/serializeHelper.cpp
index 822ddec..62b8196 100644
--- a/src/misc/serializeHelper.cpp
+++ b/src/misc/serializeHelper.cpp
@@ -47,12 +47,13 @@ namespace epics {
                 DeserializableControl* control) {
             control->ensureData(1);
             int8 b = buffer->getByte();
-            if(b==-1)
-                return -1;
-            else if(b==-2) {
+            if (b == -1)
+                return -1;  // NULL
+            else if (b == -2) {
                 control->ensureData(sizeof(int32));
                 int32 s = buffer->getInt();
-                if(s<0) THROW_BASE_EXCEPTION("negative size");
+                if (s < 0)
+                    THROW_BASE_EXCEPTION("negative size");
                 return s;
             }
             else
@@ -100,44 +101,37 @@ namespace epics {
         }
 
         string SerializeHelper::deserializeString(ByteBuffer* buffer,
-                DeserializableControl* control) {
+                DeserializableControl* control, std::size_t max) {
 
             std::size_t size = SerializeHelper::readSize(buffer, control);
-            if(size!=(size_t)-1)    // TODO null strings check, to be removed in the future
-            {
-                if (buffer->getRemaining()>=size)
-                {
-                    // entire string is in buffer, simply create a string out of it (copy)
-                    std::size_t pos = buffer->getPosition();
-                    string str(buffer->getBuffer()+pos, size);
-                    buffer->setPosition(pos+size);
-                    return str;
-                }
-                else
-                {
-                    string str;
-                    str.reserve(size);
-                    try {
-                        std::size_t i = 0;
-                        while(true) {
-                            std::size_t toRead = min(size-i, buffer->getRemaining());
-                            std::size_t pos = buffer->getPosition();
-                            str.append(buffer->getBuffer()+pos, toRead);
-                            buffer->setPosition(pos+toRead);
-                            i += toRead;
-                            if(i<size)
-                                control->ensureData(1); // at least one
-                            else
-                                break;
-                        }
-                        return str;
-                    } catch(...) {
-                        throw;
-                    }
-                }
+            if (size == (std::size_t) -1)
+                return std::string();   // NULL from old implementations
+
+            if (size > max)
+                THROW_BASE_EXCEPTION("string too long");
+
+            if (buffer->getRemaining() >= size) { // All present
+                std::size_t pos = buffer->getPosition();
+                string str(buffer->getBuffer() + pos, size);
+                buffer->setPosition(pos + size);
+                return str;
             }
-            else
-                return std::string();
+
+            string str;
+            str.reserve(size);
+            std::size_t i = 0;
+            while (true) {
+                std::size_t toRead = min(size-i, buffer->getRemaining());
+                std::size_t pos = buffer->getPosition();
+                str.append(buffer->getBuffer() + pos, toRead);
+                buffer->setPosition(pos + toRead);
+                i += toRead;
+                if (i < size)
+                    control->ensureData(1); // at least one
+                else
+                    break;
+            }
+            return str;
         }
     }
 }