Browse Source

Only intercept exceptions of type "NotFoundException" instead of any Exception

The sharing backend may throw another exception for example when the activity app encounters a problem. Previously this just triggered a 404 error page and the exception got not logged at all. With this change such exceptions get not intercepted and regularly handled as exceptions so that we have meaningful log data. Also the user will be shown a window informing him that an error happened.

Helps to debug cases such as https://github.com/owncloud/core/issues/19465
tags/v8.2RC1
Lukas Reschke 8 years ago
parent
commit
22e724e829

+ 6
- 4
apps/files_sharing/lib/controllers/sharecontroller.php View File

use OCP\User; use OCP\User;
use OCP\Util; use OCP\Util;
use OCA\Files_Sharing\Activity; use OCA\Files_Sharing\Activity;
use \OCP\Files\NotFoundException;


/** /**
* Class ShareController * Class ShareController
* @param string $token * @param string $token
* @param string $path * @param string $path
* @return TemplateResponse|RedirectResponse * @return TemplateResponse|RedirectResponse
* @throws NotFoundException
*/ */
public function showShare($token, $path = '') { public function showShare($token, $path = '') {
\OC_User::setIncognitoMode(true); \OC_User::setIncognitoMode(true);
$getPath = Filesystem::normalizePath($path); $getPath = Filesystem::normalizePath($path);
$originalSharePath .= $path; $originalSharePath .= $path;
} else { } else {
throw new OCP\Files\NotFoundException();
throw new NotFoundException();
} }


$file = basename($originalSharePath); $file = basename($originalSharePath);
/** /**
* @param string $token * @param string $token
* @return string Resolved file path of the token * @return string Resolved file path of the token
* @throws \Exception In case share could not get properly resolved
* @throws NotFoundException In case share could not get properly resolved
*/ */
private function getPath($token) { private function getPath($token) {
$linkItem = Share::getShareByToken($token, false); $linkItem = Share::getShareByToken($token, false);
$rootLinkItem = Share::resolveReShare($linkItem); $rootLinkItem = Share::resolveReShare($linkItem);
if (isset($rootLinkItem['uid_owner'])) { if (isset($rootLinkItem['uid_owner'])) {
if(!$this->userManager->userExists($rootLinkItem['uid_owner'])) { if(!$this->userManager->userExists($rootLinkItem['uid_owner'])) {
throw new \Exception('Owner of the share does not exist anymore');
throw new NotFoundException('Owner of the share does not exist anymore');
} }
OC_Util::tearDownFS(); OC_Util::tearDownFS();
OC_Util::setupFS($rootLinkItem['uid_owner']); OC_Util::setupFS($rootLinkItem['uid_owner']);
} }
} }


throw new \Exception('No file found belonging to file.');
throw new NotFoundException('No file found belonging to file.');
} }
} }

+ 16
- 5
apps/files_sharing/lib/middleware/sharingcheckmiddleware.php View File

use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Middleware; use OCP\AppFramework\Middleware;
use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\TemplateResponse;
use OCP\Files\NotFoundException;
use OCP\IConfig; use OCP\IConfig;


/** /**


/** /**
* Check if sharing is enabled before the controllers is executed * Check if sharing is enabled before the controllers is executed
*
* @param \OCP\AppFramework\Controller $controller
* @param string $methodName
* @throws NotFoundException
*/ */
public function beforeController($controller, $methodName) { public function beforeController($controller, $methodName) {
if(!$this->isSharingEnabled()) { if(!$this->isSharingEnabled()) {
throw new \Exception('Sharing is disabled.');
throw new NotFoundException('Sharing is disabled.');
} }
} }


/** /**
* Return 404 page in case of an exception
* Return 404 page in case of a not found exception
*
* @param \OCP\AppFramework\Controller $controller * @param \OCP\AppFramework\Controller $controller
* @param string $methodName * @param string $methodName
* @param \Exception $exception * @param \Exception $exception
* @return TemplateResponse
* @return NotFoundResponse
* @throws \Exception
*/ */
public function afterException($controller, $methodName, \Exception $exception){
return new NotFoundResponse();
public function afterException($controller, $methodName, \Exception $exception) {
if(is_a($exception, '\OCP\Files\NotFoundException')) {
return new NotFoundResponse();
}

throw $exception;
} }


/** /**

+ 54
- 1
apps/files_sharing/tests/middleware/sharingcheckmiddleware.php View File

*/ */


namespace OCA\Files_Sharing\Middleware; namespace OCA\Files_Sharing\Middleware;

use OCP\AppFramework\Http\NotFoundResponse;
use OCP\Files\NotFoundException;


/** /**
* @package OCA\Files_Sharing\Middleware\SharingCheckMiddleware * @package OCA\Files_Sharing\Middleware\SharingCheckMiddleware
private $appManager; private $appManager;
/** @var SharingCheckMiddleware */ /** @var SharingCheckMiddleware */
private $sharingCheckMiddleware; private $sharingCheckMiddleware;
/** @var \OCP\AppFramework\Controller */
private $controllerMock;


protected function setUp() { protected function setUp() {
$this->config = $this->getMockBuilder('\OCP\IConfig') $this->config = $this->getMockBuilder('\OCP\IConfig')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->appManager = $this->getMockBuilder('\OCP\App\IAppManager') $this->appManager = $this->getMockBuilder('\OCP\App\IAppManager')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->controllerMock = $this->getMockBuilder('\OCP\AppFramework\Controller')
->disableOriginalConstructor()->getMock();


$this->sharingCheckMiddleware = new SharingCheckMiddleware('files_sharing', $this->config, $this->appManager); $this->sharingCheckMiddleware = new SharingCheckMiddleware('files_sharing', $this->config, $this->appManager);
} }
$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled')); $this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
} }


public function testBeforeControllerWithSharingEnabled() {
$this->appManager
->expects($this->once())
->method('isEnabledForUser')
->with('files_sharing')
->will($this->returnValue(true));

$this->config
->expects($this->at(0))
->method('getAppValue')
->with('core', 'shareapi_enabled', 'yes')
->will($this->returnValue('yes'));

$this->config
->expects($this->at(1))
->method('getAppValue')
->with('core', 'shareapi_allow_links', 'yes')
->will($this->returnValue('yes'));

$this->sharingCheckMiddleware->beforeController($this->controllerMock, 'myMethod');
}

/**
* @expectedException \OCP\Files\NotFoundException
* @expectedExceptionMessage Sharing is disabled.
*/
public function testBeforeControllerWithSharingDisabled() {
$this->appManager
->expects($this->once())
->method('isEnabledForUser')
->with('files_sharing')
->will($this->returnValue(false));

$this->sharingCheckMiddleware->beforeController($this->controllerMock, 'myMethod');
}

/**
* @expectedException \Exception
* @expectedExceptionMessage My Exception message
*/
public function testAfterExceptionWithRegularException() {
$this->sharingCheckMiddleware->afterException($this->controllerMock, 'myMethod', new \Exception('My Exception message'));
}

public function testAfterExceptionWithNotFoundException() {
$this->assertEquals(new NotFoundResponse(), $this->sharingCheckMiddleware->afterException($this->controllerMock, 'myMethod', new NotFoundException('My Exception message')));
}

} }

Loading…
Cancel
Save