From eb715735bbb79e5edd39e29c1b0e5587d2d3712c Mon Sep 17 00:00:00 2001
From: Ritwik Puri <ritwikpuri5678@gmail.com>
Date: Wed, 27 Apr 2022 20:17:04 +0530
Subject: [PATCH] fix: Misc fixes (#1294)

* fix: use better api endpoint for is_valid_frappe_branch

* fix: dont allow appnames starting with number and/or dot in them

* fix: dont convert response to json

* fix: using ls-remote to check for valid branches

* refactor: changed the message displayed on invalid branch and invalid frappe path

Co-authored-by: Aradhya <aradhyatripathi51@gmail.com>
Co-authored-by: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com>
---
 bench/app.py            |  7 +++++++
 bench/utils/__init__.py | 40 +++++++++++++++++++++-------------------
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/bench/app.py b/bench/app.py
index de5b1d92..e0da4e18 100755
--- a/bench/app.py
+++ b/bench/app.py
@@ -486,6 +486,13 @@ def new_app(app, no_git=None, bench_path="."):
 
 	# For backwards compatibility
 	app = app.lower().replace(" ", "_").replace("-", "_")
+	if app[0].isdigit() or "." in app:
+		click.secho(
+			"App names cannot start with numbers(digits) or have dot(.) in them",
+			fg="red"
+		)
+		return
+
 	apps = os.path.abspath(os.path.join(bench_path, "apps"))
 	args = ["make-app", apps, app]
 	if no_git:
diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py
index 45cd1dce..d0bb0383 100644
--- a/bench/utils/__init__.py
+++ b/bench/utils/__init__.py
@@ -51,10 +51,9 @@ def is_frappe_app(directory: str) -> bool:
 
 
 def is_valid_frappe_branch(frappe_path:str, frappe_branch:str):
-	""" Check if a branch exists in a repo. Throws InvalidRemoteException if branch is not found
+	"""Check if a branch exists in a repo. Throws InvalidRemoteException if branch is not found
 
-	Uses github's api without auth to query branch.
-	If rate limited by gitapi, requests are sent to github.com
+	Uses native git command to check for branches on a remote.
 
 	:param frappe_path: git url
 	:type frappe_path: str
@@ -62,23 +61,26 @@ def is_valid_frappe_branch(frappe_path:str, frappe_branch:str):
 	:type frappe_branch: str
 	:raises InvalidRemoteException: branch for this repo doesn't exist
 	"""
-	if "http" in frappe_path and frappe_branch:
-		frappe_path = frappe_path.replace(".git", "")
+	import subprocess
+
+	if frappe_branch:
 		try:
-			owner, repo = frappe_path.split("/")[3], frappe_path.split("/")[4]
-		except IndexError:
-			raise InvalidRemoteException("Invalid git url")
-		git_api_req = f"https://api.github.com/repos/{owner}/{repo}/branches"
-		res = requests.get(git_api_req).json()
-
-		if "message" in res:
-			# slower alternative with no rate limit
-			github_req = f'https://github.com/{owner}/{repo}/tree/{frappe_branch}'
-			if requests.get(github_req).status_code != 200:
-				raise InvalidRemoteException("Invalid git url")
-
-		elif frappe_branch not in [x["name"] for x in res]:
-			raise InvalidRemoteException("Frappe branch does not exist")
+			ret = subprocess.check_output(
+				(
+					"git",
+					"ls-remote",
+					"--heads",
+					frappe_path,
+					frappe_branch,
+				),
+				encoding="UTF-8",
+			)
+			if not ret:
+				raise InvalidRemoteException(
+					f"Invalid {frappe_branch} for the remote {frappe_path}"
+				)
+		except subprocess.CalledProcessError:
+			raise InvalidRemoteException(f"Invalid frappe path {frappe_path}")
 
 
 def log(message, level=0, no_log=False):