Version Check Logic Error

Contribution Depot
1

Lines 242-245 of SceneLoader.java in jme3 seems to throw a NullPointerException when logging an incorrect version number:



[java]

String version = attribs.getValue("formatVersion");

if (version == null && !version.equals("1.0.0") && !version.equals("1.0.1"))

logger.log(Level.WARNING, "Unrecognized version number"

  • " in dotScene file: {0}", version);

    [/java]



    If the version is ever null it will throw an exception, and the version number is never actually checked against invalid values. I can only assume this is meant to be:



    [java]

    String version = attribs.getValue("formatVersion");

    if (version == null || (!version.equals("1.0.0") && !version.equals("1.0.1")))

    logger.log(Level.WARNING, "Unrecognized version number"
  • " in dotScene file: {0}", version);

    [/java]



    P.S. Perhaps this was better left to the GoogleCode issues list, but I was hoping to get the experience of making a commit myself :slight_smile:
2

Have you seen it happen or are you speculating.



I’d expect logger.log( “{0}”, null ) to print “null”

3

Speculating, noticed it in a bug sweep. However the logger.log code should not even be reachable if the version is null. The if statement logic is basically “if (a == null && !a.equals(b)…)” which should throw an exception every time a is null. What worries me is that this means the version is never actually checked for good values since the rest of the if statement is unreachable.



I can whip up a test case if need be.

4

I don’t get whats the issue… If its null you get the warning log, if its not null and not 1.0.0 or 1.0.1 then you get it too… All how its supposed to be… You cannot perform equals on null, so the check isn’t done on null. First the check on null is done and if that fails the check on the two versions is done…?

Edit: duh, I was reading your correction ^^

5

Should be or instead of and, I think. He’s right.



I misunderstood the issue.

6

[java]

if (version == null && !version.equals(“1.0.0”) && !version.equals(“1.0.1”))

[/java]



Er, what I was going for is if version == null, it passes the first condition but throws a NullPointerException on the second since you are calling “null.equals(“1.0.0”)”. When version is not null this if statement is skipped, thus it is impossible for this to actually check against 1.0.0 or 1.0.1.



If it’s not a serious issue please disregard, just thought this looked funny.

7

Hooray! Does that mean I can do a patch?

8
@cliveanderson said:
Hooray! Does that mean I can do a patch?

Sorry to pop your party, but I already fixed it :roll:
9

Oh well, glad I could help!