Skip to content

fix: When parameter "value" is json string, JsonParser should convert json string to Object directly. #4003

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

Merged
merged 1 commit into from
Aug 6, 2025

Conversation

bourne7
Copy link
Contributor

@bourne7 bourne7 commented Aug 5, 2025

when parameter "value" is json string:

{
    "name": "foo"
}

current code will convert it to json string with escaped quotes:

{\\"name\\":\\"foo\\"}

following code will fail to parse it from escaped json string to object.

@sobychacko
Copy link
Contributor

@bourne7 Could you add a unit test to verify these changes? in JsonParserTests.

@bourne7
Copy link
Contributor Author

bourne7 commented Aug 6, 2025

sure

@bourne7
Copy link
Contributor Author

bourne7 commented Aug 6, 2025

sy, i find that this problem has been fixed in this commit:

18ae16e Ahmed Maruf 81756975+ohmaruf@users.noreply.github.com on 2025-05-27 at 16:56
fix(json): prevent double-serialization of already valid JSON strings in JsonParser.toJson

@bourne7 bourne7 force-pushed the bourne7/dev branch 2 times, most recently from 5b10240 to b7fd600 Compare August 6, 2025 09:30
@bourne7
Copy link
Contributor Author

bourne7 commented Aug 6, 2025

Additional improvements:

modification on 05-28 added isValidJson method, it actually attempts deserialization to determine if the string is JSON type. In fact, this could be optimized - change it to immediately return the object if successful, avoiding double deserialization.

In most cases, Model returns a JSON string, so this optimization can eliminate one deserialization step in the majority of scenarios.

@sobychacko
Copy link
Contributor

We still need a unit test to validate the changes.

… when parsing Model response

Signed-off-by: bourne7 <lawrencepbr@gmail.com>
@bourne7
Copy link
Contributor Author

bourne7 commented Aug 6, 2025

@sobychacko unittest added ;)

@YunKuiLu

This comment was marked as outdated.

@sobychacko sobychacko added this to the 1.1.0.M1 milestone Aug 6, 2025
@sobychacko sobychacko merged commit 0715af2 into spring-projects:main Aug 6, 2025
2 checks passed
spring-builds pushed a commit that referenced this pull request Aug 6, 2025
… when parsing Model response (#4003)

Fixes #4003

Signed-off-by: bourne7 <lawrencepbr@gmail.com>
(cherry picked from commit 0715af2)
juntae6942 pushed a commit to juntae6942/spring-ai that referenced this pull request Aug 7, 2025
… when parsing Model response (spring-projects#4003)

Fixes spring-projects#4003

Auto-cherry-pick to 1.0.x

Signed-off-by: bourne7 <lawrencepbr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants