Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
80e3bea
Update decoder resource-limit test fixtures
oschwald Sep 4, 2026
56300ff
Bound decoder work per decode operation
oschwald Sep 4, 2026
7623b3c
Avoid revalidating cached pointer targets
oschwald Sep 4, 2026
3eadcb5
Bound decoder collection preallocation
oschwald Sep 4, 2026
3884fa5
Avoid resizing small decoded maps
oschwald Sep 4, 2026
2eda46c
Bound decoded payload per decode operation
oschwald Sep 4, 2026
ee6d341
Reject oversized integer encodings
oschwald Sep 4, 2026
d78374f
Skip unknown pointer values by pointer width
oschwald Sep 4, 2026
d448fb1
Restore container depth after decode failures
oschwald Sep 4, 2026
0f74740
Charge cached pointer targets by logical cost
oschwald Sep 4, 2026
e23b67b
Document decoder resource limits
oschwald Sep 4, 2026
620924d
Reduce decoder allocation overhead
oschwald Sep 4, 2026
74ccdb5
Fix UTF-8 decoding across buffer chunks
oschwald Sep 4, 2026
92f710d
Speed up UTF-8 string decoding
oschwald Sep 5, 2026
d1e6dc4
Reject truncated value headers with InvalidDatabaseException
oschwald Sep 8, 2026
140b5a7
Skip unknown integer payloads without validating their widths
oschwald Sep 8, 2026
d87f0a5
Test packed costs against the decoder resource limits
oschwald Sep 8, 2026
791a050
Run pointer fan-out tests in a bounded subprocess
oschwald Sep 8, 2026
f314068
Cover payload and skipped-depth boundaries across cache states
oschwald Sep 8, 2026
553be73
Test payload boundaries with concurrent shared-cache lookups
oschwald Sep 8, 2026
a0ed65e
Remove redundant decoder cost accessors and size casts
oschwald Sep 8, 2026
0731430
Describe file bounds accurately in decoder errors
oschwald Sep 8, 2026
715e999
Identify payload boundary assertions by cache and attempt
oschwald Sep 8, 2026
e0fa785
Clarify decoder accounting and cache contracts
oschwald Sep 8, 2026
a326219
Consolidate release notes and document skipped-field behavior
oschwald Sep 8, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
CHANGELOG
=========

4.1.1
4.2.0
------------------

* Fixed decoding of data pointers with offsets of 2 GiB or greater. The
pointer payload was decoded into an `int`, so such offsets were
sign-extended to a negative value and rejected by `Buffer.position()`
with an `IllegalArgumentException`. Every record past the 2 GiB
boundary was unreachable in databases larger than 2 GiB, which have
been supported since 4.0.0.
* Fixed decoding of data pointers with offsets of 2 GiB or greater. Records
beyond that boundary could previously fail with `IllegalArgumentException`.
* Fixed skipping unknown four-byte pointers during typed decoding. Skipped
values that extend past the database are now rejected.
* Fixed UTF-8 decoding across buffer chunks. Malformed decoded strings and
truncated values now throw `InvalidDatabaseException`.
* Added decoder limits to prevent excessive CPU and memory use from crafted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointer-to-pointer rejection is a separate behavior change, not a resource limit — a custom database that decoded in 4.1.0 will now throw — and it appears only in UPGRADING.md. Worth its own bullet here: "The decoder now rejects a data-section pointer whose target is another pointer, which the MaxMind DB format does not permit."

Same for the collection capacity-hint change.

Everything else in this section checks out — I verified each exception-type claim against a real check (control byte, extended type byte, size header, pointer, double, float), and the version bump to 4.2.0 is right for a release adding behavior changes.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept pointer-to-pointer validation and capacity-hint details in UPGRADING.md, with an explicit changelog link to those changes. Consolidated overlapping release-note entries instead of adding more implementation details to the changelog.

Codex, on Greg’s behalf.

databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB
of encoded string and bytes payload per operation. Exceeding a limit throws
`InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for accounting,
decoded-value validation, and collection capacity-hint changes.
* Improved decoder performance and reduced per-lookup allocation, including
UTF-8 string decoding.

4.1.0 (2026-05-12)
------------------
Expand Down
29 changes: 29 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,32 @@
# Upgrading to 4.2.0

## Decoder Resource Limits

Version 4.2.0 limits the work and memory used by one record or metadata decode.
The decoder rejects an operation that exceeds any of these limits:

- 65,536 decoded or skipped values under the Java reader's work accounting
- 128 nested maps or arrays
- 2 MiB of encoded string and bytes payload materialized by the decoder

A decoded pointer costs one value in addition to its target's logical costs.
Cached targets retain their value count, container depth, and payload bytes, so
cache state does not change whether a decode exceeds a limit. Skipped pointers
count as one value and their targets remain unvisited. Skipped fields receive structural bounds and
resource checks, but their contents are not fully validated.

These limits leave a wide margin above MaxMind-produced records. A custom
database containing an unusually large record that decoded in an earlier
release may now throw `InvalidDatabaseException`. The limits are not
configurable in this release.

Initial collection capacity hints are capped at 128. Built-in collections grow
as needed. Custom `List` and `Map` types constructed through an `int` constructor
receive this capped hint instead of the full declared size.

When following a pointer, the decoder rejects targets that are themselves
pointers. Decoded integers wider than their format type permits are also rejected.

# Upgrading to 4.0.0

This guide covers the breaking changes introduced in version 4.0.0 and how to
Expand Down
12 changes: 0 additions & 12 deletions src/main/java/com/maxmind/db/Buffer.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package com.maxmind.db;

import java.nio.charset.CharacterCodingException;
import java.nio.charset.CharsetDecoder;

/**
* A generic buffer abstraction that supports sequential and random access
* to binary data. Implementations may be backed by a single {@link
Expand Down Expand Up @@ -96,13 +93,4 @@ sealed interface Buffer permits SingleBuffer, MultiBuffer {
* @return a duplicate buffer
*/
Buffer duplicate();

/**
* Decodes the buffer's content into a string using the given decoder.
*
* @param decoder the charset decoder
* @return the decoded string
* @throws CharacterCodingException if decoding fails
*/
String decode(CharsetDecoder decoder) throws CharacterCodingException;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging on this file generally rather than a specific line: limit() and limit(long) (lines 42 and 50) now have no production callers. decodeString was the only one that narrowed the limit, and Decoder uses its cached capacity as the sole bound; the only remaining users are MultiBufferTest.

Worth removing both. Buffer is package-private and sealed, so it's free — and it closes a real gap: if anything reintroduced a narrowed limit, checkDataSize would pass and the buffer would throw IndexOutOfBoundsException instead of InvalidDatabaseException.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving Buffer limits for a separate cleanup. They have no production callers that narrow the limit, and removing them would expand this PR across buffer implementations and their tests.

Codex, on Greg’s behalf.

30 changes: 28 additions & 2 deletions src/main/java/com/maxmind/db/DecodedValue.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,42 @@
package com.maxmind.db;

/**
* {@code DecodedValue} is a wrapper for the decoded value.
* An opaque decoded value and its resource costs, produced by {@link NodeCache.Loader}.
* Caches retain this instance unchanged for its original key. See {@link NodeCache}.
*/
public final class DecodedValue {
private static final int PAYLOAD_SHIFT = 8;
private static final int VALUES_SHIFT = 30;
private static final long PAYLOAD_MASK = (1L << 22) - 1;

// Final fields preserve their initialized values when a cache publishes this object.
final Object value;
private final long costs;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning about racy publication is sound, but nothing exercises it. MultiThreadedTest uses the default cache on MaxMind-DB-test-decoder.mmdb and asserts two field values; no test decodes a limit-relevant fixture concurrently through a shared CHMCache.

Not deterministically testable, but e.g. 256 threads against pointer-decoder-dos.mmdb with one shared CHMCache, asserting every thread sees the same rejection, would at least exercise the path.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a four-worker test using one Reader and CHMCache per database. It checks payload-boundary acceptance and rejection twice per worker. This exercises shared-cache accounting without claiming to prove final-field publication through a stress test.

Codex, on Greg’s behalf.


DecodedValue(Object value) {
DecodedValue(Object value, int values, long payloadBytes, int depth) {
this.value = value;
this.costs = ((long) values << VALUES_SHIFT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cost packing has zero headroom, no guard, and nothing ties it to Decoder's limits.

The constructor masks and validates nothing. Field widths live here; the limits they encode live in Decoder (MAX_VALUES, MAX_PAYLOAD_BYTES, MAX_DEPTH), both private, with no reference in either direction. Measured round-trip:

at limit (1<<21, 128)   : values=65536 payload=2097152 depth=128   ok
MAX_PAYLOAD_BYTES=1<<22 : payload -> 0,   values corrupted
MAX_DEPTH=256           : depth   -> 0,   payload corrupted

The depth case fails in the unsafe direction: a cached target recording depth cost 0 passes charge()'s depth > MAX_DEPTH - this.depth test at any current depth, which defeats the container-nesting bound that exists to keep the Java stack from overflowing — the exact DoS this PR closes.

Suggest range-checking (or asserting) the three arguments here, and deriving PAYLOAD_SHIFT/VALUES_SHIFT/PAYLOAD_MASK from named *_BITS constants so the layout is self-describing.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept the packed representation and tied its round-trip tests to the actual Decoder limits. Tests also cover zero, independent fields, and mixed costs. The current limits fit, so we are avoiding per-construction guards for states the decoder cannot currently produce.

Codex, on Greg’s behalf.

| (payloadBytes << PAYLOAD_SHIFT)
| depth;
}

Object value() {
return value;
}

static int values(long costs) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These read as absolute counts but every one returns a delta (the cost of one target, as computed by load()). valueCost() / payloadCost() / depthCost() would prevent the misreading.

Broader point on the packing itself: 620924d deliberately moved DecodedValue construction to cache boundaries only, so the sole construction site is now inside load() — once per cache miss, never per decoded value. The packing therefore saves ~8-16 bytes across a default 4096-entry CHMCache and replaces three field loads with one load plus three shifts, on a path that just did a full decode. There's no JMH harness in the repo and no measurement in either commit message. Three plain int/long fields would be strictly clearer and would eliminate the overflow coupling noted above at no measurable cost.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the redundant instance accessors, but kept the static names and packed representation. Costs are replayed on cache hits as well as measured on misses, and removing packing has no established performance-neutral result. Tests now couple the representation to the actual decoder limits.

Codex, on Greg’s behalf.

return (int) (costs >>> VALUES_SHIFT);
}

static long payloadBytes(long costs) {
return (costs >>> PAYLOAD_SHIFT) & PAYLOAD_MASK;
}

static int depth(long costs) {
return (int) (costs & 0xFF);
}

long costs() {
return this.costs;
}
}
Loading
Loading