From 143575c9dec9d1126e087dc451417b1910352ed2 Mon Sep 17 00:00:00 2001 From: moson-mo Date: Sun, 11 Jun 2023 20:31:51 +0200 Subject: [PATCH] fix: restore command, remove premature creation of pkgbase We're currently creating a "PackageBases" when the "restore" command is executed. This is problematic for pkgbases that never existed before. In those cases it will create the record but fail in the update.py script. Thus it leaves an orphan "PackageBases" record in the DB (which does not have any related "Packages" record(s)) Navigating to such a packages /pkgbase/... URL will result in a crash since it is not foreseen to have "orphan" pkgbase records. We can safely remove the early creation of that record because it'll be taken care of in the update.py script that is being called We'll also fix some tests. Before it was executing a dummy script instead of "update.py" which might be a bit misleading since it did not check the real outcome of our "restore" action. Signed-off-by: moson-mo --- aurweb/git/serve.py | 24 +++++------------------- test/setup.sh | 9 +-------- test/t1200-git-serve.t | 23 +++++++++++++++-------- 3 files changed, 21 insertions(+), 35 deletions(-) diff --git a/aurweb/git/serve.py b/aurweb/git/serve.py index 2ac1f10e..333d0394 100755 --- a/aurweb/git/serve.py +++ b/aurweb/git/serve.py @@ -52,7 +52,7 @@ def list_repos(user): conn.close() -def create_pkgbase(pkgbase, user): +def validate_pkgbase(pkgbase, user): if not re.match(repo_regex, pkgbase): raise aurweb.exceptions.InvalidRepositoryNameException(pkgbase) if pkgbase_exists(pkgbase): @@ -62,26 +62,12 @@ def create_pkgbase(pkgbase, user): cur = conn.execute("SELECT ID FROM Users WHERE Username = ?", [user]) userid = cur.fetchone()[0] + + conn.close() + if userid == 0: raise aurweb.exceptions.InvalidUserException(user) - now = int(time.time()) - cur = conn.execute( - "INSERT INTO PackageBases (Name, SubmittedTS, " - + "ModifiedTS, SubmitterUID, MaintainerUID, " - + "FlaggerComment) VALUES (?, ?, ?, ?, ?, '')", - [pkgbase, now, now, userid, userid], - ) - pkgbase_id = cur.lastrowid - - cur = conn.execute( - "INSERT INTO PackageNotifications " + "(PackageBaseID, UserID) VALUES (?, ?)", - [pkgbase_id, userid], - ) - - conn.commit() - conn.close() - def pkgbase_adopt(pkgbase, user, privileged): pkgbase_id = pkgbase_from_name(pkgbase) @@ -577,7 +563,7 @@ def serve(action, cmdargv, user, privileged, remote_addr): # noqa: C901 checkarg(cmdargv, "repository name") pkgbase = cmdargv[1] - create_pkgbase(pkgbase, user) + validate_pkgbase(pkgbase, user) os.environ["AUR_USER"] = user os.environ["AUR_PKGBASE"] = pkgbase diff --git a/test/setup.sh b/test/setup.sh index 2db897bf..ccf24086 100644 --- a/test/setup.sh +++ b/test/setup.sh @@ -56,7 +56,7 @@ ssh-options = restrict repo-path = ./aur.git/ repo-regex = [a-z0-9][a-z0-9.+_-]*$ git-shell-cmd = ./git-shell.sh -git-update-cmd = ./update.sh +git-update-cmd = $GIT_UPDATE ssh-cmdline = ssh aur@aur.archlinux.org [update] @@ -90,13 +90,6 @@ echo $GIT_NAMESPACE EOF chmod +x git-shell.sh -cat >update.sh <<-\EOF -#!/bin/sh -echo $AUR_USER -echo $AUR_PKGBASE -EOF -chmod +x update.sh - AUR_CONFIG=config export AUR_CONFIG diff --git a/test/t1200-git-serve.t b/test/t1200-git-serve.t index dbb465bc..bb3a004f 100755 --- a/test/t1200-git-serve.t +++ b/test/t1200-git-serve.t @@ -137,14 +137,21 @@ test_expect_success "Try to push to someone else's repository as Trusted User." ' test_expect_success "Test restore." ' + # Delete from DB echo "DELETE FROM PackageBases WHERE Name = '"'"'foobar'"'"';" | \ sqlite3 aur.db && - cat >expected <<-EOF && - user - foobar - EOF + # "Create branch" as if it had been there + new=$(git -C aur.git rev-parse HEAD^) && + echo $new > aur.git/.git/refs/heads/foobar && + # Restore deleted package SSH_ORIGINAL_COMMAND="restore foobar" AUR_USER=user AUR_PRIVILEGED=0 \ - cover "$GIT_SERVE" 2>&1 >actual + cover "$GIT_SERVE" 2>&1 && + # We should find foobar with a new ID (3) in the DB after restore + echo "SELECT ID FROM PackageBases WHERE Name = '"'"'foobar'"'"';" | \ + sqlite3 aur.db >actual && + cat >expected <<-EOF && + 3 + EOF test_cmp expected actual ' @@ -174,7 +181,7 @@ test_expect_success "Adopt a package base as a regular user." ' SSH_ORIGINAL_COMMAND="adopt foobar" AUR_USER=user AUR_PRIVILEGED=0 \ cover "$GIT_SERVE" 2>&1 && cat >expected <<-EOF && - *foobar + foobar EOF SSH_ORIGINAL_COMMAND="list-repos" AUR_USER=user AUR_PRIVILEGED=0 \ cover "$GIT_SERVE" 2>&1 >actual && @@ -252,7 +259,7 @@ test_expect_success "Try to steal another user's package as a Trusted User." ' cover "$GIT_SERVE" 2>&1 >actual && test_cmp expected actual && cat >expected <<-EOF && - *foobar + foobar EOF SSH_ORIGINAL_COMMAND="list-repos" AUR_USER=tu AUR_PRIVILEGED=1 \ cover "$GIT_SERVE" 2>&1 >actual && @@ -340,7 +347,7 @@ test_expect_success "Disown a package base and check (co-)maintainer list." ' SSH_ORIGINAL_COMMAND="disown foobar" AUR_USER=user AUR_PRIVILEGED=0 \ cover "$GIT_SERVE" 2>&1 && cat >expected <<-EOF && - *foobar + foobar EOF SSH_ORIGINAL_COMMAND="list-repos" AUR_USER=user2 AUR_PRIVILEGED=0 \ cover "$GIT_SERVE" 2>&1 >actual &&