Support optional tail in plist deserialisation

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsi/include/dds/ddsi/ddsi_plist_generic.h

@@ -13,6 +13,7 @@
 #define DDSI_PLIST_GENERIC_H
 
 #include <stddef.h>
+#include <assert.h>
 #include <stdbool.h>
 
 #include "dds/export.h"
@@ -39,9 +40,19 @@ enum pserop {
   XbPROP, /* boolean: omit in serialized form; skip serialization if false; always true on deserialize */
   XG, /* GUID */
   XK, /* keyhash */
-  XQ /* arbitary non-nested sequence */
+  XQ, /* arbitary non-nested sequence */
+  Xopt, /* remainder is optional on deser, 0-init if not present */
 } ddsrt_attribute_packed;
 
+inline bool pserop_seralign_is_1 (enum pserop op) {
+  /* NB: XbPROP is never serialized, so its alignment is irrelevant.  If ever there
+     is a need to allow calling this function when op = XbPROP, it needs to be changed
+     to taking the address of the pserop, and in that case inspect the following
+     operator */
+  assert (op != XbPROP && op != Xopt && op != XSTOP);
+  return (op >= Xo && op <= XK);
+}
+
 DDS_EXPORT void plist_fini_generic (void * __restrict dst, const enum pserop *desc, bool aliased);
 DDS_EXPORT dds_return_t plist_deser_generic (void * __restrict dst, const void * __restrict src, size_t srcsize, bool bswap, const enum pserop * __restrict desc);
 DDS_EXPORT dds_return_t plist_ser_generic (void **dst, size_t *dstsize, const void *src, const enum pserop * __restrict desc);

src/core/ddsi/src/q_plist.c

@@ -98,10 +98,10 @@ struct piddesc {
   size_t plist_offset;   /* offset from start of nn_plist_t */
   size_t size;           /* in-memory size for copying */
   union {
-    /* descriptor for generic code: 11 is enough for the current set of
+    /* descriptor for generic code: 12 is enough for the current set of
        parameters, compiler will warn if one ever tries to use more than
-       will fit; on non-GCC/Clang and 32-bits machines */
+       will fit */
-    const enum pserop desc[11];
+    const enum pserop desc[12];
     struct {
       dds_return_t (*deser) (void * __restrict dst, size_t * __restrict dstoff, struct flagset *flagset, uint64_t flag, const struct dd * __restrict dd, size_t * __restrict srcoff);
       dds_return_t (*ser) (struct nn_xmsg *xmsg, nn_parameterid_t pid, const void *src, size_t srcoff);
@@ -114,6 +114,8 @@ struct piddesc {
   dds_return_t (*deser_validate_xform) (void * __restrict dst, const struct dd * __restrict dd);
 };
 
+extern inline bool pserop_seralign_is_1 (enum pserop op);
+
 static void log_octetseq (uint32_t cat, const struct ddsrt_log_cfg *logcfg, uint32_t n, const unsigned char *xs);
 static dds_return_t validate_history_qospolicy (const dds_history_qospolicy_t *q);
 static dds_return_t validate_resource_limits_qospolicy (const dds_resource_limits_qospolicy_t *q);
@@ -336,6 +338,7 @@ static size_t ser_generic_srcsize (const enum pserop * __restrict desc)
       case XK: SIMPLE (XK, nn_keyhash_t); break;
       case XbPROP: SIMPLE (XbPROP, unsigned char); break;
       case XQ: SIMPLE (XQ, ddsi_octetseq_t); while (*++desc != XSTOP) { } break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -388,6 +391,7 @@ static void fini_generic_embeddable (void * __restrict dst, size_t * __restrict
         }, ddsrt_free (x->value));
         while (desc + 1 != desc_end && *++desc != XSTOP) { }
         break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -395,6 +399,26 @@ static void fini_generic_embeddable (void * __restrict dst, size_t * __restrict
 #undef COMPLEX
 }
 
+static size_t pserop_memalign (enum pserop op)
+{
+  switch (op)
+  {
+    case XO: case XQ: return alignof (ddsi_octetseq_t);
+    case XS: return alignof (char *);
+    case XG: return alignof (ddsi_guid_t);
+    case XK: return alignof (nn_keyhash_t);
+    case Xb: case Xbx2: return 1;
+    case Xo: case Xox2: return 1;
+    case XbCOND: case XbPROP: return 1;
+    case XE1: case XE2: case XE3: return sizeof (uint32_t);
+    case Xi: case Xix2: case Xix3: case Xix4: return sizeof (int32_t);
+    case Xu: case Xux2: case Xux3: case Xux4: case Xux5: return sizeof (uint32_t);
+    case XD: case XDx2: return alignof (dds_duration_t);
+    case XSTOP: case Xopt: assert (0);
+  }
+  return 0;
+}
+
 static dds_return_t deser_generic (void * __restrict dst, size_t * __restrict dstoff, struct flagset *flagset, uint64_t flag, const struct dd * __restrict dd, size_t * __restrict srcoff, const enum pserop * __restrict desc)
 {
   enum pserop const * const desc_in = desc;
@@ -409,8 +433,7 @@ static dds_return_t deser_generic (void * __restrict dst, size_t * __restrict ds
     switch (*desc)
     {
       case XSTOP:
-        *flagset->present |= flag;
+        goto success;
-        return 0;
       case XO: { /* octet sequence */
         ddsi_octetseq_t * const x = deser_generic_dst (dst, dstoff, alignof (ddsi_octetseq_t));
         if (deser_uint32 (&x->length, dd, srcoff) < 0 || dd->bufsz - *srcoff < x->length)
@@ -543,9 +566,29 @@ static dds_return_t deser_generic (void * __restrict dst, size_t * __restrict ds
         while (*++desc != XSTOP) { }
         break;
       }
+      case Xopt: { /* remainder is optional; alignment is very nearly always 4 */
+        bool end_of_input;
+        if (pserop_seralign_is_1 (desc[1]))
+          end_of_input = (*srcoff + 1 > dd->bufsz);
+        else
+        {
+          *srcoff = (*srcoff + 3) & ~(size_t)3;
+          end_of_input = (*srcoff + 4 > dd->bufsz);
+        }
+        if (end_of_input)
+        {
+          void * const x = deser_generic_dst (dst, dstoff, pserop_memalign (desc[1]));
+          size_t rem_size = ser_generic_srcsize (desc + 1);
+          memset (x, 0, rem_size);
+          goto success;
+        }
+      }
     }
     desc++;
   }
+success:
+  *flagset->present |= flag;
+  return 0;
 
 fail:
   fini_generic_embeddable (dst, &dstoff_in, desc_in, desc, *flagset->aliased & flag);
@@ -604,6 +647,7 @@ static void ser_generic_size_embeddable (size_t *dstoff, const void *src, size_t
         for (uint32_t i = 0; i < x->length; i++)
           ser_generic_size_embeddable (dstoff, x->value, i * elem_size, desc + 1);
       }); while (*++desc != XSTOP) { } break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -761,6 +805,8 @@ static dds_return_t ser_generic_embeddable (char * const data, size_t *dstoff, c
         while (*++desc != XSTOP) { }
         break;
       }
+      case Xopt:
+        break;
     }
     desc++;
   }
@@ -832,6 +878,7 @@ static dds_return_t unalias_generic (void * __restrict dst, size_t * __restrict
           unalias_generic (x->value, &elem_off, gen_seq_aliased, desc + 1);
         }
       }); while (*++desc != XSTOP) { } break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -917,6 +964,7 @@ static dds_return_t valid_generic (const void *src, size_t srcoff, const enum ps
           }
         }
       }); while (*++desc != XSTOP) { } break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -978,6 +1026,7 @@ static bool equal_generic (const void *srcx, const void *srcy, size_t srcoff, co
           }
         }
       }); while (*++desc != XSTOP) { } break;
+      case Xopt: break;
     }
     desc++;
   }
@@ -1091,7 +1140,9 @@ static const struct piddesc piddesc_omg[] = {
   QP  (DEADLINE,                            deadline, XD),
   QP  (LATENCY_BUDGET,                      latency_budget, XD),
   QP  (LIVELINESS,                          liveliness, XE2, XD),
-  QP  (PROPERTY_LIST,                       property, XQ, XbPROP, XS, XS, XSTOP, XQ, XbPROP, XS, XO, XSTOP),
+  /* Property list used to be of type [(String,String]), security changed into ([String,String],Maybe [(String,[Word8])]),
+     the "Xopt" here is to allow both forms on input, with an assumed empty second sequence if the old form was received */
+  QP  (PROPERTY_LIST,                       property, XQ, XbPROP, XS, XS, XSTOP, Xopt, XQ, XbPROP, XS, XO, XSTOP),
   /* Reliability encoding does not follow the rules (best-effort/reliable map to 1/2 instead of 0/1 */
   { PID_RELIABILITY, PDF_QOS | PDF_FUNCTION, QP_RELIABILITY, "RELIABILITY",
     offsetof (struct nn_plist, qos.reliability), membersize (struct nn_plist, qos.reliability),

src/core/ddsi/tests/plist_generic.c

@@ -17,7 +17,7 @@
 #include "dds/ddsi/ddsi_plist_generic.h"
 
 struct desc {
-  const enum pserop desc[10];
+  const enum pserop desc[20];
   const void *data;
   size_t exp_sersize;
   const unsigned char *exp_ser;
@@ -104,6 +104,29 @@ struct desc descs[] = {
     },
     &(struct{char b; oseq seq;}){1, {3, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
       {1,"orange",2}, {1,"fig",4}, {1,"prune",5}}}},
+  },
+  { {Xb,XQ,XbPROP,XS,Xo,XSTOP, Xopt,XQ,XbPROP,XS,Xo,XSTOP, XSTOP},
+    &(struct{char b; oseq seq, seq2;}){1,
+      {5, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+        {0,"apple",1}, {1,"orange",2}, {0,"cherry",3}, {1,"fig",4}, {1,"prune",5}}},
+      {2, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+        {1,"oak",8}, {0,"beech",9}}}
+    },
+    57, (raw){1, 0,0,0,
+      SER32(3),
+      SER32(7), 'o','r','a','n','g','e',0, 2,
+      SER32(4), 'f','i','g',0, 4, 0,0,0,
+      SER32(6), 'p','r','u','n','e',0, 5,
+      0,
+      SER32(1),
+      SER32(4), 'o','a','k',0, 8
+    },
+    &(struct{char b; oseq seq, seq2;}){1,
+      {3, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+        {1,"orange",2}, {1,"fig",4}, {1,"prune",5}}},
+      {1,  (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+        {1,"oak",8}}}
+    }
   }
 };
 
@@ -236,3 +259,56 @@ CU_Test (ddsi_plist_generic, invalid_input)
       CU_ASSERT_FATAL (ret != DDS_RETCODE_OK);
   }
 }
+
+CU_Test (ddsi_plist_generic, optional)
+{
+  union {
+    uint64_t u;
+    void *p;
+    char buf[256];
+  } mem;
+
+  enum pserop ser_desc[] = {Xb,XQ,XbPROP,XS,Xo,XSTOP,XSTOP};
+  enum pserop deser_desc[] = {Xb,XQ,XbPROP,XS,Xo,XSTOP, Xopt,XQ,XbPROP,XS,Xo,XSTOP, XSTOP};
+  const void *data = &(struct{char b; oseq seq;}){
+    1, {5, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+      {0,"apple",1}, {1,"orange",2}, {0,"cherry",3}, {1,"fig",4}, {1,"prune",5}}}};
+  size_t exp_sersize = 43;
+  const unsigned char *exp_ser = (raw){
+    1, 0,0,0, SER32(3),
+    SER32(7), 'o','r','a','n','g','e',0, 2,
+    SER32(4), 'f','i','g',0, 4, 0,0,0,
+    SER32(6), 'p','r','u','n','e',0, 5
+  };
+  const void *exp_data = &(struct{char b; oseq seq; oseq seq2;}){
+    1, {3, (unsigned char *)(struct{char b;char *s;uint8_t o;}[]){
+      {1,"orange",2}, {1,"fig",4}, {1,"prune",5}}},
+    {0, NULL}};
+
+  size_t memsize;
+  void *ser;
+  size_t sersize;
+  dds_return_t ret;
+  ret = plist_ser_generic (&ser, &sersize, data, ser_desc);
+  CU_ASSERT_FATAL (ret == DDS_RETCODE_OK);
+  CU_ASSERT (sersize == exp_sersize);
+  /* if sizes don't match, still check prefix */
+  size_t cmpsize = (sersize < exp_sersize) ? sersize : exp_sersize;
+  if (memcmp (ser, exp_ser, cmpsize) != 0)
+  {
+    printf ("ddsi_plist_generic_optional: memcmp\n");
+    for (size_t k = 0; k < cmpsize; k++)
+      printf ("  %3zu  %02x  %02x\n", k, ((unsigned char *)ser)[k], exp_ser[k]);
+    CU_ASSERT (!(bool)"memcmp");
+  }
+  /* check */
+  memsize = plist_memsize_generic (deser_desc);
+  CU_ASSERT_FATAL (memsize <= sizeof (mem));
+  memset (&mem, 0xee, sizeof (mem));
+  ret = plist_deser_generic (&mem, ser, sersize, false, deser_desc);
+  CU_ASSERT_FATAL (ret == DDS_RETCODE_OK);
+  /* the compare function should be happy with it */
+  CU_ASSERT (plist_equal_generic (exp_data, &mem, deser_desc));
+  plist_fini_generic (&mem, deser_desc, true);
+  ddsrt_free (ser);
+}