Spymemcached Java Client

view setKey and friends should be quoting/escaping Strings passed in

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.8.0-dp, 2.8.0-dp2
  • Fix Version/s: None
  • Component/s: library
  • Security Level: Public
  • Labels:
    None

Description

The current API forces the user to quote and encode the values that they're passing as parameters to the view. This should be simpler for the user, but it is not.

For instance, if a user wanted the string asd"asd as their couch "key" start then they'd have to enter it "\"asd\"asd\"", which is a big burden on the developer.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mike Wiederhold added a comment -
This was filed incorrectly. On String arguments for query parameters I automatically add quotes.

private String getArg(String key, Object value) {
    // Special case
    if (key.equals(STARTKEYDOCID)) {
      return key + "=" + value;
    }
    if (value instanceof Boolean) {
      return key + "=" + ((Boolean) value).toString();
    } else if (value instanceof Integer) {
      return key + "=" + ((Integer) value).toString();
    } else if (value instanceof Stale) {
      return key + "=" + ((Stale) value).toString();
    } else {
      return key + "=\"" + value + "\"";
    }
  }
Show
Mike Wiederhold added a comment - This was filed incorrectly. On String arguments for query parameters I automatically add quotes. private String getArg(String key, Object value) {     // Special case     if (key.equals(STARTKEYDOCID)) {       return key + "=" + value;     }     if (value instanceof Boolean) {       return key + "=" + ((Boolean) value).toString();     } else if (value instanceof Integer) {       return key + "=" + ((Integer) value).toString();     } else if (value instanceof Stale) {       return key + "=" + ((Stale) value).toString();     } else {       return key + "=\"" + value + "\"";     }   }
Hide
Permalink
Matt Ingenthron added a comment - - edited
This needs to url encode the string, not just quote it. it will be an API change, but it's the right thing to do. We also need to document that we're expecting the string to be in the platform's default character encoding, just to match what is commonly done in the JDK[1].

For example, someone could call setKey() with the following valid JSON:
["val1","val2"]
And I think we just need to verify that we're converting it to URL encoded before sending the HTTP GET to the sserver:
%5B%22val1%22%2C%22val2%22%5D

The Java method call itself would look like: setKey("[\"val1\", \"val2\"]");

Or, more likely, they're using a JSON library to generate the string that's getting passed in to setKey(). In fact, our test should do that.

I think this is the right behavior for setKey(String key) at the moment. We may later want to add a setRawKey() or a setKey(String key, CouchbaseClient.encoding encoding), where that second param is some kind of encoding scheme we come up with if there's not a good standard one to use. That does bring up that we should probably have setKey() that takes a String and a Charset argument, since they may not supply UTF-8.

[1]: http://download.oracle.com/javase/6/docs/api/java/net/URLEncoder.html
Show
Matt Ingenthron added a comment - - edited This needs to url encode the string, not just quote it. it will be an API change, but it's the right thing to do. We also need to document that we're expecting the string to be in the platform's default character encoding, just to match what is commonly done in the JDK[1]. For example, someone could call setKey() with the following valid JSON: ["val1","val2"] And I think we just need to verify that we're converting it to URL encoded before sending the HTTP GET to the sserver: %5B%22val1%22%2C%22val2%22%5D The Java method call itself would look like: setKey("[\"val1\", \"val2\"]"); Or, more likely, they're using a JSON library to generate the string that's getting passed in to setKey(). In fact, our test should do that. I think this is the right behavior for setKey(String key) at the moment. We may later want to add a setRawKey() or a setKey(String key, CouchbaseClient.encoding encoding), where that second param is some kind of encoding scheme we come up with if there's not a good standard one to use. That does bring up that we should probably have setKey() that takes a String and a Charset argument, since they may not supply UTF-8. [1]: http://download.oracle.com/javase/6/docs/api/java/net/URLEncoder.html
Hide
Permalink
Mike Wiederhold added a comment -
Lowering priority because this isn't the cause of a bug. It might be nice to add this in the future, but at the moment it is not a mandatory fix. Also, URLEncoder is deprecated so if this is fixed we should probably use something else for the encoding.
Show
Mike Wiederhold added a comment - Lowering priority because this isn't the cause of a bug. It might be nice to add this in the future, but at the moment it is not a mandatory fix. Also, URLEncoder is deprecated so if this is fixed we should probably use something else for the encoding.
Hide
Permalink
Mike Wiederhold added a comment -
I'm going to close this issue because it appears that this is taken care of by the underlying http library.
Show
Mike Wiederhold added a comment - I'm going to close this issue because it appears that this is taken care of by the underlying http library.

People

  • Assignee:
    Mike Wiederhold
    Reporter:
    Matt Ingenthron
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: