From da7c20d081902e125d20162aff2b6c71761f92ef Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 16 Feb 2018 16:04:19 +0100 Subject: [PATCH] [stable10] Properly encode returned Webdav URL in ViewController Affects the URL as displayed in the files app cog icon and also the one returned in the "Webdav-Location" header --- apps/files/lib/Controller/ViewController.php | 6 +-- .../tests/Controller/ViewControllerTest.php | 48 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index 6f690fea07a9..f1a119e4605b 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -220,7 +220,7 @@ public function index($dir = '', $view = '', $fileid = null) { return $item1['order'] - $item2['order']; }); $nav->assign('navigationItems', $navItems); - $nav->assign('webdavUrl', $this->urlGenerator->getAbsoluteUrl($this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . $user . '/')); + $nav->assign('webdavUrl', $this->urlGenerator->getAbsoluteUrl($this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . rawurlencode($user) . '/')); $contentItems = []; @@ -307,8 +307,8 @@ public function showFile($fileId) { } $response = new RedirectResponse($this->urlGenerator->linkToRoute('files.view.index', $params)); if ($isFilesView) { - $webdavUrl = $this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . $uid . '/'; - $webdavUrl .= ltrim($baseFolder->getRelativePath($file->getPath()), '/'); + $webdavUrl = $this->urlGenerator->linkTo('', 'remote.php') . '/dav/files/' . rawurlencode($uid) . '/'; + $webdavUrl .= \OCP\Util::encodePath(ltrim($baseFolder->getRelativePath($file->getPath()), '/')); $response->addHeader('Webdav-Location', $webdavUrl); } return $response; diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index 3443b3e407be..4688cc4bda24 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -79,7 +79,7 @@ public function setUp() { $this->user = $this->createMock('\OCP\IUser'); $this->user->expects($this->any()) ->method('getUID') - ->will($this->returnValue('testuser1')); + ->will($this->returnValue('test@#?%test')); $this->userSession->expects($this->any()) ->method('getUser') ->will($this->returnValue($this->user)); @@ -188,8 +188,8 @@ public function testIndexWithRegularBrowser() { ->with('', 'remote.php') ->willReturn('/owncloud/remote.php'); $this->urlGenerator->method('getAbsoluteUrl') - ->with('/owncloud/remote.php/dav/files/' . $this->user->getUID() . '/') - ->willReturn('http://example.org/owncloud/remote.php/dav/files/' . $this->user->getUID() . '/'); + ->with('/owncloud/remote.php/dav/files/test%40%23%3F%25test/') + ->willReturn('http://example.org/owncloud/remote.php/dav/files/test%40%23%3F%25test/'); $nav = new Template('files', 'appnavigation'); $nav->assign('navigationItems', [ @@ -257,7 +257,7 @@ public function testIndexWithRegularBrowser() { 'icon' => '', ], ]); - $nav->assign('webdavUrl', 'http://example.org/owncloud/remote.php/dav/files/' . $this->user->getUID() . '/'); + $nav->assign('webdavUrl', 'http://example.org/owncloud/remote.php/dav/files/test%40%23%3F%25test/'); $expected = new Http\TemplateResponse( 'files', @@ -328,13 +328,13 @@ public function testShowFileRouteWithFolder($useShowFile) { $node = $this->createMock('\OCP\Files\Folder'); $node->expects($this->any()) ->method('getPath') - ->will($this->returnValue('/testuser1/files/test/sub')); + ->will($this->returnValue('/test@#?%test/files/to sp@ce/a@b#?%')); $baseFolder = $this->createMock('\OCP\Files\Folder'); $this->rootFolder->expects($this->once()) ->method('get') - ->with('testuser1/files/') + ->with('test@#?%test/files/') ->will($this->returnValue($baseFolder)); $baseFolder->expects($this->any()) @@ -343,17 +343,17 @@ public function testShowFileRouteWithFolder($useShowFile) { ->will($this->returnValue([$node])); $baseFolder->expects($this->any()) ->method('getRelativePath') - ->with('/testuser1/files/test/sub') - ->will($this->returnValue('/test/sub')); + ->with('/test@#?%test/files/to sp@ce/a@b#?%') + ->will($this->returnValue('/to sp@ce/a@b#?%')); $this->urlGenerator ->expects($this->once()) ->method('linkToRoute') - ->with('files.view.index', ['dir' => '/test/sub']) - ->will($this->returnValue('/owncloud/index.php/apps/files/?dir=/test/sub')); + ->with('files.view.index', ['dir' => '/to sp@ce/a@b#?%']) + ->will($this->returnValue('/owncloud/index.php/apps/files/?dir=/to%20sp%40ce/a%40b%23%3F%25')); - $expected = new Http\RedirectResponse('/owncloud/index.php/apps/files/?dir=/test/sub'); - $expected->addHeader('Webdav-Location', '/owncloud/remote.php/dav/files/testuser1/test/sub'); + $expected = new Http\RedirectResponse('/owncloud/index.php/apps/files/?dir=/to%20sp%40ce/a%40b%23%3F%25'); + $expected->addHeader('Webdav-Location', '/owncloud/remote.php/dav/files/test%40%23%3F%25test/to%20sp%40ce/a%40b%23%3F%25'); if ($useShowFile) { $this->assertEquals($expected, $this->viewController->showFile(123)); } else { @@ -368,13 +368,13 @@ public function testShowFileRouteWithFile($useShowFile) { $parentNode = $this->createMock('\OCP\Files\Folder'); $parentNode->expects($this->any()) ->method('getPath') - ->will($this->returnValue('testuser1/files/test')); + ->will($this->returnValue('test@#?%test/files/test')); $baseFolder = $this->createMock('\OCP\Files\Folder'); $this->rootFolder->expects($this->once()) ->method('get') - ->with('testuser1/files/') + ->with('test@#?%test/files/') ->will($this->returnValue($baseFolder)); $node = $this->createMock('\OCP\Files\File'); @@ -386,7 +386,7 @@ public function testShowFileRouteWithFile($useShowFile) { ->will($this->returnValue('somefile.txt')); $node->expects($this->any()) ->method('getPath') - ->will($this->returnValue('testuser1/files/test/somefile.txt')); + ->will($this->returnValue('test@#?%test/files/test/somefile.txt')); $baseFolder->expects($this->any()) ->method('getById') @@ -395,8 +395,8 @@ public function testShowFileRouteWithFile($useShowFile) { $baseFolder->expects($this->any()) ->method('getRelativePath') ->will($this->returnValueMap([ - ['testuser1/files/test', '/test'], - ['testuser1/files/test/somefile.txt', '/test/somefile.txt'], + ['test@#?%test/files/test', '/test'], + ['test@#?%test/files/test/somefile.txt', '/test/somefile.txt'], ])); $this->urlGenerator @@ -406,7 +406,7 @@ public function testShowFileRouteWithFile($useShowFile) { ->will($this->returnValue('/owncloud/index.php/apps/files/?dir=/test&scrollto=somefile.txt')); $expected = new Http\RedirectResponse('/owncloud/index.php/apps/files/?dir=/test&scrollto=somefile.txt'); - $expected->addHeader('Webdav-Location', '/owncloud/remote.php/dav/files/testuser1/test/somefile.txt'); + $expected->addHeader('Webdav-Location', '/owncloud/remote.php/dav/files/test%40%23%3F%25test/test/somefile.txt'); if ($useShowFile) { $this->assertEquals($expected, $this->viewController->showFile(123)); } else { @@ -421,7 +421,7 @@ public function testShowFileRouteWithInvalidFileId($useShowFile) { $baseFolder = $this->createMock('\OCP\Files\Folder'); $this->rootFolder->expects($this->once()) ->method('get') - ->with('testuser1/files/') + ->with('test@#?%test/files/') ->will($this->returnValue($baseFolder)); $baseFolder->expects($this->at(0)) @@ -446,7 +446,7 @@ public function testShowFileRouteWithInvalidFileIdLoggedIn() { $baseFolder = $this->createMock('\OCP\Files\Folder'); $this->rootFolder->expects($this->once()) ->method('get') - ->with('testuser1/files/') + ->with('test@#?%test/files/') ->will($this->returnValue($baseFolder)); $baseFolder->expects($this->at(0)) @@ -480,19 +480,19 @@ public function testShowFileRouteWithTrashedFile($useShowFile) { $parentNode = $this->createMock('\OCP\Files\Folder'); $parentNode->expects($this->once()) ->method('getPath') - ->will($this->returnValue('testuser1/files_trashbin/files/test.d1462861890/sub')); + ->will($this->returnValue('test@#?%test/files_trashbin/files/test.d1462861890/sub')); $baseFolderFiles = $this->createMock('\OCP\Files\Folder'); $baseFolderTrash = $this->createMock('\OCP\Files\Folder'); $this->rootFolder->expects($this->at(0)) ->method('get') - ->with('testuser1/files/') + ->with('test@#?%test/files/') ->will($this->returnValue($baseFolderFiles)); //The index is pointing to 2, because nodeExists internally calls get method. $this->rootFolder->expects($this->at(2)) ->method('get') - ->with('testuser1/files_trashbin/files/') + ->with('test@#?%test/files_trashbin/files/') ->will($this->returnValue($baseFolderTrash)); $baseFolderFiles->expects($this->once()) @@ -514,7 +514,7 @@ public function testShowFileRouteWithTrashedFile($useShowFile) { ->will($this->returnValue([$node])); $baseFolderTrash->expects($this->at(1)) ->method('getRelativePath') - ->with('testuser1/files_trashbin/files/test.d1462861890/sub') + ->with('test@#?%test/files_trashbin/files/test.d1462861890/sub') ->will($this->returnValue('/test.d1462861890/sub')); $this->urlGenerator