disallow "null" size by default

pvDataCPP only explicitly checks for "null" size (-1) for Union, where it indicates the implicit "null" arm. Also string, where "null" is equivalent to zero length string.
epics-base · Nov 9, 2023 · 17464a1 · 17464a1
1 parent 2a56a08
commit 17464a1
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git a/src/dataencode.cpp b/src/dataencode.cpp
@@ -520,7 +520,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt,  const FieldDesc* desc, const
         switch (desc->code.code) {
         case TypeCode::Union: {
             Size select{};
-            from_wire(buf, select);
+            from_wire(buf, select, true);
             if(select.size==size_t(-1)) {
                 fld = Value();
                 return;
@@ -627,7 +627,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt,  const FieldDesc* desc, const
             for(auto& elem : arr) {
                 if(from_wire_as<uint8_t>(buf)!=0) { // strictly 1 or 0
                     Size select{};
-                    from_wire(buf, select);
+                    from_wire(buf, select, true);
 
                     if(select.size==size_t(-1)) {
                         // null element.  treated the same as 0 case (which is what actually happens)

diff --git a/src/pvaproto.h b/src/pvaproto.h
@@ -282,7 +282,7 @@ void to_wire(Buffer& buf, const Size& size)
 }
 
 inline
-void from_wire(Buffer& buf, Size& size)
+void from_wire(Buffer& buf, Size& size, bool allow_null=false)
 {
     if(!buf.ensure(1)) {
         buf.fault(__FILE__, __LINE__);
@@ -292,17 +292,16 @@ void from_wire(Buffer& buf, Size& size)
     if(s<254) {
         size.size = s;
 
-    } else if(s==255) {
-        // special "null"  used to encode empty Union
-        size.size = size_t(-1);
-
     } else if(s==254) {
         uint32_t ls = 0;
         from_wire(buf, ls);
         size.size = ls;
 
+    } else if(s==255 && allow_null) {
+        // special "null"  used to encode empty Union and (sometimes) empty string
+        size.size = size_t(-1);
+
     } else {
-        // unreachable (64-bit size so far not used)
         buf.fault(__FILE__, __LINE__);
     }
 }
@@ -331,7 +330,7 @@ inline
 void from_wire(Buffer& buf, std::string& s)
 {
     Size len{0};
-    from_wire(buf, len);
+    from_wire(buf, len, true);
     if(len.size==size_t(-1)) {
         s.clear();
 

diff --git a/test/testxcode.cpp b/test/testxcode.cpp
@@ -1161,6 +1161,20 @@ void testRegressCNEN()
     testTrue(prototype2.equalType(prototypeE))<<" "<<prototype2<<"\n"<<prototypeE;
 }
 
+void testRegressBadBitMask()
+{
+    testDiag("%s", __func__);
+
+    {
+        // "null" bitmask not allowed
+        uint8_t input[] = "\xff";
+        FixedBuf buf(false, input);
+        BitMask mask;
+        from_wire(buf, mask);
+        testFalse(buf.good())<<" at"<<buf.file()<<":"<<buf.line();
+    }
+}
+
 // test the common case for a pvRequest of caching an empty Struct
 void testEmptyRequest()
 {
@@ -1204,7 +1218,7 @@ void testEmptyRequest()
 
 MAIN(testxcode)
 {
-    testPlan(142);
+    testPlan(143);
     testSetup();
     testDeserializeString();
     testSerialize1();
@@ -1219,6 +1233,7 @@ MAIN(testxcode)
     testXCodeNTNDArray();
     testRegressRedundantBitMask();
     testRegressCNEN();
+    testRegressBadBitMask();
     testBadFieldName();
     testEmptyRequest();
     return testDone();