diff --git a/kallithea/config/app_cfg.py b/kallithea/config/app_cfg.py --- a/kallithea/config/app_cfg.py +++ b/kallithea/config/app_cfg.py @@ -30,6 +30,7 @@ from alembic.migration import MigrationC from sqlalchemy import create_engine import mercurial +from kallithea.lib.middleware.permanent_repo_url import PermanentRepoUrl from kallithea.lib.middleware.https_fixup import HttpsFixup from kallithea.lib.middleware.simplegit import SimpleGit from kallithea.lib.middleware.simplehg import SimpleHg @@ -208,6 +209,8 @@ def setup_application(app): # Enable https redirects based on HTTP_X_URL_SCHEME set by proxy if any(asbool(config.get(x)) for x in ['https_fixup', 'force_https', 'use_htsts']): app = HttpsFixup(app, config) + + app = PermanentRepoUrl(app, config) return app diff --git a/kallithea/config/routing.py b/kallithea/config/routing.py --- a/kallithea/config/routing.py +++ b/kallithea/config/routing.py @@ -33,28 +33,18 @@ def make_map(config): rmap.minimization = False rmap.explicit = False - from kallithea.lib.utils import (is_valid_repo, is_valid_repo_group, - get_repo_by_id) + from kallithea.lib.utils import is_valid_repo, is_valid_repo_group def check_repo(environ, match_dict): """ - check for valid repository for proper 404 handling - - :param environ: - :param match_dict: + Check for valid repository for proper 404 handling. + Also, a bit of side effect modifying match_dict ... """ - repo_name = match_dict.get('repo_name') - if match_dict.get('f_path'): # fix for multiple initial slashes that causes errors match_dict['f_path'] = match_dict['f_path'].lstrip('/') - by_id_match = get_repo_by_id(repo_name) - if by_id_match: - repo_name = by_id_match - match_dict['repo_name'] = repo_name - - return is_valid_repo(repo_name, config['base_path']) + return is_valid_repo(match_dict['repo_name'], config['base_path']) def check_group(environ, match_dict): """ diff --git a/kallithea/lib/base.py b/kallithea/lib/base.py --- a/kallithea/lib/base.py +++ b/kallithea/lib/base.py @@ -266,23 +266,6 @@ class BaseVCSController(object): def _handle_request(self, environ, start_response): raise NotImplementedError() - def _get_by_id(self, repo_name): - """ - Gets a special pattern _ from clone url and tries to replace it - with a repository_name for support of _ permanent URLs - - :param repo_name: - """ - - data = repo_name.split('/') - if len(data) >= 2: - from kallithea.lib.utils import get_repo_by_id - by_id_match = get_repo_by_id(repo_name) - if by_id_match: - data[1] = safe_str(by_id_match) - - return '/'.join(data) - def _check_permission(self, action, authuser, repo_name): """ Checks permissions using action (push/pull) user and repository diff --git a/kallithea/lib/middleware/permanent_repo_url.py b/kallithea/lib/middleware/permanent_repo_url.py new file mode 100644 --- /dev/null +++ b/kallithea/lib/middleware/permanent_repo_url.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +""" +kallithea.lib.middleware.permanent_repo_url +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +middleware to handle permanent repo URLs, replacing PATH_INFO '/_123/yada' with +'/name/of/repo/yada' after looking 123 up in the database. +""" + + +from kallithea.lib.utils import safe_str, fix_repo_id_name + + +class PermanentRepoUrl(object): + + def __init__(self, app, config): + self.application = app + self.config = config + + def __call__(self, environ, start_response): + path_info = environ['PATH_INFO'] + if path_info.startswith('/'): # it must + path_info = '/' + safe_str(fix_repo_id_name(path_info[1:])) + environ['PATH_INFO'] = path_info + + return self.application(environ, start_response) diff --git a/kallithea/lib/middleware/simplegit.py b/kallithea/lib/middleware/simplegit.py --- a/kallithea/lib/middleware/simplegit.py +++ b/kallithea/lib/middleware/simplegit.py @@ -141,14 +141,11 @@ class SimpleGit(BaseVCSController): :param environ: environ where PATH_INFO is stored """ try: - environ['PATH_INFO'] = self._get_by_id(environ['PATH_INFO']) - repo_name = GIT_PROTO_PAT.match(environ['PATH_INFO']).group(1) + return GIT_PROTO_PAT.match(environ['PATH_INFO']).group(1) except Exception: log.error(traceback.format_exc()) raise - return repo_name - def __get_action(self, environ): """ Maps Git request commands into a pull or push command. diff --git a/kallithea/lib/middleware/simplehg.py b/kallithea/lib/middleware/simplehg.py --- a/kallithea/lib/middleware/simplehg.py +++ b/kallithea/lib/middleware/simplehg.py @@ -167,16 +167,13 @@ class SimpleHg(BaseVCSController): :param environ: environ where PATH_INFO is stored """ try: - environ['PATH_INFO'] = self._get_by_id(environ['PATH_INFO']) - repo_name = '/'.join(environ['PATH_INFO'].split('/')[1:]) - if repo_name.endswith('/'): - repo_name = repo_name.rstrip('/') + path_info = environ['PATH_INFO'] + if path_info.startswith('/'): + return path_info[1:].rstrip('/') except Exception: log.error(traceback.format_exc()) raise - return repo_name - def __get_action(self, environ): """ Maps Mercurial request commands into 'pull' or 'push'. diff --git a/kallithea/lib/utils.py b/kallithea/lib/utils.py --- a/kallithea/lib/utils.py +++ b/kallithea/lib/utils.py @@ -78,29 +78,35 @@ def get_user_group_slug(request): return None -def _extract_id_from_repo_name(repo_name): - if repo_name.startswith('/'): - repo_name = repo_name.lstrip('/') - by_id_match = re.match(r'^_(\d{1,})', repo_name) - if by_id_match: - return by_id_match.groups()[0] +def _get_permanent_id(s): + """Helper for decoding stable URLs with repo ID. For a string like '_123' + return 123. + """ + by_id_match = re.match(r'^_(\d+)$', s) + if by_id_match is None: + return None + return int(by_id_match.group(1)) -def get_repo_by_id(repo_name): +def fix_repo_id_name(path): """ - Extracts repo_name by id from special urls. Example url is _11/repo_name + Rewrite repo_name for _ permanent URLs. - :param repo_name: - :return: repo_name if matched else None + Given a path, if the first path element is like _, return the path with + this part expanded to the corresponding full repo name, else return the + provided path. """ - _repo_id = _extract_id_from_repo_name(repo_name) - if _repo_id: + first, rest = path, '' + if '/' in path: + first, rest_ = path.split('/', 1) + rest = '/' + rest_ + repo_id = _get_permanent_id(first) + if repo_id is not None: from kallithea.model.db import Repository - repo = Repository.get(_repo_id) - if repo: - # TODO: return repo instead of reponame? or would that be a layering violation? - return repo.repo_name - return None + repo = Repository.get(repo_id) + if repo is not None: + return repo.repo_name + rest + return path def action_logger(user, action, repo, ipaddr='', commit=False): diff --git a/kallithea/tests/other/test_libs.py b/kallithea/tests/other/test_libs.py --- a/kallithea/tests/other/test_libs.py +++ b/kallithea/tests/other/test_libs.py @@ -532,26 +532,32 @@ class TestLibs(TestController): @parametrize('test,expected', [ ("", None), - ("/_2", '2'), - ("_2", '2'), - ("/_2/", '2'), - ("_2/", '2'), - - ("/_21", '21'), - ("_21", '21'), - ("/_21/", '21'), - ("_21/", '21'), + ("/_2", None), + ("_2", 2), + ("_2/", None), + ]) + def test_get_permanent_id(self, test, expected): + from kallithea.lib.utils import _get_permanent_id + extracted = _get_permanent_id(test) + assert extracted == expected, 'url:%s, got:`%s` expected: `%s`' % (test, _test, expected) - ("/_21/foobar", '21'), - ("_21/121", '21'), - ("/_21/_12", '21'), - ("_21/prefix/foo", '21'), + @parametrize('test,expected', [ + ("", ""), + ("/", "/"), + ("/_ID", '/_ID'), + ("ID", "ID"), + ("_ID", 'NAME'), + ("_ID/", 'NAME/'), + ("_ID/1/2", 'NAME/1/2'), + ("_IDa", '_IDa'), ]) - def test_get_repo_by_id(self, test, expected): - from kallithea.lib.utils import _extract_id_from_repo_name - _test = _extract_id_from_repo_name(test) - assert _test == expected, 'url:%s, got:`%s` expected: `%s`' % (test, _test, expected) - + def test_fix_repo_id_name(self, test, expected): + repo = Repository.get_by_repo_name(HG_REPO) + test = test.replace('ID', str(repo.repo_id)) + expected = expected.replace('NAME', repo.repo_name).replace('ID', str(repo.repo_id)) + from kallithea.lib.utils import fix_repo_id_name + replaced = fix_repo_id_name(test) + assert replaced == expected, 'url:%s, got:`%s` expected: `%s`' % (test, replaced, expected) @parametrize('canonical,test,expected', [ ('http://www.example.org/', '/abc/xyz', 'http://www.example.org/abc/xyz'),