From d515d34eb26ecd73cabb2c42ced8b8ddaa423863 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 6 Dec 2025 08:46:03 -0800 Subject: [PATCH 1/3] Fix enum pointer decoding with @MaxMindDbCreator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an enum with @MaxMindDbCreator was accessed via a pointer in the database, decoding failed with ConstructorNotFoundException. This affected enums like ConnectionType in GeoIP2 when data was deduplicated via pointers. The bug was in requiresLookupContext() which called loadConstructorMetadata() before checking for creator methods. Enums don't have usable public constructors, causing the exception. Fix: - Add cls.isEnum() to early-return checks (zero overhead for enums) - Add @MaxMindDbCreator check for non-enum classes with creators - Add negative caching to getCachedCreator() to avoid repeated method scanning for classes without creators Fixes: https://github.com/maxmind/GeoIP2-java/issues/644 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 8 ++ src/main/java/com/maxmind/db/Decoder.java | 17 ++- src/test/java/com/maxmind/db/ReaderTest.java | 104 +++++++++++++++++++ 3 files changed, 126 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbb2519..2d19be4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ CHANGELOG ========= +4.0.2 +------------------ + +* Fixed a bug where enums with `@MaxMindDbCreator` would throw + `ConstructorNotFoundException` when the data was stored via a pointer + in the database. This commonly occurred with deduplicated data in larger + databases. Reported by Fabrice Bacchella. GitHub #644 in GeoIP2-java. + 4.0.1 (2025-12-02) ------------------ diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 608fac57..0c3aa807 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -30,6 +30,9 @@ class Decoder { private static final int[] POINTER_VALUE_OFFSETS = {0, 0, 1 << 11, (1 << 19) + (1 << 11), 0}; + // Sentinel to cache "no creator method exists" to avoid repeated method scanning + private static final CachedCreator NO_CREATOR = new CachedCreator(null, null); + private final NodeCache cache; private final long pointerBase; @@ -184,10 +187,17 @@ private boolean requiresLookupContext(Class cls) { || cls.equals(Object.class) || Map.class.isAssignableFrom(cls) || List.class.isAssignableFrom(cls) + || cls.isEnum() || isSimpleType(cls)) { return false; } + // Non-enum classes with @MaxMindDbCreator don't require lookup context + // since they just convert simple values (strings, booleans, etc.) + if (getCachedCreator(cls) != null) { + return false; + } + var cached = getCachedConstructor(cls); if (cached == null) { cached = loadConstructorMetadata(cls); @@ -960,14 +970,15 @@ private Object convertValue(Object value, Class targetType) { private CachedCreator getCachedCreator(Class cls) { CachedCreator cached = this.creators.get(cls); + if (cached == NO_CREATOR) { + return null; // Known to have no creator + } if (cached != null) { return cached; } CachedCreator creator = findCreatorMethod(cls); - if (creator != null) { - this.creators.putIfAbsent(cls, creator); - } + this.creators.putIfAbsent(cls, creator != null ? creator : NO_CREATOR); return creator; } diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 890a95ac..041bdf48 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2042,6 +2042,110 @@ private void testIpV6(Reader reader, File file) throws IOException { } } + // ========================================================================= + // Tests for enum with @MaxMindDbCreator when data is stored via pointer + // See: https://github.com/maxmind/GeoIP2-java/issues/644 + // ========================================================================= + + /** + * Enum with @MaxMindDbCreator for converting string values. + * This simulates how ConnectionType enum works in geoip2. + */ + enum ConnectionTypeEnum { + DIALUP("Dialup"), + CABLE_DSL("Cable/DSL"), + CORPORATE("Corporate"), + CELLULAR("Cellular"), + SATELLITE("Satellite"), + UNKNOWN("Unknown"); + + private final String name; + + ConnectionTypeEnum(String name) { + this.name = name; + } + + @MaxMindDbCreator + public static ConnectionTypeEnum fromString(String s) { + if (s == null) { + return UNKNOWN; + } + return switch (s) { + case "Dialup" -> DIALUP; + case "Cable/DSL" -> CABLE_DSL; + case "Corporate" -> CORPORATE; + case "Cellular" -> CELLULAR; + case "Satellite" -> SATELLITE; + default -> UNKNOWN; + }; + } + } + + /** + * Model class that uses the ConnectionTypeEnum for the connection_type field. + */ + static class TraitsModel { + ConnectionTypeEnum connectionType; + + @MaxMindDbConstructor + public TraitsModel( + @MaxMindDbParameter(name = "connection_type") + ConnectionTypeEnum connectionType + ) { + this.connectionType = connectionType; + } + } + + /** + * Top-level model for Enterprise database records. + */ + static class EnterpriseModel { + TraitsModel traits; + + @MaxMindDbConstructor + public EnterpriseModel( + @MaxMindDbParameter(name = "traits") + TraitsModel traits + ) { + this.traits = traits; + } + } + + /** + * This test passes because IP 74.209.24.0 has connection_type stored inline. + */ + @ParameterizedTest + @MethodSource("chunkSizes") + public void testEnumCreatorWithInlineData(int chunkSize) throws IOException { + try (var reader = new Reader(getFile("GeoIP2-Enterprise-Test.mmdb"), chunkSize)) { + var ip = InetAddress.getByName("74.209.24.0"); + var result = reader.get(ip, EnterpriseModel.class); + assertNotNull(result); + assertNotNull(result.traits); + assertEquals(ConnectionTypeEnum.CABLE_DSL, result.traits.connectionType); + } + } + + /** + * This test verifies that enums with @MaxMindDbCreator work correctly when + * the data is stored via a pointer (common for deduplication in databases). + * + *

Previously, this would throw ConstructorNotFoundException because + * requiresLookupContext() called loadConstructorMetadata() before checking + * for creator methods. + */ + @ParameterizedTest + @MethodSource("chunkSizes") + public void testEnumCreatorWithPointerData(int chunkSize) throws IOException { + try (var reader = new Reader(getFile("GeoIP2-Enterprise-Test.mmdb"), chunkSize)) { + var ip = InetAddress.getByName("89.160.20.112"); + var result = reader.get(ip, EnterpriseModel.class); + assertNotNull(result); + assertNotNull(result.traits); + assertEquals(ConnectionTypeEnum.CORPORATE, result.traits.connectionType); + } + } + static File getFile(String name) { return new File(ReaderTest.class.getResource("/maxmind-db/test-data/" + name).getFile()); } From 33336309815fd4da91c47316a546b842ea939ddd Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 6 Dec 2025 09:04:00 -0800 Subject: [PATCH 2/3] Update maxmind-db test data submodule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/test/java/com/maxmind/db/ReaderTest.java | 4 ++-- src/test/resources/maxmind-db | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 041bdf48..909aacb1 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -493,8 +493,8 @@ public void testNoIpV4SearchTreeStream(int chunkSizes) throws IOException { private void testNoIpV4SearchTree(Reader reader) throws IOException { - assertEquals("::0/64", reader.get(InetAddress.getByName("1.1.1.1"), String.class)); - assertEquals("::0/64", reader.get(InetAddress.getByName("192.1.1.1"), String.class)); + assertEquals("::/64", reader.get(InetAddress.getByName("1.1.1.1"), String.class)); + assertEquals("::/64", reader.get(InetAddress.getByName("192.1.1.1"), String.class)); } @ParameterizedTest diff --git a/src/test/resources/maxmind-db b/src/test/resources/maxmind-db index 86095bd9..b6eb21df 160000 --- a/src/test/resources/maxmind-db +++ b/src/test/resources/maxmind-db @@ -1 +1 @@ -Subproject commit 86095bd9855d6313c501fe0097891a3c6734ae90 +Subproject commit b6eb21df882cc939d6b31bc05811a101f17fe148 From 0dbb03cfd7bcef19be528212b83e814e7b054ff5 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 6 Dec 2025 09:07:10 -0800 Subject: [PATCH 3/3] Improve error messages for constructor argument mismatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When constructor invocation fails with IllegalArgumentException, the error diagnostic now: 1. Reports null-to-primitive issues: "null value for primitive double parameter 'ip_risk'" - this identifies the actual root cause when a database field is missing and the model uses a primitive type. 2. Uses proper type compatibility checks for boxed/primitive pairs. Previously, Boolean/boolean pairs were incorrectly reported as type mismatches even though auto-unboxing handles them correctly. This makes debugging much easier by pointing to the actual problem instead of reporting misleading type mismatches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 3 + src/main/java/com/maxmind/db/Decoder.java | 62 ++++++++++++++++++-- src/test/java/com/maxmind/db/ReaderTest.java | 38 ++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d19be4b..a6058418 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ CHANGELOG `ConstructorNotFoundException` when the data was stored via a pointer in the database. This commonly occurred with deduplicated data in larger databases. Reported by Fabrice Bacchella. GitHub #644 in GeoIP2-java. +* Improved error messages when constructor invocation fails. The error now + correctly identifies null values being passed to primitive parameters + instead of reporting misleading boxed/primitive type mismatches. 4.0.1 (2025-12-02) ------------------ diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 0c3aa807..f76b5851 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -706,18 +706,68 @@ private Object decodeMapIntoObject(int size, Class cls) var sbErrors = new StringBuilder(); for (var key : parameterIndexes.keySet()) { var index = parameterIndexes.get(key); - if (parameters[index] != null - && !parameters[index].getClass().isAssignableFrom(parameterTypes[index])) { - sbErrors.append(" argument type mismatch in " + key + " MMDB Type: " - + parameters[index].getClass().getCanonicalName() - + " Java Type: " + parameterTypes[index].getCanonicalName()); + if (parameters[index] == null && parameterTypes[index].isPrimitive()) { + sbErrors.append(" null value for primitive ") + .append(parameterTypes[index].getName()) + .append(" parameter '").append(key).append("'."); + } else if (parameters[index] != null + && !isAssignableType(parameters[index].getClass(), parameterTypes[index])) { + sbErrors.append(" argument type mismatch in ").append(key) + .append(" MMDB Type: ") + .append(parameters[index].getClass().getCanonicalName()) + .append(" Java Type: ") + .append(parameterTypes[index].getCanonicalName()) + .append("."); } } throw new DeserializationException( - "Error creating object of type: " + cls.getSimpleName() + " - " + sbErrors, e); + "Error creating object of type: " + cls.getSimpleName() + " -" + sbErrors, e); } } + /** + * Checks if an actual type can be assigned to an expected type, + * accounting for primitive/boxed equivalents (e.g., Boolean to boolean). + */ + private static boolean isAssignableType(Class actual, Class expected) { + if (expected.isAssignableFrom(actual)) { + return true; + } + // Check primitive/boxed equivalents for auto-unboxing + if (expected.isPrimitive()) { + return actual.equals(boxedType(expected)); + } + return false; + } + + private static Class boxedType(Class primitive) { + if (primitive == boolean.class) { + return Boolean.class; + } + if (primitive == byte.class) { + return Byte.class; + } + if (primitive == short.class) { + return Short.class; + } + if (primitive == int.class) { + return Integer.class; + } + if (primitive == long.class) { + return Long.class; + } + if (primitive == float.class) { + return Float.class; + } + if (primitive == double.class) { + return Double.class; + } + if (primitive == char.class) { + return Character.class; + } + return primitive; + } + private boolean shouldInstantiateFromContext(Class parameterType) { if (parameterType == null || parameterType.isPrimitive() diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 909aacb1..adfe2f3d 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2146,6 +2146,44 @@ public void testEnumCreatorWithPointerData(int chunkSize) throws IOException { } } + // Model class with primitive double field for testing null-to-primitive error messages + static class IpRiskModelWithPrimitive { + public final double ipRisk; + + @MaxMindDbConstructor + public IpRiskModelWithPrimitive( + @MaxMindDbParameter(name = "ip_risk") double ipRisk + ) { + this.ipRisk = ipRisk; + } + } + + /** + * Tests that error messages correctly report null-to-primitive issues instead of + * misleading boxed/primitive type mismatch messages. + * + *

IP 11.1.2.3 in the IP Risk test database doesn't have an ip_risk field. + * When deserializing to a model with a primitive double, the error message should + * correctly identify "null value for primitive double" rather than reporting + * misleading Boolean/boolean mismatches. + */ + @ParameterizedTest + @MethodSource("chunkSizes") + public void testNullToPrimitiveErrorMessage(int chunkSize) throws IOException { + try (var reader = new Reader(getFile("GeoIP2-IP-Risk-Test.mmdb"), chunkSize)) { + var ip = InetAddress.getByName("11.1.2.3"); + + var exception = assertThrows(DeserializationException.class, + () -> reader.get(ip, IpRiskModelWithPrimitive.class)); + + // Error message should mention null value for primitive, not type mismatch + assertTrue(exception.getMessage().contains("null value for primitive double"), + "Error message should identify null-to-primitive issue: " + exception.getMessage()); + assertTrue(exception.getMessage().contains("ip_risk"), + "Error message should name the problematic parameter: " + exception.getMessage()); + } + } + static File getFile(String name) { return new File(ReaderTest.class.getResource("/maxmind-db/test-data/" + name).getFile()); }