From 22c9a01ee845d2b92fcab6b6cb10ac6ff0eec52e Mon Sep 17 00:00:00 2001 From: cweiske Date: Tue, 28 Sep 2010 22:14:31 +0000 Subject: rewrite api/posts/delete to be more secure and add unit tests for it git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@769 b3834d28-1941-0410-a4f8-b48e95affb8f --- www/api/posts_delete.php | 63 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 18 deletions(-) (limited to 'www/api/posts_delete.php') diff --git a/www/api/posts_delete.php b/www/api/posts_delete.php index a63cc62..982b686 100644 --- a/www/api/posts_delete.php +++ b/www/api/posts_delete.php @@ -1,33 +1,60 @@ + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ // Force HTTP authentication first! $httpContentType = 'text/xml'; require_once 'httpauth.inc.php'; -/* Service creation: only useful services are created */ -$bookmarkservice =SemanticScuttle_Service_Factory::get('Bookmark'); - +$bs = SemanticScuttle_Service_Factory::get('Bookmark'); +$uId = $userservice->getCurrentUserId(); -// Note that del.icio.us only errors out if no URL was passed in; there's no error on attempting -// to delete a bookmark you don't have. // Error out if there's no address -if (is_null($_REQUEST['url'])) { +if (!isset($_REQUEST['url']) + || $_REQUEST['url'] == '' +) { $deleted = false; +} else if (!$bs->bookmarkExists($_REQUEST['url'], $uId)) { + //the user does not have such a bookmark + // Note that del.icio.us only errors out if no URL was passed in; + // there's no error on attempting to delete a bookmark you don't have. + // this sucks, and I don't care about being different but correct here. + header('HTTP/1.0 404 Not Found'); + $deleted = false; + } else { - $bookmark = $bookmarkservice->getBookmarkByAddress($_REQUEST['url']); - $bid = $bookmark['bId']; - $delete = $bookmarkservice->deleteBookmark($bid); - $deleted = true; + $bookmark = $bs->getBookmarkByAddress($_REQUEST['url'], false); + $bId = $bookmark['bId']; + $deleted = $bs->deleteBookmark($bId); + if (!$deleted) { + //something really went wrong + header('HTTP/1.0 500 Internal Server Error'); + } } // Set up the XML file and output the result. -echo '\r\n"; -echo ''; +echo '\r\n"; +echo ''; ?> \ No newline at end of file -- cgit v1.2.3 From 70c39a8eea7896271c0ad3f0c435ec06c64074d1 Mon Sep 17 00:00:00 2001 From: cweiske Date: Wed, 29 Sep 2010 20:49:14 +0000 Subject: delicious returns a proper error message when deleting non-existant items, which we do now, too git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@770 b3834d28-1941-0410-a4f8-b48e95affb8f --- doc/developers/api | 10 ++++++++++ tests/Api/PostsDeleteTest.php | 2 +- www/api/posts_delete.php | 14 +++++--------- 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 doc/developers/api (limited to 'www/api/posts_delete.php') diff --git a/doc/developers/api b/doc/developers/api new file mode 100644 index 0000000..efa05fe --- /dev/null +++ b/doc/developers/api @@ -0,0 +1,10 @@ +SemanticScuttle API +=================== + +SemanticScuttle tries to implement the delicious API v1 as closely as sensible. + +Where it makes sense and the delicious API just does things plainly wrong +(i.e. when returning a wrong status code on an error), we do it better. + +- http://www.delicious.com/help/api +- http://support.delicious.com/forum/comments.php?DiscussionID=5286&page=1 diff --git a/tests/Api/PostsDeleteTest.php b/tests/Api/PostsDeleteTest.php index 626746f..d9fb6cd 100644 --- a/tests/Api/PostsDeleteTest.php +++ b/tests/Api/PostsDeleteTest.php @@ -215,7 +215,7 @@ class Api_PostsDeleteTest extends TestBaseApi $this->assertTag( array( 'tag' => 'result', - 'attributes' => array('code' => 'something went wrong') + 'attributes' => array('code' => 'item not found') ), $res->getBody(), '', false diff --git a/www/api/posts_delete.php b/www/api/posts_delete.php index 982b686..03cc968 100644 --- a/www/api/posts_delete.php +++ b/www/api/posts_delete.php @@ -4,8 +4,6 @@ * The delicious API is implemented here. * * The delicious API behaves like that: - * - returns "done" even if the bookmark doesn't exist - * - we do it correctly * - does NOT allow the hash for the url parameter * - doesn't set the Content-Type to text/xml * - we do it correctly, too @@ -35,26 +33,24 @@ $uId = $userservice->getCurrentUserId(); if (!isset($_REQUEST['url']) || $_REQUEST['url'] == '' ) { - $deleted = false; + $msg = 'something went wrong'; } else if (!$bs->bookmarkExists($_REQUEST['url'], $uId)) { //the user does not have such a bookmark - // Note that del.icio.us only errors out if no URL was passed in; - // there's no error on attempting to delete a bookmark you don't have. - // this sucks, and I don't care about being different but correct here. header('HTTP/1.0 404 Not Found'); - $deleted = false; - + $msg = 'item not found'; } else { $bookmark = $bs->getBookmarkByAddress($_REQUEST['url'], false); $bId = $bookmark['bId']; $deleted = $bs->deleteBookmark($bId); + $msg = 'done'; if (!$deleted) { //something really went wrong header('HTTP/1.0 500 Internal Server Error'); + $msg = 'something really went wrong'; } } // Set up the XML file and output the result. echo '\r\n"; -echo ''; +echo ''; ?> \ No newline at end of file -- cgit v1.2.3 From 20ec8b4958dc2d6a4a08f9b0dcae27c90f0155ef Mon Sep 17 00:00:00 2001 From: cweiske Date: Wed, 29 Sep 2010 20:50:38 +0000 Subject: test for api/posts/update git-svn-id: https://semanticscuttle.svn.sourceforge.net/svnroot/semanticscuttle/trunk@771 b3834d28-1941-0410-a4f8-b48e95affb8f --- tests/AllTests.php | 1 + tests/Api/PostsUpdateTest.php | 135 ++++++++++++++++++++++++++++++++++++++++++ www/api/posts_delete.php | 1 + www/api/posts_update.php | 46 ++++++++++---- 4 files changed, 170 insertions(+), 13 deletions(-) create mode 100644 tests/Api/PostsUpdateTest.php (limited to 'www/api/posts_delete.php') diff --git a/tests/AllTests.php b/tests/AllTests.php index 61e1a57..799f43b 100644 --- a/tests/AllTests.php +++ b/tests/AllTests.php @@ -66,6 +66,7 @@ class AllTests extends PHPUnit_Framework_TestSuite $suite->addTestFile($tdir . '/UserTest.php'); $suite->addTestFile($tdir . '/Api/ExportCsvTest.php'); $suite->addTestFile($tdir . '/Api/PostsDeleteTest.php'); + $suite->addTestFile($tdir . '/Api/PostsUpdateTest.php'); return $suite; } diff --git a/tests/Api/PostsUpdateTest.php b/tests/Api/PostsUpdateTest.php new file mode 100644 index 0000000..c497a55 --- /dev/null +++ b/tests/Api/PostsUpdateTest.php @@ -0,0 +1,135 @@ + + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ + +require_once dirname(__FILE__) . '/../prepare.php'; +require_once 'HTTP/Request2.php'; + +if (!defined('PHPUnit_MAIN_METHOD')) { + define('PHPUnit_MAIN_METHOD', 'Api_PostsUpdateTest::main'); +} + +/** + * Unit tests for the SemanticScuttle last-update time API. + * + * @category Bookmarking + * @package SemanticScuttle + * @author Benjamin Huynh-Kim-Bang + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + */ +class Api_PostsUpdateTest extends TestBaseApi +{ + protected $urlPart = 'api/posts/update'; + + + + /** + * Used to run this test class standalone + * + * @return void + */ + public static function main() + { + require_once 'PHPUnit/TextUI/TestRunner.php'; + PHPUnit_TextUI_TestRunner::run( + new PHPUnit_Framework_TestSuite(__CLASS__) + ); + } + + + + /** + * Test if authentication is required when sending no auth data + */ + public function testAuthWithoutAuthData() + { + $req = $this->getRequest(null, false); + $res = $req->send(); + $this->assertEquals(401, $res->getStatus()); + } + + + + /** + * Test if authentication is required when sending wrong user data + + */ + public function testAuthWrongCredentials() + { + $req = $this->getRequest(null, false); + $req->setAuth('user', 'password', HTTP_Request2::AUTH_BASIC); + $res = $req->send(); + $this->assertEquals(401, $res->getStatus()); + } + + + + /** + * See if posts/update behaves correct if there is one bookmark + */ + public function testPostUpdateOneBookmark() + { + $this->bs->deleteAll(); + + list($req, $uId) = $this->getAuthRequest(); + $bId = $this->addBookmark( + $uId, 'http://example.org/tag1', 0, + array('unittest', 'tag1') + ); + + $data = $this->bs->getBookmarks(0, null, $uId); + $this->assertEquals(1, $data['total']); + $bookmark = $data['bookmarks'][0]; + + //send request + $res = $req->send(); + + $this->assertEquals(200, $res->getStatus()); + //verify MIME content type + $this->assertEquals( + 'text/xml; charset=utf-8', + $res->getHeader('content-type') + ); + + //verify xml + $this->assertTag( + array( + 'tag' => 'update', + 'attributes' => array( + 'inboxnew' => '0' + ) + ), + $res->getBody(), + '', false + ); + //check time + $xml = simplexml_load_string($res->getBody()); + $this->assertTrue(isset($xml['time'])); + $this->assertEquals( + strtotime($bookmark['bDatetime']), + strtotime( + (string)$xml['time'] + ) + ); + } + +} + +if (PHPUnit_MAIN_METHOD == 'Api_PostsUpdateTest::main') { + Api_PostsUpdateTest::main(); +} +?> \ No newline at end of file diff --git a/www/api/posts_delete.php b/www/api/posts_delete.php index 03cc968..69b2429 100644 --- a/www/api/posts_delete.php +++ b/www/api/posts_delete.php @@ -19,6 +19,7 @@ * @author Eric Dane * @license GPL http://www.gnu.org/licenses/gpl.html * @link http://sourceforge.net/projects/semanticscuttle + * @link http://www.delicious.com/help/api */ // Force HTTP authentication first! diff --git a/www/api/posts_update.php b/www/api/posts_update.php index 4aeedc3..4b080e2 100644 --- a/www/api/posts_update.php +++ b/www/api/posts_update.php @@ -1,24 +1,44 @@ + * @author Christian Weiske + * @author Eric Dane + * @license GPL http://www.gnu.org/licenses/gpl.html + * @link http://sourceforge.net/projects/semanticscuttle + * @link http://www.delicious.com/help/api + */ // Force HTTP authentication first! $httpContentType = 'text/xml'; require_once 'httpauth.inc.php'; -/* Service creation: only useful services are created */ -$bookmarkservice =SemanticScuttle_Service_Factory::get('Bookmark'); - - -// Get the posts relevant to the passed-in variables. -$bookmarks =& $bookmarkservice->getBookmarks(0, 1, $userservice->getCurrentUserId()); +$bs = SemanticScuttle_Service_Factory::get('Bookmark'); +$bookmarks = $bs->getBookmarks(0, 1, $userservice->getCurrentUserId()); // Set up the XML file and output all the tags. -echo '\r\n"; -foreach($bookmarks['bookmarks'] as $row) { - echo ''; +echo '\r\n"; +//foreach is used in case there are no bookmarks +foreach ($bookmarks['bookmarks'] as $row) { + echo ''; } ?> \ No newline at end of file -- cgit v1.2.3