Fix parsing JSON array of string with comma#5604
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
smortex
force-pushed
the
fix-json-array-with-comma
branch
from
35d1b6c to
942daa3
Compare
|
@smortex, to be honest, at first I was looking for a generic solution that can be parameterized. This one also has an effect in most cases, and that is the reason I am not sure it can be applied everywhere. Edit: Upps, I forgot the most important part! Thank you for the contribution! ;) |
|
Yeah, it might not be optimal. In fact I was expecting to find some kind of data structure that contained items in a list, and realized this was basically just a string and that there was code to manage quotes, so I tried to tweak it and see how the test suite broke with that change. To my surprise it did not break many tests, and the few broken ones made sense, telling me that this was either something doable, or an area that lacks a lot of tests 😁. I hoped for the former! Today, I downloaded the artifacts form CI (nice! I do not recall they existed) and installed the package on one of my systems and this indeed fix this issue. I really don't have the knowledge required to tell where we can/should put code to handle this specific issue if the Thank you! |
Whish I could, but I have to check and test all the cases too, as I nearly never touched/saw this part of the code yet. |
|
The right fix is to include the comma as in the forbidden_chars argument to str_repr_encode() when it is called from the Json parser. That should properly escape the comma and will not change the default behavior for the encode function |
When a JSON array is parsed, a string value containing a comma and no single or double quote is kept as-is and added to a list. The comma being a delimiter for list items, when the list is serialized to JSON again, that comma is not treated as a part of the original string, but as a separator: ``` ["a", "b", "c,d"] -> "a,b,c,d" ["a", "b", "c", "d"] <- "a,b,c,d" ``` Add the comma to the list of forbidden chars in a string to force quoting of the value, preventing this bug: ``` ["a", "b", "c,d"] -> "a,b,\"c,d\"" ["a", "b", "c,d"] <- "a,b,\"c,d\"" ``` Signed-off-by: Romain Tartière <[email protected]>
smortex
force-pushed
the
fix-json-array-with-comma
branch
from
942daa3 to
05e6d10
Compare
|
Thanks @bazsi! I did not checked previously, but as there is a single call to |
|
Thanks to both of you! I will review this within a few days. |
Signed-off-by: Hofi <[email protected]>
Signed-off-by: Hofi <[email protected]>
4 participants
When a JSON array is parsed, a string value containing a comma and no single or double quote is kept as-is and added to a list. The comma being a delimiter for list items, when the list is serialized to JSON again, that comma is not treated as a part of the original string, but as a separator:
Add the comma to the list of forbidden symbols in a string to force quoting of the value,preventing this bug: