validate and normalize received CDR data

The CDR deserializer failed to check it was staying within the bounds of
the received data, and it turns out it also was inconsistent in its
interpretation of the (undocumented) serializer instructions.  This
commit adds some information on the instruction format obtained by
reverse engineering the code and studying the output of the IDL
preprocessor, and furthermore changes a lot of the types used in the
(de)serializer code to have some more compiler support.  The IDL
preprocessor is untouched and the generated instructinos do exactly the
same thing (except where change was needed).

The bulk of this commit replaces the implementation of the
(de)serializer.  It is still rather ugly, but at least the very long
functions with several levels of nested conditions and switch statements
have been split out into multiple functions.  Most of these have single
call-sites, so the compiler hopefully inlines them nicely.

The other important thing is that it adds a "normalize" function that
validates the structure of the CDR and performs byteswapping if
necessary.  This means the deserializer can now assume a well-formed
input in native byte-order.  Checks and conditional byteswaps have been
removed accordingly.

It changes some types to make a compile-time distinction between
read-only, native-endianness input, a native-endianness output, and a
big-endian output for dealing with key hashes.  This should reduce the
risk of accidentally mixing endianness or modifying an input stream.

The preprocessor has been modified to indicate the presence of unions in
a topic type in the descriptor flags.  If a union is present, any
memory allocated in a sample is freed first and the sample is zero'd out
prior to deserializing the new value.  This is to prevent reading
garbage pointers for strings and sequences when switching union cases.

The test tool has been included in the commit but it does not get run by
itself.  Firstly, it requires the presence of OpenSplice DDS as an
alternative implementation to check the CDR processing against.
Secondly, it takes quite a while to run and is of no interest unless one
changes something in the (de)serialization.

Finally, I have no idea why there was a "CDR stream" interface among the
public functions.  The existing interfaces are fundamentally broken by
the removal of arbitrary-endianness streams, and the interfaces were
already incapable of proper error notification.  So, they have been
removed.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-05-10 17:59:06 +08:00 committed by eboasson
parent d91e7b34c9
commit 3067a69c92
25 changed files with 2315 additions and 1941 deletions

View file

@ -48,6 +48,16 @@ public class ArrayType extends AbstractType
return new ArrayType (dimensions, subtype);
}
public boolean containsUnion ()
{
Type t = subtype;
while (t instanceof TypedefType)
{
t = ((TypedefType)t).getRef ();
}
return t.containsUnion ();
}
public ArrayList <String> getMetaOp (String myname, String structname)
{
ArrayList <String> result = new ArrayList <String> ();

View file

@ -57,6 +57,11 @@ public class BasicType extends AbstractType
return new BasicType (type);
}
public boolean containsUnion ()
{
return false;
}
public ArrayList <String> getMetaOp (String myname, String structname)
{
ArrayList <String> result = new ArrayList <String> (1);

View file

@ -36,6 +36,11 @@ public class BoundedStringType extends AbstractType
return size;
}
public boolean containsUnion ()
{
return false;
}
public ArrayList <String> getMetaOp (String myname, String structname)
{
ArrayList <String> result = new ArrayList <String> ();

View file

@ -40,6 +40,11 @@ public class EnumType extends BasicType implements NamedType
return SN.toString ("_");
}
public boolean containsUnion ()
{
return false;
}
public void addEnumerand (String val)
{
vals.add (val);

View file

@ -1207,6 +1207,10 @@ public class GenVisitor extends org.eclipse.cyclonedds.parser.IDLBaseVisitor <Vo
{
topicST.add ("flags", "DDS_TOPIC_NO_OPTIMIZE");
}
if (topicmeta.containsUnion ())
{
topicST.add ("flags", "DDS_TOPIC_CONTAINS_UNION");
}
topicST.add ("alignment", topicmeta.getAlignment ());
}

View file

@ -48,6 +48,16 @@ public class SequenceType extends AbstractType
return new SequenceType (subtype.dup (), name);
}
public boolean containsUnion ()
{
Type t = subtype;
while (t instanceof TypedefType)
{
t = ((TypedefType)t).getRef ();
}
return t.containsUnion ();
}
public ArrayList <String> getMetaOp (String myname, String structname)
{
ArrayList <String> result = new ArrayList <String> ();

View file

@ -176,6 +176,23 @@ public class StructType extends AbstractType implements NamedType
return false;
}
public boolean containsUnion ()
{
for (Member m : members)
{
Type mtype = m.type;
while (mtype instanceof TypedefType)
{
mtype = ((TypedefType)mtype).getRef ();
}
if (mtype.containsUnion ())
{
return true;
}
}
return false;
}
public long getKeySize ()
{
Type mtype;

View file

@ -29,5 +29,6 @@ public interface Type
public int getMetaOpSize ();
public Alignment getAlignment ();
public Type dup ();
public boolean containsUnion ();
}

View file

@ -27,6 +27,11 @@ public class TypedefType extends AbstractType implements NamedType
return new TypedefType (name, ref);
}
public boolean containsUnion ()
{
return ref.containsUnion ();
}
public ArrayList <String> getMetaOp (String myname, String structname)
{
return ref.getMetaOp (myname, structname);

View file

@ -207,6 +207,11 @@ public class UnionType extends AbstractType implements NamedType
less than the alignment of the members. */
}
public boolean containsUnion ()
{
return true;
}
public Alignment getAlignment ()
{
Alignment result = discriminant.getAlignment ();