From 5d96e5a4a35fa0dfbb09632cb4658771ad45ea87 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 29 2020 11:45:15 +0000 Subject: [PATCH 1/3] Fix git blame when the identifier provided is a blob Fixes https://pagure.io/pagure/issue/4783 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 0e57369..5d909e0 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -768,8 +768,13 @@ def view_blame_file(repo, filename, username=None, namespace=None): 404, description="Cannot find specified identifier" ) - if isinstance(commit, pygit2.Tag): - commit = commit.peel(pygit2.Commit) + try: + if isinstance(commit, pygit2.Tag): + commit = commit.peel(pygit2.Commit) + elif isinstance(commit, pygit2.Blob): + commit = commit.peel(pygit2.Commit) + except Exception: + flask.abort(404, description="Invalid identified provided") content = __get_file_in_tree( repo_obj, commit.tree, filename.split("/"), bail_on_tree=True diff --git a/tests/test_pagure_flask_ui_repo_view_blame.py b/tests/test_pagure_flask_ui_repo_view_blame.py index 72ea505..bcf0ccc 100644 --- a/tests/test_pagure_flask_ui_repo_view_blame.py +++ b/tests/test_pagure_flask_ui_repo_view_blame.py @@ -20,6 +20,7 @@ sys.path.insert( import tests import pagure.lib.model +from pagure.utils import __get_file_in_tree as get_file_in_tree class PagureFlaskRepoViewBlameFileSimpletests(tests.Modeltests): @@ -204,6 +205,24 @@ class PagureFlaskRepoViewBlameFiletests(tests.Modeltests): data = self.regex.findall(output_text) self.assertEqual(len(data), 1) + def test_view_blame_file_on_blob(self): + """ Test the view_blame_file endpoint """ + # Retrieve the blob of the `sources` file in head + repo_obj = pygit2.Repository( + os.path.join(self.path, "repos", "test.git") + ) + commit = repo_obj[repo_obj.head.target] + content = get_file_in_tree( + repo_obj, commit.tree, ["sources"], bail_on_tree=True + ) + + output = self.app.get( + "/test/blame/sources?identifier=%s" % content.oid.hex + ) + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn("Invalid identified provided", output_text) + def test_view_blame_file_binary(self): """ Test the view_blame_file endpoint """ # Add binary content From 89a24d37b8fb7127e12422179f12c8b7745e8421 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 29 2020 11:45:15 +0000 Subject: [PATCH 2/3] Fix view_commits when the identified provided is a blob Fixes https://pagure.io/pagure/issue/4784 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 5d909e0..cf95f80 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -322,6 +322,14 @@ def view_commits(repo, branchname=None, username=None, namespace=None): if isinstance(commit, pygit2.Tag): commit = commit.peel(pygit2.Commit) branchname = commit.oid.hex + elif isinstance(commit, pygit2.Blob): + try: + commit = commit.peel(pygit2.Commit) + branchname = commit.oid.hex + except Exception: + flask.abort( + 404, description="Invalid branch/identifier provided" + ) elif not repo_obj.is_empty and not repo_obj.head_is_unborn: branch = repo_obj.lookup_branch(repo_obj.head.shorthand) commit = branch.peel(pygit2.Commit) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 7a83290..63bcf4f 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -31,6 +31,7 @@ sys.path.insert( import pagure.lib.query import tests from pagure.lib.repo import PagureRepo +from pagure.utils import __get_file_in_tree as get_file_in_tree class PagureFlaskRepotests(tests.Modeltests): @@ -2282,6 +2283,28 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn("Commits - test - Pagure", output_text) self.assertEqual(output_text.count(''), 1) + def test_view_commits_from_blob(self): + """ Test the view_commits endpoint given a blob. """ + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + + tests.add_content_to_git(os.path.join(self.path, "repos", "test.git")) + + # Retrieve the blob of the `sources` file in head + repo_obj = pygit2.Repository( + os.path.join(self.path, "repos", "test.git") + ) + commit = repo_obj[repo_obj.head.target] + content = get_file_in_tree( + repo_obj, commit.tree, ["sources"], bail_on_tree=True + ) + + output = self.app.get("/test/commits/%s" % content.oid.hex) + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn("Invalid branch/identifier provided", output_text) + def test_view_commit_from_tag(self): """ Test the view_commit endpoint given a tag. """ From 9982ed8cbe7b383f2200443e91ba791a861c4e48 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 29 2020 11:45:16 +0000 Subject: [PATCH 3/3] When viewing file's history, use the default branch if needed If the user does not specify a branch, we will show the history in the default branch so define the branchname variable so it can be used in the templates Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index cf95f80..aa1201b 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -829,10 +829,16 @@ def view_history_file(repo, filename, username=None, namespace=None): repo_obj = flask.g.repo_obj branchname = flask.request.args.get("identifier") - if repo_obj.is_empty: flask.abort(404, description="Empty repo cannot have a file") + if not branchname: + try: + branch = repo_obj.lookup_branch(repo_obj.head.shorthand) + branchname = branch.branch_name + except pygit2.GitError: + flask.abort(400, description="Invalid repository") + try: log = pagure.lib.repo.PagureRepo.log( flask.g.reponame, diff --git a/tests/__init__.py b/tests/__init__.py index 165c37d..8048fee 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1000,6 +1000,7 @@ def add_tag_git_repo(folder, tagname, obj_hash, message): def add_content_to_git( folder, branch="master", + folders=None, filename="sources", content="foo", message=None, @@ -1012,9 +1013,13 @@ def add_content_to_git( ) # Create a file in that git repo - with open( - os.path.join(newfolder, filename), "a", encoding="utf-8" - ) as stream: + if folders: + if not os.path.exists(os.path.join(newfolder, folders)): + os.makedirs(os.path.join(newfolder, folders)) + filename = os.path.join(folders, filename) + + filepath = os.path.join(newfolder, filename) + with open(filepath, "a", encoding="utf-8") as stream: stream.write("%s\n" % content) repo.index.add(filename) repo.index.write() diff --git a/tests/test_pagure_flask_ui_repo_view_history.py b/tests/test_pagure_flask_ui_repo_view_history.py index 1c53f4c..36e7f11 100644 --- a/tests/test_pagure_flask_ui_repo_view_history.py +++ b/tests/test_pagure_flask_ui_repo_view_history.py @@ -234,6 +234,21 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output_text = output.get_data(as_text=True) self.assertIn("No history could be found for this file", output_text) + def test_view_history_file_existing_folder(self): + """ Test the view_history_file endpoint """ + tests.add_content_to_git( + os.path.join(self.path, "repos", "test.git"), folders="foo/bar" + ) + + output = self.app.get("/test/history/foo/bar/") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Add content to file foo/bar/sources", output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 1) + def test_view_history_file_unborn_head_no_identifier(self): repo_obj = pygit2.Repository( os.path.join(self.path, "repos", "test.git") @@ -243,4 +258,4 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/sources") self.assertEqual(output.status_code, 400) output_text = output.get_data(as_text=True) - self.assertIn("No history could be found for this file", output_text) + self.assertIn("Invalid repository", output_text)