You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by be...@google.com on 2010/04/15 04:40:19 UTC

Re: make trusted parameters override existing parameters (issue850046)

http://codereview.appspot.com/850046/diff/4001/5002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java
(right):

http://codereview.appspot.com/850046/diff/4001/5002#newcode408
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:408:
private void overrideParameters(List<Parameter> params) {
rename this parameter authParams?

http://codereview.appspot.com/850046/diff/4001/5002#newcode414
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:414:
for (int i = 0; i < params.size(); i++) {
Use "for (Parameter param : params)" instead of array index.

http://codereview.appspot.com/850046/diff/4001/5002#newcode418
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:418:
for (int i = 0; i < trustedParams.size(); i++) {
for (Parameter param : trustedParams)

http://codereview.appspot.com/850046/diff/4001/5002#newcode425
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthRequest.java:425:
while (it.hasNext()) {
for (String key : paramMap.keySet())

http://codereview.appspot.com/850046/diff/4001/5001
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java
(right):

http://codereview.appspot.com/850046/diff/4001/5001#newcode1945
java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java:1945:
* 4) trusted parameter can override existing parameter.
3, not 4?

http://codereview.appspot.com/850046/diff/4001/5001#newcode1946
java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java:1946:
* @throws Exception
drop @throws

http://codereview.appspot.com/850046/diff/4001/5001#newcode1972
java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java:1972:
* @throws Exception
drop @throws, it's not important enough to put in a comment.

http://codereview.appspot.com/850046/diff/4001/5001#newcode2004
java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/OAuthRequestTest.java:2004:
HttpResponse response =
client.sendGet(FakeOAuthServiceProvider.RESOURCE_URL);
OK, so I know I said that whatever behavior is fine, but it would be
nicer for developers if we blocked them from setting "foo" as a trusted
parameter rather than just getting the signature wrong.  Let's modify
OAuthRequest to return an error if someone passes a trusted param that
is not oauth/opensocial/xoauth.

You can use the same error handling as is in sanitize().

throw responseParams.oauthRequestException(OAuthError.INVALID_REQUEST,
         "invalid parameter name " + name +      "invalid parameter name
" + name +
        ", applications may not override opensocial or oauth
parameters");	       ", applications may not override opensocial or
oauth parameters");

http://codereview.appspot.com/850046/show