Uploaded image for project: 'Couchbase Java Client'
  1. Couchbase Java Client
  2. JCBC-48

StringUtils.isJsonObject does not follow JSON spec which breaks Query

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Duplicate
    • Affects Version/s: 1.1dp
    • Fix Version/s: 1.1-dp4
    • Component/s: Core
    • Security Level: Public
    • Labels:
      None
    • Environment:
      all java

      Description

      net.spy.memcached.util.StringUtils does not implement isJsonObject to spec (http://www.json.org/) which breaks queries.

      • In particular, valid JSON numbers outside of the Java integer range are rejected and any String starting with "{" or "[" is permitted.
      • There is a size restriction for the key that is not checked (net.spy.memcached.util.validateKey)
      • There is a related issue, http://www.couchbase.com/issues/browse/JCBC-41 related to quoting string keys, but I think there is confusion with what a valid key is that should be cleared up first. If setKey takes unquoted strings, but the query string requires a JSON string it should be clear in the docs and setKey should wrap and escape unquoted strings which will avoid special handling. Quoted strings passed to setKey should be treated as JSON and internal quotes if unescaped should probably be treated as a runtime exception at query time. Unless explicitly set elsewhere, only numeric types should by assumed to be JSON numbers.

      Instead of isJsonObject and Object.toString, it may make sense to create isKey/toKey methods

      isJsonObject should either be fixed or removed from spymemcached library, but the 2.8.1 version should never be used.

      No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

        Hide
        bdw429s Brad Wood added a comment -

        I'm unclear on whether or not the issue is resolved. The last comment says it will be tracked with JCBC-41, but that ticket doesn't appear to really be related to this one.

        Show
        bdw429s Brad Wood added a comment - I'm unclear on whether or not the issue is resolved. The last comment says it will be tracked with JCBC-41 , but that ticket doesn't appear to really be related to this one.
        Hide
        daschl Michael Nitschinger added a comment -

        This will be tracked in JCBC-41!

        Show
        daschl Michael Nitschinger added a comment - This will be tracked in JCBC-41 !
        Hide
        TimSmith Tim Smith (Inactive) added a comment -

        Here's sample code that is difficult to get right:

        https://gist.github.com/3375697

        The setRangeStart() method needs to be called with an oddly-escaped JSON-encoded string:

        myQuery.setRangeStart("[\"Level+2\"]");

        This may be more relevant to JCBC-32.

        This comment is to remind that somewhere in here is an opportunity to help the developer easily create the right query string without getting confused by multiple levels of quoting.

        Thanks!

        Tim

        Show
        TimSmith Tim Smith (Inactive) added a comment - Here's sample code that is difficult to get right: https://gist.github.com/3375697 The setRangeStart() method needs to be called with an oddly-escaped JSON-encoded string: myQuery.setRangeStart(" [\"Level+2\"] "); This may be more relevant to JCBC-32 . This comment is to remind that somewhere in here is an opportunity to help the developer easily create the right query string without getting confused by multiple levels of quoting. Thanks! Tim
        Hide
        TimSmith Tim Smith (Inactive) added a comment -

        See related JCBC-32 .

        Show
        TimSmith Tim Smith (Inactive) added a comment - See related JCBC-32 .
        Hide
        ingenthr Matt Ingenthron added a comment -

        Excellent points, thanks for raising them. We're working to get an update here soon.

        Show
        ingenthr Matt Ingenthron added a comment - Excellent points, thanks for raising them. We're working to get an update here soon.
        Hide
        SteveC Steven Cooke added a comment -

        I think fixing this will also fix http://www.couchbase.com/issues/browse/JCBC-40

        Show
        SteveC Steven Cooke added a comment - I think fixing this will also fix http://www.couchbase.com/issues/browse/JCBC-40

          People

          • Assignee:
            daschl Michael Nitschinger
            Reporter:
            SteveC Steven Cooke
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Gerrit Reviews

              There are no open Gerrit changes