Skip to content
This repository was archived by the owner on Aug 22, 2025. It is now read-only.

check escaper for the key barrier#9

Merged
voylex merged 1 commit intopaypal:masterfrom
voylex:escaper
Jun 28, 2021
Merged

check escaper for the key barrier#9
voylex merged 1 commit intopaypal:masterfrom
voylex:escaper

Conversation

@voylex
Copy link
Copy Markdown
Contributor

@voylex voylex commented Jun 19, 2021

this is related to a paypal existing bug, where the Java SDK can not deserialize certain response

where the customer input the given string in the request
"C/ Dinamarca "IPEX", 2"

And in order to send the string correctly, the SDK caller need to escape the special character,
the input string will have to be
"C/ Dinamarca \ \ \ "IPEX \ \ \ ", 2"

SDK will escape one backslash, and send the following to the API
"C/ Dinamarca \ "IPEX \ ", 2"

when the API successfully process the request, it will send
"C/ Dinamarca \ \ \ "IPEX \ \ \ ", 2 "

when we deserialize the JSON string, we are using the double quote as the ending indicator, However, it will fail if the responses contain a double quote that is supposed to be escaped, and thus only process "C/ Dinamarca \ \ \ " instead of the whole string.

@voylex
Copy link
Copy Markdown
Contributor Author

voylex commented Jun 21, 2021

the StringEscapeUtil.unescape method seems not working for the above specific case.
input:
"test \ \ \ ", "

output from the escape util:
"test ", "

expected:
"test \", "

the back slash will not stay if we are using unescape method

@voylex
Copy link
Copy Markdown
Contributor Author

voylex commented Jun 22, 2021

Again, tried with the StringEscapeUtil.unescape(), it seems not working, the json decoder will be confused about the first double quote it gets.

and to be clear:
input is
\ "test \ \ \ ", \ "

current fix will return

\ "test \ \ \ ", \ " -> looks like -> " test ", " to the api caller

with the jsonescaper, it will return as
\ "test \ ", \ "
from the json string perspective, it has to stop and process this part \ "test \ "

private static final char KEY_DELIMITER = ':';
private static final char PAIR_DELIMITER = ',';
private static final char KEY_BARRIER = '"';
private static final char KEY_ESCAPER = 92; // the backslash value
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using an int value here will work since they are interchangeable. However, considering the readability and consistency (with other char const value above), will it be better to use the char value? Or any specific reason to use int value instead?


Zoo zoo = new Json().decode(serializedZoo, Zoo.class);

assertEquals(zoo.name, "test \\\", ");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To test whether your parsing is correct, in this test it should include:

  1. create object
  2. serialize the object to string
  3. deserialize the string to object
  4. compare the objects in step 1 and 3, they should be identical.

@voylex
Copy link
Copy Markdown
Contributor Author

voylex commented Jun 25, 2021

@p1ngle hey Peng, just resolve the comments, can you check now? thanks !

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

One minor type

private static final char KEY_DELIMITER = ':';
private static final char PAIR_DELIMITER = ',';
private static final char KEY_BARRIER = '"';
private static final char KEY_ESCAPER = 92; // '/' the backslash value
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

backslash is \

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@voylex voylex merged commit b4a5b54 into paypal:master Jun 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant