Solr
  1. Solr
  2. SOLR-6445

Upgrade Noggit and allow flexible JSON input

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      Support single quotes and unquoted keys

      //all the following must be valid and equivalent
      {"id" :"mykey"}
      {'id':'mykey'}
      {id: "mykey"}
      
      1. SOLR-6445.patch
        0.5 kB
        Noble Paul

        Activity

        Hide
        Uwe Schindler added a comment -

        This violates JSON standard: "A string is a sequence of Unicode code points wrapped with quotation marks (U+0022)."
        See: http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf

        Show
        Uwe Schindler added a comment - This violates JSON standard: "A string is a sequence of Unicode code points wrapped with quotation marks (U+0022)." See: http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf
        Hide
        Jack Krupansky added a comment -

        +1 for violating the JSON standard! Okay, sure maybe we should have an option to require "strict" JSON, but it should default to "false".

        Could we support unquoted simple "name" values as well? Like:

        {id: my-key}
        

        And if people strenuously object, maybe we just need to have a "Solr JSON" (SJSON or SON - Solr Object Notation) format with the relaxed rules.

        Show
        Jack Krupansky added a comment - +1 for violating the JSON standard! Okay, sure maybe we should have an option to require "strict" JSON, but it should default to "false". Could we support unquoted simple "name" values as well? Like: {id: my-key} And if people strenuously object, maybe we just need to have a "Solr JSON" (SJSON or SON - Solr Object Notation) format with the relaxed rules.
        Hide
        Yonik Seeley added a comment - - edited

        The latest version of noggit (v0.6) already supports single quoted and unquoted strings.
        It's already in use in heliosearch.
        https://github.com/Heliosearch/heliosearch/commit/06e9392

        Show
        Yonik Seeley added a comment - - edited The latest version of noggit (v0.6) already supports single quoted and unquoted strings. It's already in use in heliosearch. https://github.com/Heliosearch/heliosearch/commit/06e9392
        Hide
        Noble Paul added a comment -

        This violates JSON standard: "A string is a sequence of Unicode code points wrapped with quotation marks (U+0022)."

        yes it does. We leave the choice to the users. We don't have to be the guardian angels of 'json standard'

        The latest version of noggit (v0.6) already supports single quoted and unquoted strings.

        Yes, I saw the code. But i need to set those flags explicitly , right?

        Show
        Noble Paul added a comment - This violates JSON standard: "A string is a sequence of Unicode code points wrapped with quotation marks (U+0022)." yes it does. We leave the choice to the users. We don't have to be the guardian angels of 'json standard' The latest version of noggit (v0.6) already supports single quoted and unquoted strings. Yes, I saw the code. But i need to set those flags explicitly , right?
        Hide
        Yonik Seeley added a comment -

        Yes, you can set the flags explicitly, but the defaults are already more permissive.

        And the JSON standard allows accepting more than JSON: http://www.ietf.org/rfc/rfc4627.txt

        JSON parser MUST accept all texts that conform to the JSON grammar.
        A JSON parser MAY accept non-JSON forms or extensions.

        Show
        Yonik Seeley added a comment - Yes, you can set the flags explicitly, but the defaults are already more permissive. And the JSON standard allows accepting more than JSON: http://www.ietf.org/rfc/rfc4627.txt JSON parser MUST accept all texts that conform to the JSON grammar. A JSON parser MAY accept non-JSON forms or extensions.
        Hide
        Uwe Schindler added a comment -

        Hi,
        sorry I was not arguing against this change! I just wanted to mention that "JSON" has a spec. I strongly agree that "keys" should be free to use no quotes at all, because that is what you know from ECMA Script. ECMA Script also allows single quotes. If we allow that, of course single quotes must be escaped in input data.

        Interestingly at the same time when Noble Paul's issue was opened, in my mailbox the announcement for PHP 5.6 went in. Interestingly they do the opposite: Their parser json_decode was more flexible before, now its hardened to be more strict: http://php.net/releases/5_6_0.php (I don't know how many apps will break).

        In any case, we may accept more relax JSON - I have no problem with that, but when delivering JSON in the ResponseWriter we should be 100% according to spec (this includes also escaping forward slashes). Because the client may be a browser, and browsers only accept 100% valid JSON for security reasons if you parse with the official JSON API in newer browsers (if you use unsafe JSONP or eval() to parse, you should be killed).

        Show
        Uwe Schindler added a comment - Hi, sorry I was not arguing against this change! I just wanted to mention that "JSON" has a spec. I strongly agree that "keys" should be free to use no quotes at all, because that is what you know from ECMA Script. ECMA Script also allows single quotes. If we allow that, of course single quotes must be escaped in input data. Interestingly at the same time when Noble Paul 's issue was opened, in my mailbox the announcement for PHP 5.6 went in. Interestingly they do the opposite: Their parser json_decode was more flexible before, now its hardened to be more strict: http://php.net/releases/5_6_0.php (I don't know how many apps will break). In any case, we may accept more relax JSON - I have no problem with that, but when delivering JSON in the ResponseWriter we should be 100% according to spec (this includes also escaping forward slashes). Because the client may be a browser, and browsers only accept 100% valid JSON for security reasons if you parse with the official JSON API in newer browsers (if you use unsafe JSONP or eval() to parse, you should be killed).
        Hide
        Uwe Schindler added a comment -

        Could we support unquoted simple "name" values as well?

        I disagree. The proposal in this issue was to allow all "Javascript" syntax. But Javascript does not allow values to be without quotes. The reason is simple: You cannot guess the type from it (at least the javascript grammar cannot).

        One thing: We should (if relaxing) also allow trailing commas, like:

        {
          "key1": "value",
          "key2": "value", // <-- here
        }
        
        Show
        Uwe Schindler added a comment - Could we support unquoted simple "name" values as well? I disagree. The proposal in this issue was to allow all "Javascript" syntax. But Javascript does not allow values to be without quotes. The reason is simple: You cannot guess the type from it (at least the javascript grammar cannot). One thing: We should (if relaxing) also allow trailing commas, like: { "key1": "value", "key2": "value", // <-- here }
        Hide
        Noble Paul added a comment -

        but when delivering JSON in the ResponseWriter we should be 100% according to spec

        absolutely. We should always emit fully compliant JSON

        Show
        Noble Paul added a comment - but when delivering JSON in the ResponseWriter we should be 100% according to spec absolutely. We should always emit fully compliant JSON
        Hide
        Noble Paul added a comment -

        We should replace the noggit0.5 jar with this

        Show
        Noble Paul added a comment - We should replace the noggit0.5 jar with this
        Hide
        Yonik Seeley added a comment - - edited

        We should replace the noggit0.5 jar with this [ attachment ]

        We don't check in jars any more... I think all that needs to be done to upgrade noggit is this?
        https://github.com/Heliosearch/heliosearch/commit/06e9392

        Show
        Yonik Seeley added a comment - - edited We should replace the noggit0.5 jar with this [ attachment ] We don't check in jars any more... I think all that needs to be done to upgrade noggit is this? https://github.com/Heliosearch/heliosearch/commit/06e9392
        Hide
        Noble Paul added a comment -

        updating noggit version to 0.6

        Show
        Noble Paul added a comment - updating noggit version to 0.6
        Hide
        ASF subversion and git services added a comment -

        Commit 1621934 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1621934 ]

        Upgrade Noggit and allow flexible JSON input SOLR-6445

        Show
        ASF subversion and git services added a comment - Commit 1621934 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1621934 ] Upgrade Noggit and allow flexible JSON input SOLR-6445
        Hide
        ASF subversion and git services added a comment -

        Commit 1621936 from Noble Paul in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1621936 ]

        Upgrade Noggit and allow flexible JSON input SOLR-6445

        Show
        ASF subversion and git services added a comment - Commit 1621936 from Noble Paul in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1621936 ] Upgrade Noggit and allow flexible JSON input SOLR-6445
        Hide
        ASF subversion and git services added a comment -

        Commit 1621943 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1621943 ]

        Upgrade Noggit and allow flexible JSON input SOLR-6445

        Show
        ASF subversion and git services added a comment - Commit 1621943 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1621943 ] Upgrade Noggit and allow flexible JSON input SOLR-6445
        Hide
        ASF subversion and git services added a comment -

        Commit 1621944 from Noble Paul in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1621944 ]

        Upgrade Noggit and allow flexible JSON input SOLR-6445

        Show
        ASF subversion and git services added a comment - Commit 1621944 from Noble Paul in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1621944 ] Upgrade Noggit and allow flexible JSON input SOLR-6445
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

          • Assignee:
            Noble Paul
            Reporter:
            Noble Paul
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development