From 53bb706e3e098c1436ca8d4423f6cda38247934f Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 15:15:49 +0200 Subject: [PATCH 1/7] With a docker-compose file tests can use the hostname fron the browser, http://svn/ preferrably --- repos-web/open/SvnOpenFile.test.php | 1 - 1 file changed, 1 deletion(-) diff --git a/repos-web/open/SvnOpenFile.test.php b/repos-web/open/SvnOpenFile.test.php index 0af1eb76..64762402 100644 --- a/repos-web/open/SvnOpenFile.test.php +++ b/repos-web/open/SvnOpenFile.test.php @@ -12,7 +12,6 @@ class TestSvnOpenFile extends UnitTestCase { function setUp() { _svnOpenFile_setInstance(null); - $_SERVER['SERVER_NAME'] = 'localhost'; } function testGetUrl() { From e1787ee6a431dd83d49197fdb0bfb52b7e88049d Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 17:55:34 +0200 Subject: [PATCH 2/7] Allows folder downloads in addition to files, if path matches a p-regex from env --- docker-compose.yml | 2 ++ repos-web/open/SvnOpenFile.class.php | 13 ++++++++++++- repos-web/open/SvnOpenFile.test.php | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 76167457..320b5a67 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -20,5 +20,7 @@ services: - "9000" links: - svn:svn + environment: + REPOS_DOWNLOAD_RULE: "|^/svn/[^/]+/downloadableDirs/.+|" volumes: - .:/opt/rweb diff --git a/repos-web/open/SvnOpenFile.class.php b/repos-web/open/SvnOpenFile.class.php index e063fad8..73d6fd8d 100644 --- a/repos-web/open/SvnOpenFile.class.php +++ b/repos-web/open/SvnOpenFile.class.php @@ -413,7 +413,18 @@ function isReadableInHead() { $this->_head(); return ($this->headStatus == 200); } - + + function isDownloadAllowed() { + $rule = isset($_SERVER['REPOS_DOWNLOAD_RULE']) ? $_SERVER['REPOS_DOWNLOAD_RULE'] : ''; + if ($rule) { + $urlFull = $this->getUrlNoquery(); + $pathFromRoot = substr($urlFull, strpos($urlFull, '/', 8)); + if (preg_match($rule, $pathFromRoot)) return true; + } + // backwards compatibility with no env + return $this->isFile(); + } + /** * Checks for write access. * Authenticates if prompted by server upon DAV request. diff --git a/repos-web/open/SvnOpenFile.test.php b/repos-web/open/SvnOpenFile.test.php index 64762402..1b57c3b3 100644 --- a/repos-web/open/SvnOpenFile.test.php +++ b/repos-web/open/SvnOpenFile.test.php @@ -434,6 +434,20 @@ function testIsFolderNameLooksLikeFile() { $file = new SvnOpenFile('/test/a.folder', HEAD, false); } + function testIsDownloadAllowed() { + $orgRule = isset($_SERVER['REPOS_DOWNLOAD_RULE']) && $_SERVER['REPOS_DOWNLOAD_RULE']; + + $file = new SvnOpenFile('/testfolder/file.txt', HEAD, false); + $this->assertEqual('http://svn/svn', $file->getRepository(), 'This tests assumes the docker-compose setup or equivalent. %s'); + $_SERVER['REPOS_DOWNLOAD_RULE'] = '|^/svn/test.*|'; + $this->assertTrue($file->isDownloadAllowed(), 'Matching REPOS_DOWNLOAD_RULE so should be downloadable. %s'); + + $_SERVER['REPOS_DOWNLOAD_RULE'] = $orgRule; + return; // the test below requires a response, mock or real + $file = new SvnOpenFile('/test/a.txt', HEAD, false); + $this->assertTrue($file->isDownloadAllowed(), 'Files can always be downloadable, as they are streamed. %s'); + } + } testrun(new TestSvnOpenFile()); From 1a64c686354ed29e9dee9506ec62498e8cf1f950 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 18:04:41 +0200 Subject: [PATCH 3/7] Enables download for folders when the rule evaluates to true --- repos-web/open/download/index.php | 5 ++++- repos-web/open/index_en.html | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/repos-web/open/download/index.php b/repos-web/open/download/index.php index 2cb98d5a..3057f139 100644 --- a/repos-web/open/download/index.php +++ b/repos-web/open/download/index.php @@ -16,6 +16,10 @@ '). Maybe it exists in a version other than '.$revisionRule->getValue().'.', E_USER_ERROR); } +if (!$file->isDownloadAllowed()) { + trigger_error('Download has been disabled at '.$file->getPath(), E_USER_ERROR); +} + // Revision number should be "last changed" so we don't get different downloads for identical file // This is also the revision number expected by the "based on version" feature in upload changes // For folders: last changed revision can not be used because subitems might have changed @@ -33,7 +37,6 @@ } if ($file->isFolder()) { - echo 'This service has been disabled.'; exit; require dirname(__FILE__).'/zipfolder.php'; $zip = reposExportZip($file); if ($zip === false) { diff --git a/repos-web/open/index_en.html b/repos-web/open/index_en.html index 18a489d0..9422ce56 100644 --- a/repos-web/open/index_en.html +++ b/repos-web/open/index_en.html @@ -78,8 +78,8 @@

Vi

List contents

Shows a defailed view of the folder's contents at the current revision.

{=/if} -{=if $file,isFile}

Download

-

Download this +{=if $file,isDownloadAllowed}

Download

+

Download this {=if $file,isFile} file {=else} folder as zip archive {=/if} to your hard drive. The revision number {=$file,revision} will be appended to the filename, so you will know which version it is.

From b5afb59af1e8c556f3e7029573e5bb88878e33a9 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 19:32:43 +0200 Subject: [PATCH 4/7] Shows a proper error message when download is forbidden --- repos-web/open/download/index.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/repos-web/open/download/index.php b/repos-web/open/download/index.php index 3057f139..2beeee73 100644 --- a/repos-web/open/download/index.php +++ b/repos-web/open/download/index.php @@ -17,7 +17,12 @@ } if (!$file->isDownloadAllowed()) { - trigger_error('Download has been disabled at '.$file->getPath(), E_USER_ERROR); + require("../../conf/Presentation.class.php"); + $p = Presentation::getInstance(); + $p->showErrorNoRedirect('Download has been disabled at '.$file->getPath(). + '. Folder downloads can grow very big, which is why the feature is blocked by default.', + '405 Method Not Allowed'); + exit; } // Revision number should be "last changed" so we don't get different downloads for identical file From 8b0e7b5feacab4e4dea2933f6bbe8e06559df11c Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 19:33:20 +0200 Subject: [PATCH 5/7] Reuses the same kind of error message for an old TODO --- repos-web/open/download/index.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/repos-web/open/download/index.php b/repos-web/open/download/index.php index 2beeee73..844a66ba 100644 --- a/repos-web/open/download/index.php +++ b/repos-web/open/download/index.php @@ -10,10 +10,12 @@ // if revision is set it is peg $file = new SvnOpenFile(getTarget(), $revisionRule->getValue()); if ($file->getStatus() != 200) { - // TODO have some kind of forwarding to the error pages for matching status code require("../../conf/Presentation.class.php"); - trigger_error('Failed to read '.$file->getPath().' from repository (status '.$file->getStatus(). - '). Maybe it exists in a version other than '.$revisionRule->getValue().'.', E_USER_ERROR); + $p = Presentation::getInstance(); + $p->showErrorNoRedirect('Failed to read '.$file->getPath().' from repository (status '.$file->getStatus(). + '). Maybe it exists in '.($revisionRule->getValue() ? 'a revision other than '.$revisionRule->getValue().'.' : 'a historical revision.'), + '404 Page Not Found'); + exit; } if (!$file->isDownloadAllowed()) { From b58a8737991abdd716e05c889b064454dd291888 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 19:40:04 +0200 Subject: [PATCH 6/7] Sets response status code from showError function if provided with a headline that looks like it --- repos-web/conf/Presentation.class.php | 3 +++ repos-web/open/download/index.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/repos-web/conf/Presentation.class.php b/repos-web/conf/Presentation.class.php index ac756f81..7cea7f63 100644 --- a/repos-web/conf/Presentation.class.php +++ b/repos-web/conf/Presentation.class.php @@ -440,6 +440,9 @@ function showError($error_msg, $headline='An error occurred') { * @param String $headline the contents of the

tag */ function showErrorNoRedirect($error_msg, $headline='An error occurred') { + if (preg_match('/^[1-5][0-9][0-9] /', $headline)) { + header("HTTP/1.1 $headline"); + } $template = $this->getLocaleFile(dirname(__FILE__) . '/Error'); $this->assign('headline', $headline); $this->assign('error', $error_msg); diff --git a/repos-web/open/download/index.php b/repos-web/open/download/index.php index 844a66ba..ca958d2a 100644 --- a/repos-web/open/download/index.php +++ b/repos-web/open/download/index.php @@ -14,7 +14,7 @@ $p = Presentation::getInstance(); $p->showErrorNoRedirect('Failed to read '.$file->getPath().' from repository (status '.$file->getStatus(). '). Maybe it exists in '.($revisionRule->getValue() ? 'a revision other than '.$revisionRule->getValue().'.' : 'a historical revision.'), - '404 Page Not Found'); + '404 Not Found'); exit; } From c394ddab825f468ff16dc6fe9a27370b8a8eccb2 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Sat, 13 May 2017 20:10:21 +0200 Subject: [PATCH 7/7] Fixes https://github.com/Reposoft/rweb/issues/9 --- repos-web/open/json/listjson.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/repos-web/open/json/listjson.php b/repos-web/open/json/listjson.php index eaaa1345..11c654a9 100644 --- a/repos-web/open/json/listjson.php +++ b/repos-web/open/json/listjson.php @@ -28,7 +28,17 @@ function getListXml($url, $rev=false) { $list->addArgUrl($url); } -if ($list->exec()) trigger_error('Could not read entry for URL '.$url, E_USER_ERROR); + if ($list->exec()) { + $err = implode(array_slice($list->getOutput(), -1)); + if (strBegins($err,'svn: E200009')) { + header('HTTP/1.1 404 Not Found', true, 404); + } else { + header('HTTP/1.1 500 Internal Server Error', true, 500); + } + echo '{"end": "'.preg_replace('/"/', '\\"', $err); + echo '"}'."\n"; + exit; + } // TODO detect access denied return implode($list->getOutput(),"\n");