patrodyne / hisrc-basicjaxb

XJC plugins and tools for JAXB.
BSD 3-Clause "New" or "Revised" License
15 stars 6 forks source link

SimpleToString not nullsafe and uses string lengths #16

Closed dschulten closed 8 months ago

dschulten commented 10 months ago

If a JAXB bean generated with -XsimpleToString contains any null collections, the generated toString method fails with NPE.

Proof: the generated code looks like this:

{
            List<AttributedURIType> theMessageID;
            theMessageID = (((this.messageID!= null)&&(!this.messageID.isEmpty()))?this.getMessageID():null);
            stringBuilder.append("<size=");
            stringBuilder.append(theMessageID.size());
            stringBuilder.append(">");
        }

If this.theMessageID is null, the expression returns null and theMessageID.size() throws an NPE

patrodyne commented 10 months ago

Good catch, thank you. I've added a unit test. I'm adding some overdue javadoc on the generating classes before committing the fix. I'll report back here when the fix is committed.

patrodyne commented 10 months ago

I have committed the generation of a ternary operator to check for isSet before invoking .length on arrays or .size() on collections in ToStringCodeGenerationImplementor.java to generate code like this...

hisrc-basicjaxb/higher/tests/simple/target/generated-sources/xjc/org/jvnet/basicjaxb/tests/simple/customer/Customer.java

...
{
    List<String> theMiddleInitials;
    theMiddleInitials = (this.isSetMiddleInitials()?this.getMiddleInitials():null);
    stringBuilder.append(", ");
    stringBuilder.append("middleInitials=");
    stringBuilder.append("<size=");
    stringBuilder.append((this.isSetMiddleInitials()?theMiddleInitials.size():"null"));
    stringBuilder.append(">");
}
...
patrodyne commented 10 months ago

Or if generateIsSetMethod="false" then the generated code looks like...

hisrc-basicjaxb/higher/tests/simple/target/generated-sources/xjc/org/jvnet/basicjaxb/tests/simple/customer/Customer.java

{
    List<String> theMiddleInitials;
    theMiddleInitials = (((this.middleInitials!= null)&&(!this.middleInitials.isEmpty()))?this.getMiddleInitials():null);
    stringBuilder.append(", ");
    stringBuilder.append("middleInitials=");
    stringBuilder.append("<size=");
    stringBuilder.append((((this.middleInitials!= null)&&(!this.middleInitials.isEmpty()))?theMiddleInitials.size():"null"));
    stringBuilder.append(">");
}

And I just noticed the isEmpty check so I'll change the fall back option from "null" to "0" for more consistent semantics.

patrodyne commented 8 months ago

The fix for SimpleToStringPlugin is available in HiSrc BasicJAXB Release 2.2.0.