mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #19046 from owncloud/issue-18924-throw-error-on-invalid-page 

Throw an error when the page count or perPage setting is invalid
This commit is contained in:
Thomas Müller 2015-09-17 13:02:14 +02:00
parent c9397dffea 754850f473
commit 4cff2f1ab3
2 changed files with 96 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
apps/files_sharing/api/sharees.php
View File

@ -20,6 +20,7 @@
*/
namespace OCA\Files_Sharing\API;
use OCP\AppFramework\Http;
use OCP\Contacts\IManager;
use OCP\IGroup;
use OCP\IGroupManager;
@ -291,8 +292,15 @@ class Sharees {
public function search() {
$search = isset($_GET['search']) ? (string) $_GET['search'] : '';
$itemType = isset($_GET['itemType']) ? (string) $_GET['itemType'] : null;
$page = !empty($_GET['page']) ? max(1, (int) $_GET['page']) : 1;
$perPage = !empty($_GET['perPage']) ? max(1, (int) $_GET['perPage']) : 200;
$page = isset($_GET['page']) ? (int) $_GET['page'] : 1;
$perPage = isset($_GET['perPage']) ? (int) $_GET['perPage'] : 200;
if ($perPage <= 0) {
return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST, 'Invalid perPage argument');
}
if ($page <= 0) {
return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST, 'Invalid page');
}
$shareTypes = [
Share::SHARE_TYPE_USER,
@ -348,7 +356,7 @@ class Sharees {
protected function searchSharees($search, $itemType, array $shareTypes, $page, $perPage) {
// Verify arguments
if ($itemType === null) {
return new \OC_OCS_Result(null, 400, 'missing itemType');
return new \OC_OCS_Result(null, Http::STATUS_BAD_REQUEST, 'Missing itemType');
}
// Get users

112
apps/files_sharing/tests/api/shareestest.php
View File

@ -21,10 +21,9 @@
namespace OCA\Files_Sharing\Tests\API;
use Doctrine\DBAL\Connection;
use OC\Share\Constants;
use OCA\Files_Sharing\API\Sharees;
use OCA\Files_sharing\Tests\TestCase;
use OCP\AppFramework\Http;
use OCP\Share;
class ShareesTest extends TestCase {
@ -652,15 +651,6 @@ class ShareesTest extends TestCase {
], '', false, '', null, [0, 1], 1, 200, false],
// Test pagination
[[
'page' => 0,
], '', true, '', null, $allTypes, 1, 200, false],
[[
'page' => '0',
], '', true, '', null, $allTypes, 1, 200, false],
[[
'page' => -1,
], '', true, '', null, $allTypes, 1, 200, false],
[[
'page' => 1,
], '', true, '', null, $allTypes, 1, 200, false],
@ -669,15 +659,6 @@ class ShareesTest extends TestCase {
], '', true, '', null, $allTypes, 10, 200, false],
// Test perPage
[[
'perPage' => 0,
], '', true, '', null, $allTypes, 1, 200, false],
[[
'perPage' => '0',
], '', true, '', null, $allTypes, 1, 200, false],
[[
'perPage' => -1,
], '', true, '', null, $allTypes, 1, 1, false],
[[
'perPage' => 1,
], '', true, '', null, $allTypes, 1, 1, false],
@ -758,6 +739,75 @@ class ShareesTest extends TestCase {
$_GET = $oldGet;
}
public function dataSearchInvalid() {
return [
// Test invalid pagination
[[
'page' => 0,
], 'Invalid page'],
[[
'page' => '0',
], 'Invalid page'],
[[
'page' => -1,
], 'Invalid page'],
// Test invalid perPage
[[
'perPage' => 0,
], 'Invalid perPage argument'],
[[
'perPage' => '0',
], 'Invalid perPage argument'],
[[
'perPage' => -1,
], 'Invalid perPage argument'],
];
}
/**
* @dataProvider dataSearchInvalid
*
* @param array $getData
* @param string $message
*/
public function testSearchInvalid($getData, $message) {
$oldGet = $_GET;
$_GET = $getData;
$config = $this->getMockBuilder('OCP\IConfig')
->disableOriginalConstructor()
->getMock();
$config->expects($this->never())
->method('getAppValue');
$sharees = $this->getMockBuilder('\OCA\Files_Sharing\API\Sharees')
->setConstructorArgs([
$this->groupManager,
$this->userManager,
$this->contactsManager,
$config,
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
])
->setMethods(array('searchSharees', 'isRemoteSharingAllowed'))
->getMock();
$sharees->expects($this->never())
->method('searchSharees');
$sharees->expects($this->never())
->method('isRemoteSharingAllowed');
/** @var \PHPUnit_Framework_MockObject_MockObject|\OCA\Files_Sharing\API\Sharees $sharees */
$ocs = $sharees->search();
$this->assertInstanceOf('\OC_OCS_Result', $ocs);
$this->assertOCSError($ocs, $message);
$_GET = $oldGet;
}
public function dataIsRemoteSharingAllowed() {
return [
['file', true],
@ -940,13 +990,7 @@ class ShareesTest extends TestCase {
$ocs = $this->invokePrivate($this->sharees, 'searchSharees', ['', null, [], [], 0, 0, false]);
$this->assertInstanceOf('\OC_OCS_Result', $ocs);
$this->assertSame(400, $ocs->getStatusCode(), 'Expected status code 400');
$this->assertSame([], $ocs->getData(), 'Expected that no data is send');
$meta = $ocs->getMeta();
$this->assertNotEmpty($meta);
$this->assertArrayHasKey('message', $meta);
$this->assertSame('missing itemType', $meta['message']);
$this->assertOCSError($ocs, 'Missing itemType');
}
public function dataGetPaginationLink() {
@ -992,4 +1036,18 @@ class ShareesTest extends TestCase {
$this->assertEquals($expected, $this->invokePrivate($this->sharees, 'isV2'));
}
/**
* @param \OC_OCS_Result $ocs
* @param string $message
*/
protected function assertOCSError(\OC_OCS_Result $ocs, $message) {
$this->assertSame(Http::STATUS_BAD_REQUEST, $ocs->getStatusCode(), 'Expected status code 400');
$this->assertSame([], $ocs->getData(), 'Expected that no data is send');
$meta = $ocs->getMeta();
$this->assertNotEmpty($meta);
$this->assertArrayHasKey('message', $meta);
$this->assertSame($message, $meta['message']);
}
}