starred/WireMock.Net-wiremock
1
0
Fork 0
mirror of https://github.com/wiremock/WireMock.Net.git synced 2026-08-01 01:38:35 +02:00
Code Issues 32 Packages Projects Releases 10 Wiki Activity

Why does JsonMatcher accept an invalid JSON string? #291

New Issue
Closed
opened 2025-12-29 15:19:47 +01:00 by adam · 8 comments
adam commented 2025-12-29 15:19:47 +01:00
Owner
Copy Link
Copy Source

Originally created by @lobsteropteryx on GitHub (Aug 6, 2020).

It appears that the JsonMatcher constructor accepts an invalid JSON string, and swallows the parsing exception.

Is there a valid use case for this behavior? As far as I can tell, it still won't properly match an invalid JSON string. For me, the preferred behavior would be to let the caller know right away that the string is not valid JSON.

I would be happy to submit a pull request with this change, but I wanted to make sure I'm not missing something.

Thank you much, this is a fantastic tool that we use every day!

Originally created by @lobsteropteryx on GitHub (Aug 6, 2020). It appears that the `JsonMatcher` constructor accepts an invalid JSON string, and [swallows](https://github.com/WireMock-Net/WireMock.Net/blob/8ae0abb023537605107c154dfebe8be3338df0bd/src/WireMock.Net/Matchers/JsonMatcher.cs#L114) the parsing exception. Is there a valid use case for this behavior? As far as I can tell, it still won't properly match an invalid JSON string. For me, the preferred behavior would be to let the caller know right away that the string is not valid JSON. I would be happy to submit a pull request with this change, but I wanted to make sure I'm not missing something. Thank you much, this is a fantastic tool that we use every day!
adam added the question label 2025-12-29 15:19:47 +01:00
adam closed this issue 2025-12-29 15:19:47 +01:00
adam commented 2025-12-29 15:19:49 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 7, 2020):

@lobsteropteryx
Thanks for the question.

I think that more places in the code do a try-catch-swallow. So it's best approach to detect all these places and use a new configuration setting bool? ThrowOnException.

What do you think?

@StefH commented on GitHub (Aug 7, 2020): @lobsteropteryx Thanks for the question. I think that more places in the code do a try-catch-swallow. So it's best approach to detect all these places and use a new configuration setting `bool? ThrowOnException`. What do you think?
adam commented 2025-12-29 15:19:50 +01:00
Author
Owner
Copy Link
Copy Source

@lobsteropteryx commented on GitHub (Aug 7, 2020):

I will have to dig a little deeper to see where else that might happen, but a boolean setting would be fine.

Another possibility might be to try to parse the string immediately when constructing the JsonMatcher. That would limit the blast radius, since I believe it's only created two places internally.

Is this something you would like help with?

@lobsteropteryx commented on GitHub (Aug 7, 2020): I will have to dig a little deeper to see where else that might happen, but a boolean setting would be fine. Another possibility might be to try to parse the string immediately when constructing the `JsonMatcher`. That would limit the blast radius, since I believe it's only created two places internally. Is this something you would like help with?
adam commented 2025-12-29 15:19:51 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 7, 2020):

I see your point.

This code https://github.com/WireMock-Net/WireMock.Net/blob/8ae0abb023537605107c154dfebe8be3338df0bd/src/WireMock.Net/Matchers/JsonMatcher.cs#L91 uses the Value, but if indeed the conversion from that object Value to JToken jtokenValue is done in the constructor, any errors are found earlier.

And if this code fails:

JToken jtokenInput = input is JToken tokenInput ? tokenInput : JObject.FromObject(input);

For exception I can use that configuration setting bool? ThrowOnException.

@StefH commented on GitHub (Aug 7, 2020): I see your point. This code https://github.com/WireMock-Net/WireMock.Net/blob/8ae0abb023537605107c154dfebe8be3338df0bd/src/WireMock.Net/Matchers/JsonMatcher.cs#L91 uses the `Value`, but if indeed the conversion from that `object Value` to `JToken jtokenValue` is done in the constructor, any errors are found earlier. And if this code fails: ``` c# JToken jtokenInput = input is JToken tokenInput ? tokenInput : JObject.FromObject(input); ``` For exception I can use that _configuration setting bool? ThrowOnException_.
adam commented 2025-12-29 15:19:51 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 7, 2020):

Parsing the code in the constructor : you can try this in preview MyGet : WireMock.Net.1.2.17-ci-13680.nupkg

@StefH commented on GitHub (Aug 7, 2020): Parsing the code in the constructor : you can try this in preview MyGet : `WireMock.Net.1.2.17-ci-13680.nupkg`
adam commented 2025-12-29 15:19:51 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 8, 2020):

Hello @lobsteropteryx,

A new preview version can be found on MyGet: WireMock.Net.1.2.17-ci-13684. Can you please test?

Set the ThrowExceptionWhenMatcherFails in the WireMockServerSettings to true.

https://github.com/WireMock-Net/WireMock.Net/pull/500

@StefH commented on GitHub (Aug 8, 2020): Hello @lobsteropteryx, A new preview version can be found on MyGet: `WireMock.Net.1.2.17-ci-13684`. Can you please test? Set the `ThrowExceptionWhenMatcherFails` in the `WireMockServerSettings` to true. https://github.com/WireMock-Net/WireMock.Net/pull/500
adam commented 2025-12-29 15:19:51 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 12, 2020):

@lobsteropteryx Did you have time to test this new setting?

@StefH commented on GitHub (Aug 12, 2020): @lobsteropteryx Did you have time to test this new setting?
adam commented 2025-12-29 15:19:51 +01:00
Author
Owner
Copy Link
Copy Source

@lobsteropteryx commented on GitHub (Aug 12, 2020):

Hi @StefH!

I've done some testing, and I like the new behavior--parsing the string immediately, and raising an exception for invalid JSON.

You might have an issue with the ThrowException parameter; as it stands right now, you'll still get the same exception raised whether ThrowException is true or false, since you're calling ConvertValueToJToken here: https://github.com/WireMock-Net/WireMock.Net/blob/JsonMatcherFixes/src/WireMock.Net/Matchers/JsonMatcher.cs#L98

I actually think this is correct, and perhaps the ThrowException parameter isn't needed; I can't think of a situation where I wouldn't want the exception to be thrown right away.

@lobsteropteryx commented on GitHub (Aug 12, 2020): Hi @StefH! I've done some testing, and I like the new behavior--parsing the string immediately, and raising an exception for invalid JSON. You might have an issue with the `ThrowException` parameter; as it stands right now, you'll still get the same exception raised whether `ThrowException` is true or false, since you're calling `ConvertValueToJToken` here: https://github.com/WireMock-Net/WireMock.Net/blob/JsonMatcherFixes/src/WireMock.Net/Matchers/JsonMatcher.cs#L98 I actually think this is correct, and perhaps the `ThrowException` parameter isn't needed; I can't think of a situation where I wouldn't want the exception to be thrown right away.
adam commented 2025-12-29 15:19:52 +01:00
Author
Owner
Copy Link
Copy Source

@StefH commented on GitHub (Aug 12, 2020):

I also think that throwing exception in constructor is correct. You get this exception immediately when adding a invalid mapping.

Later today I will merge PR and create new official release.

@StefH commented on GitHub (Aug 12, 2020): I also think that throwing exception in constructor is correct. You get this exception immediately when adding a invalid mapping. Later today I will merge PR and create new official release.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
bug
doc
doc
duplicate
feature
feature
invalid
pull-request
Mirrored from GitHub Pull Request
 question
wontfix
No labels question
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/WireMock.Net-wiremock#291