changeset 7718:4b41a96416f5

middleware: introduce BaseVCSController parse_request retrieving repo_name and use that for VCS dispatch Return a parsed_request namespace. For now, it just contains repo_name, thus replacing is_hg / is_git and __get_repository. The fallback to the wrapped application now happens in the base class and the VCS implementations can be simplified.
author Mads Kiilerich <mads@kiilerich.com>
date Tue, 08 Jan 2019 13:02:34 +0100
parents 8d0362047e29
children 04ace15a511e
files kallithea/lib/base.py kallithea/lib/middleware/simplegit.py kallithea/lib/middleware/simplehg.py
diffstat 3 files changed, 51 insertions(+), 94 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/base.py	Tue Jan 08 13:02:34 2019 +0100
+++ b/kallithea/lib/base.py	Tue Jan 08 13:02:34 2019 +0100
@@ -201,6 +201,13 @@
         self.authenticate = BasicAuth('', auth_modules.authenticate,
                                       config.get('auth_ret_code'))
 
+    @classmethod
+    def parse_request(cls, environ):
+        """If request is parsed as a request for this VCS, return a namespace with the parsed request.
+        If the request is unknown, return None.
+        """
+        raise NotImplementedError()
+
     def _authorize(self, environ, start_response, action, repo_name, ip_addr):
         """Authenticate and authorize user.
 
@@ -297,7 +304,10 @@
     def __call__(self, environ, start_response):
         start = time.time()
         try:
-            return self._handle_request(environ, start_response)
+            parsed_request = self.parse_request(environ)
+            if parsed_request is None:
+                return self.application(environ, start_response)
+            return self._handle_request(parsed_request, environ, start_response)
         finally:
             log = logging.getLogger('kallithea.' + self.__class__.__name__)
             log.debug('Request time: %.3fs', time.time() - start)
--- a/kallithea/lib/middleware/simplegit.py	Tue Jan 08 13:02:34 2019 +0100
+++ b/kallithea/lib/middleware/simplegit.py	Tue Jan 08 13:02:34 2019 +0100
@@ -49,38 +49,28 @@
 GIT_PROTO_PAT = re.compile(r'^/(.+)/(info/refs|git-upload-pack|git-receive-pack)$')
 
 
-def is_git(environ):
-    path_info = environ['PATH_INFO']
-    isgit_path = GIT_PROTO_PAT.match(path_info)
-    log.debug('pathinfo: %s detected as Git %s',
-        path_info, isgit_path is not None
-    )
-    return isgit_path
-
-
 class SimpleGit(BaseVCSController):
 
-    def _handle_request(self, environ, start_response):
-        if not is_git(environ):
-            return self.application(environ, start_response)
+    @classmethod
+    def parse_request(cls, environ):
+        path_info = environ.get('PATH_INFO', '')
+        m = GIT_PROTO_PAT.match(path_info)
+        if m is None:
+            return None
 
+        class parsed_request(object):
+            repo_name = safe_unicode(m.group(1).rstrip('/'))
+            cmd = m.group(2)
+
+        return parsed_request
+
+    def _handle_request(self, parsed_request, environ, start_response):
         ip_addr = self._get_ip_addr(environ)
         # skip passing error to error controller
         environ['pylons.status_code_redirect'] = True
 
-        #======================================================================
-        # EXTRACT REPOSITORY NAME FROM ENV
-        #======================================================================
-        try:
-            str_repo_name = self.__get_repository(environ)
-            repo_name = safe_unicode(str_repo_name)
-            log.debug('Extracted repo name is %s', repo_name)
-        except Exception as e:
-            log.error('error extracting repo_name: %r', e)
-            return HTTPInternalServerError()(environ, start_response)
-
-        # quick check if that dir exists...
-        if not is_valid_repo(repo_name, self.basepath, 'git'):
+        # quick check if repo exists...
+        if not is_valid_repo(parsed_request.repo_name, self.basepath, 'git'):
             return HTTPNotFound()(environ, start_response)
 
         #======================================================================
@@ -91,7 +81,7 @@
         #======================================================================
         # CHECK PERMISSIONS
         #======================================================================
-        user, response_app = self._authorize(environ, start_response, action, repo_name, ip_addr)
+        user, response_app = self._authorize(environ, start_response, action, parsed_request.repo_name, ip_addr)
         if response_app is not None:
             return response_app(environ, start_response)
 
@@ -103,7 +93,7 @@
             'ip': ip_addr,
             'username': user.username,
             'action': action,
-            'repository': repo_name,
+            'repository': parsed_request.repo_name,
             'scm': 'git',
             'config': CONFIG['__file__'],
             'server_url': server_url,
@@ -117,10 +107,10 @@
         _set_extras(extras or {})
 
         try:
-            self._handle_githooks(repo_name, action, baseui, environ)
+            self._handle_githooks(parsed_request.repo_name, action, baseui, environ)
             log.info('%s action on Git repo "%s" by "%s" from %s',
-                     action, str_repo_name, safe_str(user.username), ip_addr)
-            app = self.__make_app(repo_name)
+                     action, parsed_request.repo_name, safe_str(user.username), ip_addr)
+            app = self.__make_app(parsed_request.repo_name)
             return app(environ, start_response)
         except Exception:
             log.error(traceback.format_exc())
@@ -132,18 +122,6 @@
         """
         return make_wsgi_app(repo_name, safe_str(self.basepath)) # FIXME: safe_str???
 
-    def __get_repository(self, environ):
-        """
-        Gets repository name out of PATH_INFO header
-
-        :param environ: environ where PATH_INFO is stored
-        """
-        try:
-            return GIT_PROTO_PAT.match(environ['PATH_INFO']).group(1)
-        except Exception:
-            log.error(traceback.format_exc())
-            raise
-
     def __get_action(self, environ):
         """
         Maps Git request commands into 'pull' or 'push'.
--- a/kallithea/lib/middleware/simplehg.py	Tue Jan 08 13:02:34 2019 +0100
+++ b/kallithea/lib/middleware/simplehg.py	Tue Jan 08 13:02:34 2019 +0100
@@ -45,24 +45,6 @@
 log = logging.getLogger(__name__)
 
 
-def is_mercurial(environ):
-    """
-    Returns True if request's target is mercurial server - header
-    ``HTTP_ACCEPT`` of such request would start with ``application/mercurial``.
-    """
-    http_accept = environ.get('HTTP_ACCEPT')
-    path_info = environ['PATH_INFO']
-    if http_accept and http_accept.startswith('application/mercurial'):
-        ishg_path = True
-    else:
-        ishg_path = False
-
-    log.debug('pathinfo: %s detected as Mercurial %s',
-        path_info, ishg_path
-    )
-    return ishg_path
-
-
 def get_header_hgarg(environ):
     """Decode the special Mercurial encoding of big requests over multiple headers.
     >>> get_header_hgarg({})
@@ -83,27 +65,27 @@
 
 class SimpleHg(BaseVCSController):
 
-    def _handle_request(self, environ, start_response):
-        if not is_mercurial(environ):
-            return self.application(environ, start_response)
+    @classmethod
+    def parse_request(cls, environ):
+        http_accept = environ.get('HTTP_ACCEPT', '')
+        if not http_accept.startswith('application/mercurial'):
+            return None
+        path_info = environ.get('PATH_INFO', '')
+        if not path_info.startswith('/'): # it must!
+            return None
 
+        class parsed_request(object):
+            repo_name = safe_unicode(path_info[1:].rstrip('/'))
+
+        return parsed_request
+
+    def _handle_request(self, parsed_request, environ, start_response):
         ip_addr = self._get_ip_addr(environ)
         # skip passing error to error controller
         environ['pylons.status_code_redirect'] = True
 
-        #======================================================================
-        # EXTRACT REPOSITORY NAME FROM ENV
-        #======================================================================
-        try:
-            str_repo_name = self.__get_repository(environ)
-            repo_name = safe_unicode(str_repo_name)
-            log.debug('Extracted repo name is %s', repo_name)
-        except Exception as e:
-            log.error('error extracting repo_name: %r', e)
-            return HTTPInternalServerError()(environ, start_response)
-
-        # quick check if that dir exists...
-        if not is_valid_repo(repo_name, self.basepath, 'hg'):
+        # quick check if repo exists...
+        if not is_valid_repo(parsed_request.repo_name, self.basepath, 'hg'):
             return HTTPNotFound()(environ, start_response)
 
         #======================================================================
@@ -117,7 +99,7 @@
         #======================================================================
         # CHECK PERMISSIONS
         #======================================================================
-        user, response_app = self._authorize(environ, start_response, action, repo_name, ip_addr)
+        user, response_app = self._authorize(environ, start_response, action, parsed_request.repo_name, ip_addr)
         if response_app is not None:
             return response_app(environ, start_response)
 
@@ -129,7 +111,7 @@
             'ip': ip_addr,
             'username': user.username,
             'action': action,
-            'repository': repo_name,
+            'repository': parsed_request.repo_name,
             'scm': 'hg',
             'config': CONFIG['__file__'],
             'server_url': server_url,
@@ -137,6 +119,7 @@
         #======================================================================
         # MERCURIAL REQUEST HANDLING
         #======================================================================
+        str_repo_name = safe_str(parsed_request.repo_name)
         repo_path = os.path.join(safe_str(self.basepath), str_repo_name)
         log.debug('Repository path is %s', repo_path)
 
@@ -146,7 +129,7 @@
 
         try:
             log.info('%s action on Mercurial repo "%s" by "%s" from %s',
-                     action, str_repo_name, safe_str(user.username), ip_addr)
+                     action, parsed_request.repo_name, safe_str(user.username), ip_addr)
             environ['REPO_NAME'] = str_repo_name # used by hgweb_mod.hgweb
             app = self.__make_app(repo_path, baseui)
             return app(environ, start_response)
@@ -160,20 +143,6 @@
         """
         return hgweb_mod.hgweb(repo_name, name=repo_name, baseui=baseui)
 
-    def __get_repository(self, environ):
-        """
-        Gets repository name out of PATH_INFO header
-
-        :param environ: environ where PATH_INFO is stored
-        """
-        try:
-            path_info = environ['PATH_INFO']
-            if path_info.startswith('/'):
-                return path_info[1:].rstrip('/')
-        except Exception:
-            log.error(traceback.format_exc())
-            raise
-
     def __get_action(self, environ):
         """
         Maps Mercurial request commands into 'pull' or 'push'.