Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add protection against StackOverflowError in JsonValueWriter #44627

Open
wants to merge 1 commit into
base: 3.4.x
Choose a base branch
from

Conversation

nosan
Copy link
Contributor

@nosan nosan commented Mar 6, 2025

Related to #44502

By default, Jackson uses a nesting depth of 1000.

Jackson:

	@Test
	void jackson() throws JsonProcessingException {
		List<Object> list = new ArrayList<>();
		list.add(list);
		new ObjectMapper().writeValueAsString(list);
	}
Document nesting depth (1001) exceeds the maximum allowed (1000, from `StreamWriteConstraints.getMaxNestingDepth()`) (through reference chain: java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->java.util.ArrayList[0]->

If you configure Jackson with a nesting depth greater than 1000 (Integer.MAX_VALUE), a StackOverflowError will also be thrown.

Jackson nestingDepth=5000

	@Test
	void jackson() throws JsonProcessingException {
		StreamWriteConstraints
			.overrideDefaultStreamWriteConstraints(StreamWriteConstraints.builder().maxNestingDepth(5000).build());
		List<Object> list = new ArrayList<>();
		list.add(list);
		ObjectMapper objectMapper = new ObjectMapper();
		objectMapper.writeValueAsString(list);
	}
java.lang.StackOverflowError
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:69)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)

Gson:

@Test
void gson() {
		List<Object> list = new ArrayList<>();
		list.add(list);
		new Gson().toJson(list);
}

java.lang.StackOverflowError
	at com.google.gson.internal.$Gson$Types.equals($Gson$Types.java:175)
	at com.google.gson.reflect.TypeToken.equals(TypeToken.java:346)
	at java.base/java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:940)
	at com.google.gson.Gson.getAdapter(Gson.java:600)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:57)
	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:99)
	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:59)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:73)
	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.write(CollectionTypeAdapterFactory.java:99)

Jsonb:

@Test
void jsonb() {
  List<Object> list = new ArrayList<>();
  list.add(list);
  JsonbBuilder.create().toJson(list);
}
SEVERE: Generating incomplete JSON

java.lang.StackOverflowError
	at java.base/java.lang.Class.reflectionData(Class.java:3214)
	at java.base/java.lang.Class.getInterfaces(Class.java:1144)
	at java.base/java.lang.Class.getInterfaces(Class.java:1140)
	at org.eclipse.yasson.internal.ComponentMatcher.searchComponentBinding(ComponentMatcher.java:210)
	at org.eclipse.yasson.internal.ComponentMatcher.getSerializerBinding(ComponentMatcher.java:145)
	at org.eclipse.yasson.internal.serializer.SerializationModelCreator.userSerializer(SerializationModelCreator.java:418)
	at org.eclipse.yasson.internal.serializer.SerializationModelCreator.serializerChainInternal(SerializationModelCreator.java:149)
	at org.eclipse.yasson.internal.serializer.SerializationModelCreator.serializerChain(SerializationModelCreator.java:133)
	at org.eclipse.yasson.internal.serializer.SerializationModelCreator.serializerChain(SerializationModelCreator.java:91)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 6, 2025
@nosan nosan force-pushed the 44502-stackoverflow-protection branch 2 times, most recently from b3f8244 to ea08a97 Compare March 6, 2025 15:21
@nosan
Copy link
Contributor Author

nosan commented Mar 6, 2025

Oh... with private static final int MAX_NESTING_DEPTH = 1000, the tests fail on CI: 😞

Expecting actual throwable to be an instance of:
  java.lang.IllegalStateException
but was:
  java.lang.StackOverflowError
	at org.springframework.boot.json.JsonWriter$MemberPath.toString(JsonWriter.java:816)
	at org.springframework.boot.json.JsonWriter$MemberPath.toString(JsonWriter.java:816)
	at org.springframework.boot.json.JsonWriter$MemberPath.toString(JsonWriter.java:816)

This commit adds validation for the maximum JSON nesting depth in
the JsonValueWriter. This helps prevent StackOverflowError that can
potentially occur due to excessive recursion when dealing with deeply
nested JSON structures.

Signed-off-by: Dmytro Nosan <[email protected]>
@nosan nosan force-pushed the 44502-stackoverflow-protection branch from 6849356 to 1777be4 Compare March 6, 2025 16:27
@@ -46,8 +46,12 @@
*/
class JsonValueWriter {

private static final int DEFAULT_MAX_NESTING_DEPTH = 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering if such a deep nesting level is necessary, as JsonWriter is primarily used for StructuredLogging, and such depth seems practically impossible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be configurable? If user decides they want more, they should be able to get more. Default level should be maybe lower, something like 32. Note that this is not only a stack-overflow protection, but also protection against overflowing the storage space for logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've prototyped some changes main...nosan:44502-json-writer-configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants