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);
}
-
}