Uploaded image for project: 'Pivot'
  1. Pivot
  2. PIVOT-967

JSONSerializer doesn't correctly handle CR characters

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 2.1, 2.0.5
    • Fix Version/s: 2.1, 2.0.5
    • Component/s: core-serialization
    • Labels:
      None
    • Environment:
      All, but particularly Windows

      Description

      The JSON standard (see JSON.org) defines specific escape sequences for the following characters: \b, \f, \n, \r, \t and \uxxxx for other control characters. But the encoding logic in JSONSerializer doesn't match this list (particularly missing \r and not encoding other control characters using \uxxxx). The "writeObject" method only recognizes \t and \n and does not encode any control character less than 0xFF. The "readString" method seems a bit weird also in its handling of control characters in the input.

      1. 967.patch
        2 kB
        Roger Whitcomb

        Activity

        Hide
        rwhitcomb Roger Whitcomb added a comment -

        This "967.patch" is my best guess as to the correct code. I have tried it in my fail case situation and everything appears to be correct. And I think all the comments I have added are correct as to intent and operation.

        Show
        rwhitcomb Roger Whitcomb added a comment - This "967.patch" is my best guess as to the correct code. I have tried it in my fail case situation and everything appears to be correct. And I think all the comments I have added are correct as to intent and operation.
        Hide
        smartini Sandro Martini added a comment -

        Hi Roger,
        to me the patch looks good ... did you tried even with our json files containing "\" (map.json, RowEditorDemo.json) ?
        We haven't json files with other control chars, so if you have some time could you commit a sample (could be useful even for future tests) maybe under tests subproject ?

        If all is ok, go with the commit ...
        Thanks a lot.

        Show
        smartini Sandro Martini added a comment - Hi Roger, to me the patch looks good ... did you tried even with our json files containing "\" (map.json, RowEditorDemo.json) ? We haven't json files with other control chars, so if you have some time could you commit a sample (could be useful even for future tests) maybe under tests subproject ? If all is ok, go with the commit ... Thanks a lot.
        Hide
        rwhitcomb Roger Whitcomb added a comment -

        I have updated the "map.json" file and JSONSerializerTest to test all these changes.

        Once more thing: JSONSerializer.writeObject couldn't correctly handle a java.util.Map passed in as the object, so I recognize this case now and put a MapAdapter around it.
        This also is tested in JSONSerializerTest.

        trunk:
        Sending core\src\org\apache\pivot\json\JSONSerializer.java
        Sending core\test\org\apache\pivot\json\test\JSONSerializerTest.java
        Sending core\test\org\apache\pivot\json\test\map.json
        Transmitting file data ...
        Committed revision 1675517.

        branches/2.0.x (merged with some Java 1.7 -> 1.6 changes):
        Sending .
        Sending core\src\org\apache\pivot\json\JSONSerializer.java
        Sending core\test\org\apache\pivot\json\test\JSONSerializerTest.java
        Sending core\test\org\apache\pivot\json\test\map.json
        Transmitting file data ...
        Committed revision 1675522.

        Show
        rwhitcomb Roger Whitcomb added a comment - I have updated the "map.json" file and JSONSerializerTest to test all these changes. Once more thing: JSONSerializer.writeObject couldn't correctly handle a java.util.Map passed in as the object, so I recognize this case now and put a MapAdapter around it. This also is tested in JSONSerializerTest. trunk: Sending core\src\org\apache\pivot\json\JSONSerializer.java Sending core\test\org\apache\pivot\json\test\JSONSerializerTest.java Sending core\test\org\apache\pivot\json\test\map.json Transmitting file data ... Committed revision 1675517. branches/2.0.x (merged with some Java 1.7 -> 1.6 changes): Sending . Sending core\src\org\apache\pivot\json\JSONSerializer.java Sending core\test\org\apache\pivot\json\test\JSONSerializerTest.java Sending core\test\org\apache\pivot\json\test\map.json Transmitting file data ... Committed revision 1675522.
        Hide
        smartini Sandro Martini added a comment -

        Hi Roger, thanks even for the additional tests. Bye

        Show
        smartini Sandro Martini added a comment - Hi Roger, thanks even for the additional tests. Bye

          People

          • Assignee:
            rwhitcomb Roger Whitcomb
            Reporter:
            rwhitcomb Roger Whitcomb
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development