spring-projects / spring-projects/spring-ai

`JsonHelper.toJson(..., true)` treats strings with a JSON prefix as valid JSON

Open Beginner friendly
#6,847 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage
Dominant language
Java
Stars
9.5k
Forks
2.9k
Avg merge
1d 7h
Merged PRs (30d)
6

Description

Bug description

JsonHelper.toJson(object, true) returns a String input unchanged when the JSON validity check accepts it.
The validity check currently calls JsonMapper.readTree(input).
Spring AI's default mapper has DeserializationFeature.FAIL_ON_TRAILING_TOKENS disabled, so readTree(...) accepts inputs that start with one valid JSON token and then contain arbitrary trailing text.

That makes forwardIfValidJson value-dependent in surprising ways:

jsonHelper.toJson("22 console logs uploaded", true);  // returns 22 console logs uploaded
jsonHelper.toJson("true story", true);                             // returns true story
jsonHelper.toJson("null value", true);                             // returns null value
jsonHelper.toJson("Console log uploaded", true);        // returns "Console log uploaded"

Only the last result is valid JSON.
The first three are forwarded because their first token parses as a JSON number, boolean, or null, and the trailing text is ignored during validation.

This affects tool result conversion because DefaultToolCallResultConverter calls jsonHelper.toJson(result, true).
A String-returning tool can therefore emit either a JSON string literal or raw invalid JSON depending only on the first token of the returned text.

Environment

  • Spring AI: main at fd3fd6ec7 (2.0.2-SNAPSHOT)
  • Modules: spring-ai-commons, spring-ai-model
  • Classes: org.springframework.ai.util.JsonHelper, org.springframework.ai.tool.execution.DefaultToolCallResultConverter
  • Java: 17+
  • No vector store involved

Steps to reproduce

JsonHelper jsonHelper = new JsonHelper();

String numericPrefix = jsonHelper.toJson("22 console logs uploaded", true);
String booleanPrefix = jsonHelper.toJson("true story", true);
String wordPrefix = jsonHelper.toJson("Console log uploaded", true);

Actual result:

numericPrefix.equals("22 console logs uploaded");
booleanPrefix.equals("true story");
wordPrefix.equals("\"Console log uploaded\"");

The same issue is visible through the default tool result converter:

DefaultToolCallResultConverter converter = new DefaultToolCallResultConverter();

converter.convert("22 console logs uploaded", String.class); // 22 console logs uploaded
converter.convert("Console log uploaded", String.class);     // "Console log uploaded"

Expected behavior

When forwardIfValidJson is enabled, a string should be forwarded only if the entire string is valid JSON.
Strings that merely start with a valid JSON token should be serialized as JSON strings, consistently with other plain-text strings.

Expected result:

jsonHelper.toJson("22 console logs uploaded", true).equals("\"22 console logs uploaded\"");
jsonHelper.toJson("true story", true).equals("\"true story\"");
jsonHelper.toJson("Console log uploaded", true).equals("\"Console log uploaded\"");
jsonHelper.toJson("{\"status\":\"ok\"}", true).equals("{\"status\":\"ok\"}");
jsonHelper.toJson("[\"ok\"]", true).equals("[\"ok\"]");

Minimal Complete Reproducible example

Failing tests for JsonHelperTests:

@Test
void shouldNotForwardStringWithJsonNumberPrefixAsValidJson() {
	assertThat(this.jsonHelper.toJson("22 console logs uploaded", true)).isEqualTo("\"22 console logs uploaded\"");
}

@Test
void shouldNotForwardStringWithJsonBooleanPrefixAsValidJson() {
	assertThat(this.jsonHelper.toJson("true story", true)).isEqualTo("\"true story\"");
}

@Test
void shouldForwardStringOnlyWhenEntireInputIsValidJson() {
	assertThat(this.jsonHelper.toJson("{\"status\":\"ok\"}", true)).isEqualTo("{\"status\":\"ok\"}");
	assertThat(this.jsonHelper.toJson("[\"ok\"]", true)).isEqualTo("[\"ok\"]");
}

A companion regression test for DefaultToolCallResultConverterTests:

@Test
void convertStringWithJsonPrefixShouldReturnJsonString() {
	String result = this.converter.convert("22 console logs uploaded", String.class);

	assertThat(result).isEqualTo("\"22 console logs uploaded\"");
}

Suggested fix

Use a strict mapper for JsonHelper.isValidJson(...), or otherwise parse one JSON value and assert that there are no trailing tokens.

One small option is to keep the default mapper unchanged and enable trailing-token rejection only for this validation path:

private final JsonMapper strictJsonMapper;

public JsonHelper(JsonMapper jsonMapper) {
	Assert.notNull(jsonMapper, "jsonMapper cannot be null");
	this.jsonMapper = jsonMapper;
	this.strictJsonMapper = jsonMapper.rebuild()
		.enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS)
		.build();
}

private boolean isValidJson(String input) {
	try {
		this.strictJsonMapper.readTree(input);
		return true;
	}
	catch (JacksonException e) {
		return false;
	}
}

That preserves the existing forwardIfValidJson behavior for complete JSON documents while preventing JSON-prefix text from being forwarded as invalid JSON.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with JsonHelper.isValidJson(...) and the JsonHelperTests cases described in the issue, then inspect DefaultToolCallResultConverter and DefaultToolCallResultConverterTests for the integration path. Ensure validation rejects valid JSON prefixes with trailing text while forwarding complete JSON unchanged, and run both regression test groups.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.