search

open-dis / opendis7-source-generator

Project to generate a type-safe Java implementation of the DIS Protocol version 7, IEEE 1278.1-2012 and SISO-REF-010 Enumerations specifications.
Other
5 stars 10 forks source link

Unmarshaling causes NPE when enumerations not found #13

Open austinarrowsmith opened 3 years ago

austinarrowsmith commented 3 years ago

The current implementation of unmarshaling PDUs performs some enumeration lookups of the extracted values. If the value isn't found, it returns a null. This causes issues later in determining the unmarshalledSize() of the PDU, resulting in a NullPointerException (NPE).

One such scenario is seen in open-dis/opendis7-source-generator#14. ie: When some of the values are high end shorts (read as negative values), resulting in a failed enumeration lookup (no negative mappings in the enumeration classes), which returns a null for that instance. Later the parent object requests a .getMarshalledSize(), which performs the same thing on all member objects, and as one is a null the method bombs out.

But, as a lot of the Enumerations are filled with holes / incomplete, it's very easy to pull out a value that results in the null.

Example

In BeamStatus.java:getMarshalledSize() :

marshalSize += beamState.getMarshalledSize(); // When `beamState` is not mapped from the enumeration possibilities, this is `null`, resulting in a NPE.

Sure, this particular example should be either on or off, but due to how it is unmarshalled, I've seen other values extracted from the byte. This could be related to the next issue (#7 ) though.

There are other PDUs though, such as the name of an emitter / radio / etc where it may not exist either.

Suggestion

If the mapping is insisted at PDU generation time, perhaps ensure that allgenerated Enumeration classes from the SISO standard (SISO-REF-010-v28 at time of viewing) contain an "unknown"/"invalid"/"other" mapping, and return that instead of null? Some already do, but not all.

terry-norbraten commented 11 months ago

@austinarrowsmith Issue open-dis/opendis7-source-generator#9 certainly supports your NPE issue. EntityTypes are created with a default PlatformDomain.OTHER which certainly isn't correct if another Domain is desired without specifically setting that Domain. We're taking a look at if forcing a "Domain" at instantiation time is the behavior we intend.