[SPY-57] view setKey and friends should be quoting/escaping Strings passed in Created: 01/Oct/11  Updated: 05/Apr/12  Due: 03/Oct/11  Resolved: 14/Dec/11

Status: Resolved
Project: Spymemcached Java Client
Component/s: library
Affects Version/s: 2.8.0-dp, 2.8.0-dp2
Fix Version/s: None
Security Level: Public

Type: Bug Priority: Minor
Reporter: Matt Ingenthron Assignee: Mike Wiederhold
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 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.

 Comments   
Comment by Mike Wiederhold [ 03/Oct/11 ]
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 + "\"";
    }
  }
Comment by Matt Ingenthron [ 03/Oct/11 ]
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
Comment by Mike Wiederhold [ 04/Oct/11 ]
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.
Comment by Mike Wiederhold [ 14/Dec/11 ]
I'm going to close this issue because it appears that this is taken care of by the underlying http library.
Generated at Wed Dec 17 16:23:07 CST 2014 using JIRA 5.2.4#845-sha1:c9f4cc41abe72fb236945343a1f485c2c844dac9.