[ziyziy89]

FrequentLongsSketch.getInstance(String) builds the hash map from the length and numActive tokens in the string without checking either against the map. The probe loops in ReversePurgeLongHashMap (adjustOrPutValue, hashProbe) only stop at an empty slot, and the only guard is an assert, so once every slot is taken a probe for a new key spins forever. getInstance("1,10,3,0,3,0,3,2,1,1,2,1,3,1") never returns, and an empty image that claims a map length of 2 ("1,10,3,5,0,0,0,2") deserializes fine but hangs on the third distinct update(), because curMapCap is computed from the 8-slot minimum while the map only has 2 slots. With assertions off this is an unbounded CPU loop from a ~30 byte input in any service that deserializes sketch strings it receives.

Reject a map length below 1 << LG_MIN_MAP_SIZE and a numActive outside [0, capacity] in getInstance(String), next to the existing "Possible Corruption" checks. serializeToString() never produces either, so valid images are unaffected; the MemorySegment path already goes through update() and resizes/purges, so it does not need the same check. Added two tests to LongsSketchTest (with a timeout so a regression fails instead of hanging the suite); mvn clean verify passes.

[leerho]

Thanks for this. Both new checks are correct, and they also fix the mismatch where curMapCap was computed from the 8-slot minimum while the real map was smaller. I ran the frequencies tests on this branch: 63 tests, 0 failures.

Before we merge, please make the size validation in both deserialization paths match what datasketches-cpp already does in frequent_items_sketch::check_size():

1. getInstance(String): also reject a current map size larger than the maximum map size (lgCur > lgMax), with the same "Possible Corruption" style as your new checks.

2. getInstance(MemorySegment): apply the same two checks to the lgMaxMapSize and lgCurMapSize preamble bytes, before the sketch is constructed:

   • lgCurMapSize must be at least LG_MIN_MAP_SIZE
   • lgCurMapSize must not exceed lgMaxMapSize

   The binary path builds its map from these bytes before checking anything else, so they should be validated first.

Please add a test for each new check, using timeOut as you did for the existing tests.

With these, Java will reject exactly the images that C++ rejects. The serializers in both languages never produce these values, so valid images are unaffected.

[leerho]

Thanks @ziyziy89, this is good work, and the fix and tests are solid. We're converting this PR to a draft for a while because it overlaps with some related changes the PMC is coordinating. This isn't a reflection on your PR. Please do go ahead and push the changes from the review when you have time. We'll mark it ready again and merge it once the related work is in place, and we'll update you here.