From 5a1c687faa741788c4028b8257abd5c1c67eaa21 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 16 Feb 2018 16:04:19 +0100 Subject: [PATCH] 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 457e211e76bc..d843769b2882 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -80,7 +80,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)); @@ -189,8 +189,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', [ @@ -258,7 +258,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', @@ -329,13 +329,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()) @@ -344,17 +344,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 { @@ -369,13 +369,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'); @@ -387,7 +387,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') @@ -396,8 +396,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 @@ -407,7 +407,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 { @@ -422,7 +422,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)) @@ -447,7 +447,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)) @@ -481,19 +481,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()) @@ -515,7 +515,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