Repository navigation
Fix issues found during the connection-reuse review - #20
Merged
Merged
Conversation
TestContext.get()/post() and MockIpdataServer.handleBulk() wrapped HTTP bodies in an InputStreamReader without a charset, and MappingTest read fixture.json the same way, so on a JVM whose default charset is not UTF-8 (JDK 11 under a POSIX locale) the emoji flags and currency symbols served by the mock came back mangled and 11 tests failed. All of these now decode with StandardCharsets.UTF_8 explicitly; TestContext.post() sends its body as UTF-8 too, and the mock server uses the same constant for its response bytes and for query-string decoding. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The API returns a code for every entry of languages[] but the Language model had no field for it: the lenient client mapper silently dropped the value and the strict test mapper would have rejected any fixture carrying it. Add Language.code() (null when the API does not send it), add the attribute to the test fixtures and cover it with LanguageTest (full model, field selection, absent value, strict mapping). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ipv6SafeClient rebuilt the request with the deprecated Request.create(HttpMethod, String, Map, byte[], Charset), which also dropped the RequestTemplate; it now uses the overload that takes the template. FieldDecoder and ApiErrorDecoder read bodies through the deprecated Response.Body.asReader(), which is UTF-8 underneath; they now pass StandardCharsets.UTF_8 explicitly. Same behaviour, and the main sources compile without deprecation warnings. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
getSingleFieldClient() returned the full-model client and getApi() the single-field client. Both exist only to feed Lombok's @DeleGate on a package-private class, so the names are swapped and both getters made private; no generated method and no public API changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
.mvn/extensions.xml declared io.takari.polyglot:polyglot-groovy 0.4.3, which lets Maven read a pom.groovy. The project uses a plain pom.xml, so the extension and its Groovy runtime were downloaded on every fresh build and never used. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The table of contents linked "API" to #create-an-instance, an anchor that does not exist; "Bulk data" was a level-3 heading although it belongs to the API section; the "Basic Usage" sample output predated carrier, is_satellite, is_anycast, languages[].code, threat.is_vpn, threat.blocklists and threat.scores. The sample is now the model serialized from the 1.1.1.1 test fixture, and languages[].code is listed with the other recently added fields. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Guava 33.4.8 is affected by GHSA-xxph-c9ww-hj94 (CVE-2026-102554: uncontrolled memory allocation when deserializing CompactHashMap, CompactHashSet or MapMakerInternalMap); 33.7.2 is the first patched version. The project compiles against Jackson directly (ObjectMapper, @JsonProperty) but only received it through feign-jackson, which pins jackson-databind 2.13.4, affected by CVE-2022-42003 (and jackson-core 2.13.4 by CVE-2025-52999). jackson-databind 2.22.4 and jackson-annotations 2.22 (annotations are released once per minor line since 2.20) are now declared as direct dependencies, which also makes them visible to the repository's dependency graph. The new versions deprecate two calls in IpdataServiceBuilder; they are replaced by their documented equivalents, available since Jackson 2.7 and Guava 25: ObjectMapper.setDefaultPropertyInclusion(Include) for setSerializationInclusion(Include), and CacheBuilder.expireAfterWrite(Duration) for expireAfterWrite(long, TimeUnit). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #19: fixes the items that review listed under "Noticed, not changed". The test helpers decode HTTP bodies as UTF-8 explicitly so the suite passes under any locale, the
Languagemodel gains thecodeattribute the API sends, the deprecated Feign calls are replaced, the README table of contents, heading level and sample output are corrected, the two Lombok delegate getters are named after what they return, the unusedpolyglot-groovybuild extension is removed, and the open Dependabot alert (Guava) is fixed. No public symbol is renamed or removed; the version number is unchanged (0.3.0).Changes
TestContext.get()/post(),MockIpdataServer.handleBulk(),MappingTest): on a JVM whose default charset is not UTF-8 (JDK 11 under a POSIX locale) 11 tests failed on emoji flags and currency symbols; the suite now passes under any locale.TestContext.post()also sends its body as UTF-8, and the mock server usesStandardCharsets.UTF_8for its response bytes and query-string decoding.languages[].codeis mapped: newLanguage.code()(nullwhen the API does not send it). Before, the lenient client mapper dropped the value silently and the strict test mapper would have rejected a fixture carrying it. The test fixtures now carry the attribute and the newLanguageTestcovers the full model,getFields(ip, IpdataField.LANGUAGES), the absent value and the strict mapper.Ipv6SafeClientusesRequest.create(..., RequestTemplate), which also keeps theRequestTemplateon the rebuilt request instead of dropping it;FieldDecoderandApiErrorDecodercallResponse.Body.asReader(StandardCharsets.UTF_8)instead of the deprecatedasReader()(UTF-8 underneath, so same behaviour). The main sources compile without deprecation warnings.#api(the#create-an-instanceanchor did not exist); "Bulk data" is a level-4 heading under API like its siblings; the "Basic Usage" sample output is regenerated from the model (it predatedcarrier,is_satellite,is_anycast,languages[].code,threat.is_vpn,threat.blocklistsandthreat.scores);languages[].codeis listed with the other recently added fields.IpdataServiceSupportdelegate getters:getApi()now returns the full-model client andgetSingleFieldClient()the single-field client. Both exist only as sources for Lombok's@Delegateon a package-private class and are now both private: no generated method and no public API changes..mvn/extensions.xmldeclaredio.takari.polyglot:polyglot-groovy0.4.3, which only matters for apom.groovy; it was downloaded on every fresh build and never used.CompactHashMap,CompactHashSetorMapMakerInternalMap) onguava33.4.8-jre; bumped to 33.7.2-jre, the first patched version. In the same commit Jackson becomes a direct dependency (jackson-databind2.22.4,jackson-annotations2.22, released once per minor line since 2.20): the code compiles against it directly but only received it throughfeign-jackson, which pinsjackson-databind2.13.4 (CVE-2022-42003; itsjackson-core2.13.4: CVE-2025-52999). The two calls these versions deprecate,ObjectMapper.setSerializationInclusion(Include)andCacheBuilder.expireAfterWrite(long, TimeUnit), are replaced by their equivalentssetDefaultPropertyInclusion(Include)andexpireAfterWrite(Duration).Not changed
ScoresandBlocklistkeep their plaingetX()getters: switching them to fluent accessors like the other models would rename public methods, and adding a second set of accessors would duplicate the API.Verification
All local commands with JDK 11 (
JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64) and the Maven wrapper (3.6.3); the CI command ismvn clean test post-integration-teston temurin 11.master(e574984), UTF-8 locale (LC_ALL=C.UTF-8):./mvnw -B -Dmaven.artifact.threads=1 clean test post-integration-test→Tests run: 80, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.master, POSIX locale (LC_ALL=POSIX LANG=POSIX, JVMfile.encoding = ANSI_X3.4-1968): same command →Tests run: 80, Failures: 11, Errors: 0, Skipped: 0, BUILD FAILURE (BulkTest4,FullModelTest4,MultipleFieldsSelectionTest2,NewResponseFieldsTest.modelRoundTripsTheNewFields1; all "JSON documents are different" on the mangled strings).7c91a05), UTF-8 locale:./mvnw -B -Dmaven.artifact.threads=1 -Dmaven.compiler.showDeprecation=true -Dmaven.compiler.showWarnings=true clean test post-integration-test→Tests run: 84, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, no javac deprecation warning (onmasterthe same flags reportRequest.createinIpv6SafeClientandasReader()inFieldDecoderandApiErrorDecoder)../mvnw -B -Dmaven.artifact.threads=1 clean test post-integration-test→Tests run: 84, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS../mvnw dependency:tree:jackson-databind:2.22.4,jackson-core:2.22.4,jackson-annotations:2.22,guava:33.7.2-jre; every other dependency unchanged.src/test/resources/io/ipdata/client/fixtures/1.1.1.1.jsondeserialized intoIpdataModeland serialized back with a snake_case mapper that keepsnullvalues, keys reordered to the API's documented order..github/workflows/ci.yml,pull_requesttrigger) for7c91a05: run Claude/verify all tests g2 mnu #12, https://github.com/ipdata/java/actions/runs/41642065187, jobtestsuccess on the first attempt (mvn clean test post-integration-test, temurin 11, ubuntu-latest). No follow-up commits were needed.🤖 Generated with Claude Code