Reviewers: chabotc, panjie,

Description:
Here are what I did in the patch:

1. there is a bug when signed fetch a get request.
postParams is null and this line of code will report a warning:
$msgParams = array_merge($msgParams, $postParams);

2. added an unittest for signed fetch post request with header
"Content-Type:application/json"

3. modified verification process in SigningFetcherTest.php.

Please review this at http://codereview.appspot.com/32108

Affected files:
  php/src/gadgets/SigningFetcher.php
  php/test/gadgets/SigningFetcherTest.php


Index: php/src/gadgets/SigningFetcher.php
===================================================================
--- php/src/gadgets/SigningFetcher.php  (revision 761149)
+++ php/src/gadgets/SigningFetcher.php  (working copy)
@@ -141,7 +141,7 @@
       }
       $msgParams = array();
       $msgParams = array_merge($msgParams, $queryParams);
-      if ($signBody) {
+      if ($signBody && isset($postParams)) {
         $msgParams = array_merge($msgParams, $postParams);
       }
       $this->addOpenSocialParams($msgParams, $request->getToken());
Index: php/test/gadgets/SigningFetcherTest.php
===================================================================
--- php/test/gadgets/SigningFetcherTest.php     (revision 761149)
+++ php/test/gadgets/SigningFetcherTest.php     (working copy)
@@ -101,15 +101,37 @@
$request->setToken(BasicSecurityToken::createFromValues('owner', 'viewer', 'app', 'domain', 'appUrl', '1', 'default'));
     $request->setPostBody('key=value&anotherkey=value');
     $this->signingFetcher->fetchRequest($request);
+    $this->verifySignedRequest($request);
+  }
+
+  /**
+   * Tests SigningFetcher->fetchRequest
+   */
+  public function testFetchRequestForJson() {
+    $request = new RemoteContentRequest('http://example.org/signed');
+    $request->setAuthType(RemoteContentRequest::$AUTH_SIGNED);
+ $request->setToken(BasicSecurityToken::createFromValues('owner', 'viewer', 'app', 'domain', 'appUrl', '1', 'default'));
+    $request->setPostBody('{key:value}');
+    $request->setHeaders('Content-Type:application/json');
+    $this->signingFetcher->fetchRequest($request);
+    $this->verifySignedRequest($request);
+  }

+  private function verifySignedRequest(RemoteContentRequest $request) {
     $url = parse_url($request->getUrl());
+    $query = array();
     parse_str($url['query'], $query);
-    parse_str($request->getPostBody(), $post);
+    $post = array();
+    $contentType = $request->getHeader('Content-Type');
+ if ((stripos($contentType, 'application/x-www-form-urlencoded') !== false || $contentType == null)) {
+      parse_str($request->getPostBody(), $post);
+    } else {
+ $this->assertEquals(sha1($request->getPostBody()), $query['oauth_body_hash']);
+    }
$oauthRequest = OAuthRequest::from_request($request->getMethod(), $request->getUrl(), array_merge($query, $post));
     $signature_method = new MockSignatureMethod();
$signature_valid = $signature_method->check_signature($oauthRequest, null, null, $query['oauth_signature']);
     $this->assertTrue($signature_valid);
   }
-
 }



Reply via email to