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

Fix json-body not escaping special characters #2551

Merged
merged 6 commits into from
Jan 24, 2024

Conversation

G-Basak
Copy link
Contributor

@G-Basak G-Basak commented Dec 31, 2023

This pull request fixes json-body not escaping special characters.

When Response-Body object is constructed from JSON file, it is stored as byte-array (plain text). So, the escape sequences are never replaced with their correct representation. They remain as is.
This creates problem when the JSON has special meaning after escaping. Such as the issue reported in : #2547

References

  • JsonNode of Jackson offers a method asText() which return the actual value of ValueNode. This will return a escaped string for a TextNode
  • Javadoc for JsonNode#asText()
  • Difference Between asText() and toString() in JsonNode
  • New member variable added to Body which stores the parsed JSON-nodes. This helps to avoid converting between escaped and non-escaped string many times.
  • The constructors of Body are modified to store the escaped JSON string when necessary.
  • Another Pull Requests(Adding a few unit tests for Body #2559) adds a few tests which is used in this PR to prove that the behavior is not changing for non-JSON body.

Submitter checklist

  • Recommended: Join WireMock Slack to get any help in #help-contributing or a project-specific channel like #wiremock-java
  • The PR request is well described and justified, including the body and the references
  • The PR title represents the desired changelog entry
  • The repository's code style is followed (see the contributing guide)
  • Test coverage that demonstrates that the change works as expected
  • For new features, there's necessary documentation in this pull request or in a subsequent PR to wiremock.org

@G-Basak G-Basak mentioned this pull request Jan 8, 2024
6 tasks
@G-Basak G-Basak changed the title Fix json-body not escaped properly Fix json-body not escaping special charecters Jan 8, 2024
@G-Basak G-Basak changed the title Fix json-body not escaping special charecters Fix json-body not escaping special characters Jan 8, 2024
@G-Basak G-Basak marked this pull request as ready for review January 8, 2024 21:15
@G-Basak G-Basak requested a review from a team as a code owner January 8, 2024 21:15
}

@Test
public void test2() {
Copy link
Contributor

@leeturner leeturner Jan 17, 2024

Choose a reason for hiding this comment

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

Super small minor thing but I was wondering if we could add a more targeted name for this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is not really needed. I had it for my local testing. 😅

}

private byte[] base64Encode(String string) {
return Base64.getEncoder().encode(string.getBytes(StandardCharsets.UTF_8));
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already some encoding support in Wiremock here. Not sure if there is any value to using that here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reused existing encoding functions.

@leeturner
Copy link
Contributor

This looks great. Appreciate the comprehensive tests. Just a few small comments really. I was also thinking it might be nice to add a few examples with this kind of escaping to the website here - https://github.com/wiremock/wiremock.org

@G-Basak
Copy link
Contributor Author

G-Basak commented Jan 17, 2024

I was also thinking it might be nice to add a few examples with this kind of escaping to the website here - https://github.com/wiremock/wiremock.org

@leeturner thank you for the suggestion.
Actually I am not really adding anything new. These escape sequence are part of standard JSON syntax.

But if you still think that documentation is needed, let me know where in the doc this information will best fit.

@leeturner leeturner added the needs-tom Tom's Train Project :) label Jan 22, 2024
@leeturner
Copy link
Contributor

Thank you for the updates. We just need Tom to cast his eyes over it and then we can take it from there

@leeturner leeturner removed the needs-tom Tom's Train Project :) label Jan 23, 2024
@leeturner leeturner self-assigned this Jan 23, 2024
Copy link
Contributor

@leeturner leeturner left a comment

Choose a reason for hiding this comment

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

Looks great. Really appreciate your contribution

@leeturner leeturner merged commit a48cc1d into wiremock:master Jan 24, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants