search

maxmind / MaxMind-DB-Reader-java

Java reader for the MaxMind DB format
https://maxmind.github.io/MaxMind-DB-Reader-java/
Apache License 2.0
113 stars 43 forks source link

Decoder NullPointerException masks underlying exception #164

Closed masseyke closed 5 months ago

masseyke commented 5 months ago

If I have an mdb file where one of the fields is the wrong type (in this case, lat and lon were stored as strings rather than doubles), then Decoder attempts to throw this exception:

java.lang.IllegalArgumentException: argument type mismatch
    at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:65)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMapIntoObject(Decoder.java:441)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMap(Decoder.java:341)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeByType(Decoder.java:162)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:151)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMapIntoObject(Decoder.java:434)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMap(Decoder.java:341)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeByType(Decoder.java:162)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:151)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:76)
    at com.maxmind.db@3.1.0/com.maxmind.db.Reader.resolveDataPointer(Reader.java:411)
    at com.maxmind.db@3.1.0/com.maxmind.db.Reader.getRecord(Reader.java:185)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.get(DatabaseReader.java:280)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.getCity(DatabaseReader.java:365)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.tryCity(DatabaseReader.java:359)

Then Decoder attempts to format a nice message explaining to the user what the problem is, but in doing so throws a NullPointerException because some optional fields are null (in this case average_income is null):

java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "parameters[index]" is null
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMapIntoObject(Decoder.java:450)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMap(Decoder.java:341)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeByType(Decoder.java:162)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:151)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMapIntoObject(Decoder.java:434)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeMap(Decoder.java:341)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decodeByType(Decoder.java:162)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:151)
    at com.maxmind.db@3.1.0/com.maxmind.db.Decoder.decode(Decoder.java:76)
    at com.maxmind.db@3.1.0/com.maxmind.db.Reader.resolveDataPointer(Reader.java:411)
    at com.maxmind.db@3.1.0/com.maxmind.db.Reader.getRecord(Reader.java:185)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.get(DatabaseReader.java:280)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.getCity(DatabaseReader.java:365)
    at com.maxmind.geoip2@4.2.0/com.maxmind.geoip2.DatabaseReader.tryCity(DatabaseReader.java:359)

So the user never gets the message about a field being stored with the wrong type, and instead is confused by a NullPointerException. I think line 450 probably ought to just ignore any parameters where parameters[index] is null.

More information on how to produce a file that caused this is at https://discuss.elastic.co/t/geoip-challenges-custom-city-mmdb/355173/13.

horgh commented 5 months ago

Thank you for the report! Yes, checking for null there seems like it would be the thing to do.