From abe97a2eb3a18c49796549fcf18a54b1f6b8d061 Mon Sep 17 00:00:00 2001 From: AK_ Date: Thu, 25 Sep 2025 22:50:24 -0400 Subject: [PATCH 1/3] [REFACTOR] simplify JsonInjector and NetworkDeserializer with safer parsing, validation, and created test classes as well --- .../java/com/maxmind/geoip2/JsonInjector.java | 41 ++++----- .../maxmind/geoip2/NetworkDeserializer.java | 49 ++++++++--- .../com/maxmind/geoip2/JsonInjectorTest.java | 40 +++++++++ .../geoip2/NetworkDeserializerTest.java | 88 +++++++++++++++++++ 4 files changed, 183 insertions(+), 35 deletions(-) create mode 100644 src/test/java/com/maxmind/geoip2/JsonInjectorTest.java create mode 100644 src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java diff --git a/src/main/java/com/maxmind/geoip2/JsonInjector.java b/src/main/java/com/maxmind/geoip2/JsonInjector.java index 21067339b..1e0e393f6 100644 --- a/src/main/java/com/maxmind/geoip2/JsonInjector.java +++ b/src/main/java/com/maxmind/geoip2/JsonInjector.java @@ -5,34 +5,31 @@ import com.fasterxml.jackson.databind.InjectableValues; import com.maxmind.db.Network; import com.maxmind.geoip2.record.Traits; +import java.util.HashMap; import java.util.List; +import java.util.Map; -class JsonInjector extends InjectableValues { - private final List locales; - private final String ip; - private final Network network; +final class JsonInjector extends InjectableValues { + private final Map injectables; - public JsonInjector(List locales, String ip, Network network) { - this.locales = locales; - this.ip = ip; - this.network = network; + JsonInjector(List locales, String ip, Network network) { + var map = new HashMap(4, 1f); + if (locales != null) { + map.put("locales", locales); + } + if (ip != null) { + map.put("ip_address", ip); + } + if (network != null) { + map.put("network", network); + } + map.put("traits", new Traits(ip, network)); + this.injectables = Map.copyOf(map); } @Override public Object findInjectableValue(Object valueId, DeserializationContext ctxt, BeanProperty forProperty, Object beanInstance) { - if ("locales".equals(valueId)) { - return locales; - } - if ("ip_address".equals(valueId)) { - return ip; - } - if ("network".equals(valueId)) { - return network; - } - if ("traits".equals(valueId)) { - return new Traits(ip, network); - } - return null; + return (valueId instanceof String k) ? injectables.get(k) : null; } -} \ No newline at end of file +} diff --git a/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java b/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java index 2e6401495..5f7901766 100644 --- a/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java +++ b/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java @@ -11,10 +11,10 @@ /** * This class provides a deserializer for the Network class. */ -public class NetworkDeserializer extends StdDeserializer { +public final class NetworkDeserializer extends StdDeserializer { /** - * Constructs a @{code NetworkDeserializer} object. + * Constructs a {@code NetworkDeserializer} with no type specified. */ public NetworkDeserializer() { this(null); @@ -30,23 +30,46 @@ public NetworkDeserializer(Class vc) { } @Override - public Network deserialize( - JsonParser jsonparser, DeserializationContext context) - throws IOException { + public Network deserialize(JsonParser jsonparser, DeserializationContext context) + throws IOException { - String cidr = jsonparser.getText(); - if (cidr == null) { + final String cidr = jsonparser.getValueAsString(); + if (cidr == null || cidr.isBlank()) { return null; } - String[] parts = cidr.split("/", 2); + return parseCidr(cidr.trim()); + } + + private static Network parseCidr(String cidr) throws IOException { + final String[] parts = cidr.split("/", 2); if (parts.length != 2) { - throw new RuntimeException("Invalid cidr format: " + cidr); + throw new IllegalArgumentException("Invalid CIDR format: " + cidr); } - int prefixLength = Integer.parseInt(parts[1]); + + final String addrPart = parts[0].trim(); + final String prefixPart = parts[1].trim(); + + final InetAddress address; try { - return new Network(InetAddress.getByName(parts[0]), prefixLength); + address = InetAddress.getByName(addrPart); } catch (UnknownHostException e) { - throw new RuntimeException(e); + throw new IOException("Unknown host in CIDR: " + cidr, e); } + + final int prefixLength; + try { + prefixLength = Integer.parseInt(prefixPart); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + "Invalid prefix length in CIDR: " + cidr, e); + } + + final int maxPrefix = (address.getAddress().length == 4) ? 32 : 128; + if (prefixLength < 0 || prefixLength > maxPrefix) { + throw new IllegalArgumentException( + "Prefix length out of range (0-" + maxPrefix + ") for CIDR: " + cidr); + } + + return new Network(address, prefixLength); } -} \ No newline at end of file +} diff --git a/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java b/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java new file mode 100644 index 000000000..892804d4d --- /dev/null +++ b/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java @@ -0,0 +1,40 @@ +package com.maxmind.geoip2; + +import com.maxmind.db.Network; +import com.maxmind.geoip2.record.Traits; +import org.junit.jupiter.api.Test; + +import java.net.InetAddress; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +final class JsonInjectorTest { + + @Test + void injectsValuesAndReusesTraits() throws Exception { + var locales = List.of("en", "fr"); + var ip = "1.2.3.4"; + var network = new Network(InetAddress.getByName("1.2.3.0"), 24); + + var injector = new JsonInjector(locales, ip, network); + + assertEquals(locales, injector.findInjectableValue("locales", null, null, null)); + assertEquals(ip, injector.findInjectableValue("ip_address", null, null, null)); + assertEquals(network, injector.findInjectableValue("network", null, null, null)); + + var traits1 = injector.findInjectableValue("traits", null, null, null); + var traits2 = injector.findInjectableValue("traits", null, null, null); + assertNotNull(traits1); + assertSame(traits1, traits2); + assertEquals(ip, ((Traits) traits1).getIpAddress()); + assertEquals(network, ((Traits) traits1).getNetwork()); + } + + @Test + void unknownOrNonStringKeyReturnsNull() { + var injector = new JsonInjector(List.of(), null, null); + assertNull(injector.findInjectableValue("unknown", null, null, null)); + assertNull(injector.findInjectableValue(123, null, null, null)); + } +} diff --git a/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java b/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java new file mode 100644 index 000000000..3c9922ec3 --- /dev/null +++ b/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java @@ -0,0 +1,88 @@ +package com.maxmind.geoip2; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.maxmind.db.Network; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.InetAddress; + +import static org.junit.jupiter.api.Assertions.*; + +final class NetworkDeserializerTest { + + + private static Network parse(String jsonString) throws IOException { + var deserializer = new NetworkDeserializer(); + JsonFactory jf = new JsonFactory(); + try (JsonParser p = jf.createParser(jsonString)) { + p.nextToken(); + return deserializer.deserialize(p, null); + } + } + private static void assertNetwork(Network n, String addr, int prefix) throws Exception { + assertNotNull(n); + assertEquals(InetAddress.getByName(addr), n.getNetworkAddress()); + assertEquals(prefix, n.getPrefixLength()); + } + + @Test + void parsesValidIPv4Cidr() throws Exception { + Network actual = parse("\"1.2.3.0/24\""); + assertNetwork(actual, "1.2.3.0", 24); + } + + @Test + void parsesValidIPv6Cidr() throws Exception { + Network actual = parse("\"2001:db8::/32\""); + assertNetwork(actual, "2001:db8::", 32); + } + + @Test + void trimsWhitespace() throws Exception { + Network actual = parse("\" 10.0.0.0/8 \""); + assertNetwork(actual, "10.0.0.0", 8); + } + + + @Test + void returnsNullOnJsonNull() throws Exception { + Network actual = parse("null"); + assertNull(actual); + } + + @Test + void returnsNullOnBlankString() throws Exception { + Network actual = parse("\" \""); + assertNull(actual); + } + + @Test + void throwsOnMissingSlash() { + assertThrows(IllegalArgumentException.class, () -> parse("\"1.2.3.0\"")); + } + + @Test + void throwsOnNonNumericPrefix() { + assertThrows(IllegalArgumentException.class, () -> parse("\"1.2.3.0/xx\"")); + } + + @Test + void throwsOnOutOfRangePrefixIpv4() { + assertThrows(IllegalArgumentException.class, () -> parse("\"1.2.3.0/64\"")); + assertThrows(IllegalArgumentException.class, () -> parse("\"1.2.3.0/-1\"")); + } + + @Test + void throwsOnOutOfRangePrefixIpv6() { + assertThrows(IllegalArgumentException.class, () -> parse("\"::/129\"")); + assertThrows(IllegalArgumentException.class, () -> parse("\"::/-1\"")); + } + + @Test + void wrapsUnknownHostInIOException() { + IOException ex = assertThrows(IOException.class, () -> parse("\"999.999.999.999/24\"")); + assertNotNull(ex.getCause()); + } +} From 7b823b0a8267b1c5527b9075072ecb44b53caaea Mon Sep 17 00:00:00 2001 From: AK_ Date: Fri, 26 Sep 2025 18:16:31 -0400 Subject: [PATCH 2/3] [REBASE + UPDATE] Removed JsonInjector and following tests --- .../com/maxmind/geoip2/JsonInjectorTest.java | 40 ------------------- 1 file changed, 40 deletions(-) delete mode 100644 src/test/java/com/maxmind/geoip2/JsonInjectorTest.java diff --git a/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java b/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java deleted file mode 100644 index 892804d4d..000000000 --- a/src/test/java/com/maxmind/geoip2/JsonInjectorTest.java +++ /dev/null @@ -1,40 +0,0 @@ -package com.maxmind.geoip2; - -import com.maxmind.db.Network; -import com.maxmind.geoip2.record.Traits; -import org.junit.jupiter.api.Test; - -import java.net.InetAddress; -import java.util.List; - -import static org.junit.jupiter.api.Assertions.*; - -final class JsonInjectorTest { - - @Test - void injectsValuesAndReusesTraits() throws Exception { - var locales = List.of("en", "fr"); - var ip = "1.2.3.4"; - var network = new Network(InetAddress.getByName("1.2.3.0"), 24); - - var injector = new JsonInjector(locales, ip, network); - - assertEquals(locales, injector.findInjectableValue("locales", null, null, null)); - assertEquals(ip, injector.findInjectableValue("ip_address", null, null, null)); - assertEquals(network, injector.findInjectableValue("network", null, null, null)); - - var traits1 = injector.findInjectableValue("traits", null, null, null); - var traits2 = injector.findInjectableValue("traits", null, null, null); - assertNotNull(traits1); - assertSame(traits1, traits2); - assertEquals(ip, ((Traits) traits1).getIpAddress()); - assertEquals(network, ((Traits) traits1).getNetwork()); - } - - @Test - void unknownOrNonStringKeyReturnsNull() { - var injector = new JsonInjector(List.of(), null, null); - assertNull(injector.findInjectableValue("unknown", null, null, null)); - assertNull(injector.findInjectableValue(123, null, null, null)); - } -} From 23d6c80eb5007ef688fd626f2fe0641d0a77a53e Mon Sep 17 00:00:00 2001 From: AK_ Date: Sat, 27 Sep 2025 16:07:06 -0400 Subject: [PATCH 3/3] [UPDATE] Remove trimming in NetworkDeserializer and update tests --- src/main/java/com/maxmind/geoip2/NetworkDeserializer.java | 6 +++--- .../java/com/maxmind/geoip2/NetworkDeserializerTest.java | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java b/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java index 5f7901766..7932b6f21 100644 --- a/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java +++ b/src/main/java/com/maxmind/geoip2/NetworkDeserializer.java @@ -37,7 +37,7 @@ public Network deserialize(JsonParser jsonparser, DeserializationContext context if (cidr == null || cidr.isBlank()) { return null; } - return parseCidr(cidr.trim()); + return parseCidr(cidr); } private static Network parseCidr(String cidr) throws IOException { @@ -46,8 +46,8 @@ private static Network parseCidr(String cidr) throws IOException { throw new IllegalArgumentException("Invalid CIDR format: " + cidr); } - final String addrPart = parts[0].trim(); - final String prefixPart = parts[1].trim(); + final String addrPart = parts[0]; + final String prefixPart = parts[1]; final InetAddress address; try { diff --git a/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java b/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java index 3c9922ec3..60acf26d0 100644 --- a/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java +++ b/src/test/java/com/maxmind/geoip2/NetworkDeserializerTest.java @@ -40,12 +40,13 @@ void parsesValidIPv6Cidr() throws Exception { } @Test - void trimsWhitespace() throws Exception { - Network actual = parse("\" 10.0.0.0/8 \""); - assertNetwork(actual, "10.0.0.0", 8); + void rejectsWhitespaceInCidr() { + assertThrows(IOException.class, () -> parse("\" 10.0.0.0/8 \"")); } + + @Test void returnsNullOnJsonNull() throws Exception { Network actual = parse("null");