handle refs with / properly, fixes #36

With this patch, Klaus handles git refs with a slash '/', e.g.
feature/foo correctly. The URL structure remains the same, but there is
no longer a difference in <rev> and <path>.

Any URL is bit by bit checked if it contains a valid ref:

    -> /klaus/tree/foo/bar/setup.py
    <- foo/bar/setup.py (no)
    <- foo/bar, setup.py (yes, foo/bar is a valid ref)

For URL reconstruction, url_for(view, ...), there are still `rev` and
`path` arguments. The view on the other hand, joins both parts to a
single URL (and splits them (again) into rev and path).

Tarball filenames for branches containing a slash are substituted with a
hyphen, so project@foo/bar becomes project@foo-bar.tar.gz

3 files changed, 49 insertions, 16 deletions

diff --git a/klaus/__init__.py b/klaus/__init__.py
index 036fada..57c7714 100644
--- a/klaus/__init__.py
+++ b/klaus/__init__.py
@@ -50,20 +50,20 @@ class Klaus(flask.Flask):
         for endpoint, rule in [
             ('repo_list',   '/'),
             ('robots_txt',  '/robots.txt/'),
-            ('blob',        '/<repo>/blob/<rev>/'),
+            ('blob',        '/<repo>/blob/'),
             ('blob',        '/<repo>/blob/<rev>/<path:path>'),
-            ('blame',       '/<repo>/blame/<rev>/'),
+            ('blame',       '/<repo>/blame/'),
             ('blame',       '/<repo>/blame/<rev>/<path:path>'),
-            ('raw',         '/<repo>/raw/<rev>/'),
+            ('raw',         '/<repo>/raw/<path:path>/'),
             ('raw',         '/<repo>/raw/<rev>/<path:path>'),
-            ('commit',      '/<repo>/commit/<rev>/'),
-            ('patch',       '/<repo>/commit/<rev>.diff'),
-            ('patch',       '/<repo>/commit/<rev>.patch'),
+            ('commit',      '/<repo>/commit/<path:rev>/'),
+            ('patch',       '/<repo>/commit/<path:rev>.diff'),
+            ('patch',       '/<repo>/commit/<path:rev>.patch'),
             ('index',       '/<repo>/'),
             ('index',       '/<repo>/<path:rev>'),
-            ('history',     '/<repo>/tree/<rev>/'),
+            ('history',     '/<repo>/tree/<rev>'),
             ('history',     '/<repo>/tree/<rev>/<path:path>'),
-            ('download',    '/<repo>/tarball/<rev>/'),
+            ('download',    '/<repo>/tarball/<path:rev>/'),
         ]:
             self.add_url_rule(rule, view_func=getattr(views, endpoint))
 
diff --git a/klaus/utils.py b/klaus/utils.py
index d6b96ba..9bc95d6 100644
--- a/klaus/utils.py
+++ b/klaus/utils.py
@@ -246,3 +246,9 @@ def guess_git_revision():
         # Either the git executable couldn't be found in the OS's PATH
         # or no ".git" directory exists, i.e. this is no "bleeding-edge" installation.
         return None
+
+
+def sanitize_branch_name(name, chars='./', repl='-'):
+    for char in chars:
+        name = name.replace(char, repl)
+    return name
diff --git a/klaus/views.py b/klaus/views.py
index c0afc03..47bf94a 100644
--- a/klaus/views.py
+++ b/klaus/views.py
@@ -21,7 +21,7 @@ else:
 from klaus import markup
 from klaus.highlighting import highlight_or_render
 from klaus.utils import parent_directory, subpaths, force_unicode, guess_is_binary, \
-                        guess_is_image, replace_dupes
+                        guess_is_image, replace_dupes, sanitize_branch_name
 
 
 README_FILENAMES = [b'README', b'README.md', b'README.rst']
@@ -39,12 +39,16 @@ def repo_list():
     return render_template('repo_list.html', repos=repos, base_href=None)
 
 
+
 def robots_txt():
     """Serve the robots.txt file to manage the indexing of the site by search engines."""
     return current_app.send_static_file('robots.txt')
 
 
-def _get_repo_and_rev(repo, rev=None):
+def _get_repo_and_rev(repo, rev=None, path=None):
+    if path and rev:
+        rev += "/" + path.rstrip("/")
+
     try:
         repo = current_app.repos[repo]
     except KeyError:
@@ -54,12 +58,21 @@ def _get_repo_and_rev(repo, rev=None):
         rev = repo.get_default_branch()
         if rev is None:
             raise NotFound("Empty repository")
-    try:
-        commit = repo.get_commit(rev)
-    except KeyError:
+
+    i = len(rev)
+    while i > 0:
+        try:
+            commit = repo.get_commit(rev[:i])
+            path = rev[i:].strip("/")
+            rev = rev[:i]
+        except (KeyError, IOError):
+            i = rev.rfind("/", 0, i)
+        else:
+            break
+    else:
         raise NotFound("No such commit %r" % rev)
 
-    return repo, rev, commit
+    return repo, rev, path, commit
 
 
 class BaseRepoView(View):
@@ -79,6 +92,18 @@ class BaseRepoView(View):
         self.context = {}
 
     def dispatch_request(self, repo, rev=None, path=''):
+        """Dispatch repository, revision (if any) and path (if any). To retain
+        compatibility with :func:`url_for`, view routing uses two arguments:
+        rev and path, although a single path is sufficient (from Git's point of
+        view, '/foo/bar/baz' may be a branch '/foo/bar' containing baz, or a
+        branch '/foo' containing 'bar/baz', but never both [1].
+
+        Hence, rebuild rev and path to a single path argument, which is then
+        later split into rev and path again, but revision now may contain
+        slashes.
+
+        [1] https://github.com/jonashaag/klaus/issues/36#issuecomment-23990266
+        """
         self.make_template_context(repo, rev, path.strip('/'))
         return self.get_response()
 
@@ -86,7 +111,8 @@ class BaseRepoView(View):
         return render_template(self.template_name, **self.context)
 
     def make_template_context(self, repo, rev, path):
-        repo, rev, commit = _get_repo_and_rev(repo, rev)
+        repo, rev, path, commit = _get_repo_and_rev(repo, rev, path)
+
         try:
             blob_or_tree = repo.get_blob_or_tree(commit, path)
         except KeyError:
@@ -354,7 +380,8 @@ class RawView(BaseBlobView):
 class DownloadView(BaseRepoView):
     """Download a repo as a tar.gz file."""
     def get_response(self):
-        tarname = "%s@%s.tar.gz" % (self.context['repo'].name, self.context['rev'])
+        tarname = "%s@%s.tar.gz" % (self.context['repo'].name,
+                                    sanitize_branch_name(self.context['rev']))
         headers = {
             'Content-Disposition': "attachment; filename=%s" % tarname,
             'Cache-Control': "no-store",  # Disables browser caching