Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/com/esotericsoftware/kryo/Kryo.java
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,7 @@ public void reset () {
depth = 0;
if (graphContext != null) graphContext.clear(2048);
classResolver.reset();
generics.reset();
if (references) {
referenceResolver.reset();
readObject = null;
Expand Down
9 changes: 6 additions & 3 deletions src/com/esotericsoftware/kryo/serializers/ReflectField.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ public void write (Output output, Object object) {
kryo.writeObject(output, value, serializer);
}
}
kryo.getGenerics().popGenericType();
} catch (IllegalAccessException ex) {
throw new KryoException("Error accessing field: " + name + " (" + object.getClass().getName() + ")", ex);
} catch (KryoException ex) {
Expand All @@ -102,6 +101,9 @@ public void write (Output output, Object object) {
KryoException ex = new KryoException(t);
ex.addTrace(name + " (" + object.getClass().getName() + ")");
throw ex;
} finally {
// Pop in a finally so an exception thrown by the nested write does not leave the generics stack unbalanced.
kryo.getGenerics().popGenericType();
}
}

Expand Down Expand Up @@ -134,8 +136,6 @@ public void read (Input input, Object object) {
else
value = kryo.readObject(input, concreteType, serializer);
}
kryo.getGenerics().popGenericType();

set(object, value);
} catch (IllegalAccessException ex) {
throw new KryoException("Error accessing field: " + name + " (" + fieldSerializer.type.getName() + ")", ex);
Expand All @@ -146,6 +146,9 @@ public void read (Input input, Object object) {
KryoException ex = new KryoException(t);
ex.addTrace(name + " (" + fieldSerializer.type.getName() + ")");
throw ex;
} finally {
// Pop in a finally so an exception thrown by the nested read does not leave the generics stack unbalanced.
kryo.getGenerics().popGenericType();
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/com/esotericsoftware/kryo/util/DefaultGenerics.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,19 @@ public int getGenericTypesSize () {
return genericTypesSize;
}

@Override
public void reset () {
// Fast path: after a balanced (successful) serialization both stacks are already empty.
if (genericTypesSize == 0 && argumentsSize == 0) return;
// Slow path: a serializer threw before popping. Discard the leaked entries so the next serialization starts clean.
for (int i = 0; i < genericTypesSize; i++)
genericTypes[i] = null;
genericTypesSize = 0;
for (int i = 0; i < argumentsSize; i++)
arguments[i] = null;
argumentsSize = 0;
}

public String toString () {
StringBuilder buffer = new StringBuilder();
for (int i = 0; i < argumentsSize; i += 2) {
Expand Down
10 changes: 10 additions & 0 deletions src/com/esotericsoftware/kryo/util/Generics.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import static com.esotericsoftware.kryo.util.Util.*;

import com.esotericsoftware.kryo.Kryo;

import java.lang.reflect.Array;
import java.lang.reflect.GenericArrayType;
import java.lang.reflect.GenericDeclaration;
Expand Down Expand Up @@ -78,6 +80,14 @@ public interface Generics {
/** Returns the number of generic types currently tracked */
int getGenericTypesSize ();

/** Discards all tracked generic type information, returning to an empty state. This is called by {@link Kryo#reset()} so a
* reused Kryo instance is not left with stale generics if a serializer throws an exception before a balancing
* {@link #popGenericType()} or {@link #popTypeVariables(int)}. Implementations should be cheap when there is nothing to
* discard, since this is called after every serialization and deserialization (when auto-reset is enabled). The default
* implementation does nothing. */
default void reset () {
}

/** Stores the type parameters for a class and, for parameters passed to super classes, the corresponding super class type
* parameters. */
class GenericsHierarchy {
Expand Down
4 changes: 4 additions & 0 deletions src/com/esotericsoftware/kryo/util/NoGenerics.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,8 @@ public Class resolveTypeVariable (TypeVariable typeVariable) {
public int getGenericTypesSize () {
return 0;
}

@Override
public void reset () {
}
}
156 changes: 156 additions & 0 deletions test/com/esotericsoftware/kryo/serializers/GenericsResetTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/* Copyright (c) 2008-2025, Nathan Sweet
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following
* conditions are met:
*
* - Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
* - Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided with the distribution.
* - Neither the name of Esoteric Software nor the names of its contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
* BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
* SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */

package com.esotericsoftware.kryo.serializers;

import static org.junit.jupiter.api.Assertions.*;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.KryoException;
import com.esotericsoftware.kryo.KryoTestCase;
import com.esotericsoftware.kryo.Serializer;
import com.esotericsoftware.kryo.io.Input;
import com.esotericsoftware.kryo.io.Output;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Test;

/** Verifies that an exception thrown while (de)serializing a generic field does not leak the generics stack and poison a reused
* Kryo instance.
* <p>
* See <a href="https://github.com/EsotericSoftware/kryo/issues/1281">#1281</a>: when a serializer threw between
* {@link com.esotericsoftware.kryo.util.Generics#pushGenericType pushGenericType} and the balancing
* {@link com.esotericsoftware.kryo.util.Generics#popGenericType popGenericType}, the leaked entry survived {@link Kryo#reset()}
* and caused a later, unrelated deserialization to fail with an {@link ArrayIndexOutOfBoundsException}. */
class GenericsResetTest extends KryoTestCase {

@Test
void testGenericsStackIsResetAfterDeserializationException () {
kryo.setReferences(false);
kryo.setRegistrationRequired(false);
kryo.register(Holder.class);
kryo.register(Item.class);
kryo.register(Payload.class, new ThrowOnReadSerializer());

// A Holder whose List<Item> element holds a Payload. Reading the Payload throws, after the generic type for the
// List<Item> field was pushed, leaking it onto the generics stack.
Holder holder = new Holder();
holder.items = new ArrayList();
Item item = new Item();
item.payload = new Payload();
holder.items.add(item);

Output output = new Output(512);
kryo.writeClassAndObject(output, holder);
byte[] failingBytes = output.toBytes();

// An unrelated, well-formed HashMap containing two non-empty nested maps. On a poisoned instance, reading the second
// nested map mistakes the leaked List generic for a Map generic and indexes [1], throwing AIOOBE.
byte[] validBytes = writeNestedMaps();

// The first deserialization fails inside the generic field.
assertThrows(KryoException.class, () -> kryo.readClassAndObject(new Input(failingBytes)));

// Kryo.reset() must have discarded the leaked generic type.
assertEquals(0, kryo.getGenerics().getGenericTypesSize());

// A subsequent unrelated deserialization on the same instance must succeed.
Object result = kryo.readClassAndObject(new Input(validBytes));
assertTrue(result instanceof Map);
assertEquals(2, ((Map)result).size());
}

@Test
void testGenericsStackIsResetAfterSerializationException () {
kryo.setReferences(false);
kryo.setRegistrationRequired(false);
kryo.register(Holder.class);
kryo.register(Item.class);
kryo.register(Payload.class, new ThrowOnWriteSerializer());

Holder holder = new Holder();
holder.items = new ArrayList();
Item item = new Item();
item.payload = new Payload();
holder.items.add(item);

// Writing the Payload throws, after the generic type for the List<Item> field was pushed.
assertThrows(KryoException.class, () -> kryo.writeClassAndObject(new Output(512), holder));

// Kryo.reset() must have discarded the leaked generic type.
assertEquals(0, kryo.getGenerics().getGenericTypesSize());

// A subsequent unrelated serialization/deserialization on the same instance must succeed.
byte[] validBytes = writeNestedMaps();
Object result = kryo.readClassAndObject(new Input(validBytes));
assertTrue(result instanceof Map);
assertEquals(2, ((Map)result).size());
}

/** A bare HashMap with two non-empty nested maps, mirroring the payload that triggers the AIOOBE on a poisoned instance. */
private byte[] writeNestedMaps () {
Map<String, Object> map = new HashMap();
Map<String, String> nested1 = new HashMap();
nested1.put("a", "1");
Map<String, String> nested2 = new HashMap();
nested2.put("b", "2");
map.put("k1", nested1);
map.put("k2", nested2);

Output output = new Output(512);
kryo.writeClassAndObject(output, map);
return output.toBytes();
}

public static class Holder {
public List<Item> items;
}

public static class Item {
public Object payload;
}

public static class Payload {
}

/** Writes nothing and always throws on read, simulating any read-time failure (eg a missing class). */
static class ThrowOnReadSerializer extends Serializer<Payload> {
public void write (Kryo kryo, Output output, Payload object) {
}

public Payload read (Kryo kryo, Input input, Class<? extends Payload> type) {
throw new RuntimeException("simulated read failure");
}
}

/** Always throws on write, simulating any write-time failure. */
static class ThrowOnWriteSerializer extends Serializer<Payload> {
public void write (Kryo kryo, Output output, Payload object) {
throw new RuntimeException("simulated write failure");
}

public Payload read (Kryo kryo, Input input, Class<? extends Payload> type) {
return new Payload();
}
}
}
Loading