diff --git a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala index 533437a..7443c3e 100644 --- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala @@ -329,50 +329,12 @@ }) post("/:owner/:repository/upload", uploadForm)(writableUsersOnly { (form, repository) => - context.withLoginAccount { - loginAccount => - val files = form.uploadFiles - .split("\n") - .map { line => - val i = line.indexOf(':') - CommitFile(line.substring(0, i).trim, line.substring(i + 1).trim) - } - .toSeq - - val newFiles = files.map { file => - file.copy(name = if (form.path.length == 0) file.name else s"${form.path}/${file.name}") - } - - if (form.newBranch) { - val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) - val objectId = _commit(newBranchName, newFiles, loginAccount) - val issueId = - createIssueAndPullRequest( - repository, - form.branch, - newBranchName, - form.commit, - objectId.name, - form.message, - loginAccount - ) - redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") - } else { - _commit(form.branch, newFiles, loginAccount) - if (form.path.length == 0) { - redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}") - } else { - redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}/${form.path}") - } - } - } - def _commit( branchName: String, //files: Seq[CommitFile], newFiles: Seq[CommitFile], loginAccount: Account - ): ObjectId = { + ): Either[String, ObjectId] = { commitFiles( repository = repository, branch = branchName, @@ -399,6 +361,52 @@ } } } + + context.withLoginAccount { + loginAccount => + val files = form.uploadFiles + .split("\n") + .map { line => + val i = line.indexOf(':') + CommitFile(line.substring(0, i).trim, line.substring(i + 1).trim) + } + .toSeq + + val newFiles = files.map { file => + file.copy(name = if (form.path.length == 0) file.name else s"${form.path}/${file.name}") + } + + if (form.newBranch) { + val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) + _commit(newBranchName, newFiles, loginAccount) match { + case Right(objectId) => + val issueId = + createIssueAndPullRequest( + repository, + form.branch, + newBranchName, + form.commit, + objectId.name, + form.message, + loginAccount + ) + redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } + } else { + _commit(form.branch, newFiles, loginAccount) match { + case Right(_) => + if (form.path.length == 0) { + redirect(s"/${repository.owner}/${repository.name}/tree/${encodeRefName(form.branch)}") + } else { + redirect( + s"/${repository.owner}/${repository.name}/tree/${encodeRefName(form.branch)}/${encodeRefName(form.path)}" + ) + } + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } + } + } }) get("/:owner/:repository/edit/*")(writableUsersOnly { repository => @@ -456,32 +464,7 @@ }) post("/:owner/:repository/create", editorForm)(writableUsersOnly { (form, repository) => - context.withLoginAccount { - loginAccount => - if (form.newBranch) { - val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) - val objectId = _commit(newBranchName, loginAccount) - val issueId = - createIssueAndPullRequest( - repository, - form.branch, - newBranchName, - form.commit, - objectId.name, - form.message, - loginAccount - ) - redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") - } else { - _commit(form.branch, loginAccount) - redirect( - s"/${repository.owner}/${repository.name}/blob/${form.branch}/${if (form.path.length == 0) urlEncode(form.newFileName) - else s"${form.path}/${urlEncode(form.newFileName)}"}" - ) - } - } - - def _commit(branchName: String, loginAccount: Account): ObjectId = { + def _commit(branchName: String, loginAccount: Account): Either[String, ObjectId] = { commitFile( repository = repository, branch = branchName, @@ -494,37 +477,48 @@ commit = form.commit, loginAccount = loginAccount, settings = context.settings - )._1 + ).map(_._1) } - }) - post("/:owner/:repository/update", editorForm)(writableUsersOnly { (form, repository) => context.withLoginAccount { loginAccount => if (form.newBranch) { val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) - val objectId = _commit(newBranchName, loginAccount) - val issueId = - createIssueAndPullRequest( - repository, - form.branch, - newBranchName, - form.commit, - objectId.name, - form.message, - loginAccount - ) - redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + _commit(newBranchName, loginAccount) match { + case Right(objectId) => + val issueId = + createIssueAndPullRequest( + repository, + form.branch, + newBranchName, + form.commit, + objectId.name, + form.message, + loginAccount + ) + redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } } else { - _commit(form.branch, loginAccount) - redirect( - s"/${repository.owner}/${repository.name}/blob/${urlEncode(form.branch)}/${if (form.path.length == 0) urlEncode(form.newFileName) - else s"${form.path}/${urlEncode(form.newFileName)}"}" - ) + _commit(form.branch, loginAccount) match { + case Right(_) => + if (form.path.length == 0) { + redirect( + s"/${repository.owner}/${repository.name}/blob/${encodeRefName(form.branch)}/${urlEncode(form.newFileName)}" + ) + } else { + redirect( + s"/${repository.owner}/${repository.name}/blob/${encodeRefName(form.branch)}/${encodeRefName(form.path)}/${urlEncode(form.newFileName)}" + ) + } + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } } } + }) - def _commit(branchName: String, loginAccount: Account): ObjectId = { + post("/:owner/:repository/update", editorForm)(writableUsersOnly { (form, repository) => + def _commit(branchName: String, loginAccount: Account): Either[String, ObjectId] = { commitFile( repository = repository, branch = branchName, @@ -541,37 +535,48 @@ commit = form.commit, loginAccount = loginAccount, settings = context.settings - )._1 + ).map(_._1) } - }) - post("/:owner/:repository/remove", deleteForm)(writableUsersOnly { (form, repository) => context.withLoginAccount { loginAccount => if (form.newBranch) { val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) - val objectId = _commit(newBranchName, loginAccount) - val issueId = - createIssueAndPullRequest( - repository, - form.branch, - newBranchName, - form.commit, - objectId.name, - form.message, - loginAccount - ) - redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + _commit(newBranchName, loginAccount) match { + case Right(objectId) => + val issueId = + createIssueAndPullRequest( + repository, + form.branch, + newBranchName, + form.commit, + objectId.name, + form.message, + loginAccount + ) + redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } } else { - _commit(form.branch, loginAccount) - redirect( - s"/${repository.owner}/${repository.name}/tree/${form.branch}${if (form.path.length == 0) "" - else "/" + form.path}" - ) + _commit(form.branch, loginAccount) match { + case Right(_) => + if (form.path.length == 0) { + redirect( + s"/${repository.owner}/${repository.name}/blob/${encodeRefName(form.branch)}/${urlEncode(form.newFileName)}" + ) + } else { + redirect( + s"/${repository.owner}/${repository.name}/blob/${encodeRefName(form.branch)}/${encodeRefName(form.path)}/${urlEncode(form.newFileName)}" + ) + } + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } } } + }) - def _commit(branchName: String, loginAccount: Account): ObjectId = { + post("/:owner/:repository/remove", deleteForm)(writableUsersOnly { (form, repository) => + def _commit(branchName: String, loginAccount: Account): Either[String, ObjectId] = { commitFile( repository = repository, branch = branchName, @@ -584,7 +589,41 @@ commit = form.commit, loginAccount = loginAccount, settings = context.settings - )._1 + ).map(_._1) + } + + context.withLoginAccount { + loginAccount => + if (form.newBranch) { + val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount) + _commit(newBranchName, loginAccount) match { + case Right(objectId) => + val issueId = + createIssueAndPullRequest( + repository, + form.branch, + newBranchName, + form.commit, + objectId.name, + form.message, + loginAccount + ) + redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } + } else { + _commit(form.branch, loginAccount) match { + case Right(_) => + if (form.path.length == 0) { + redirect(s"/${repository.owner}/${repository.name}/tree/${encodeRefName(form.branch)}") + } else { + redirect( + s"/${repository.owner}/${repository.name}/tree/${encodeRefName(form.branch)}/${encodeRefName(form.path)}" + ) + } + case Left(error) => Forbidden(gitbucket.core.html.error(error)) + } + } } }) diff --git a/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala b/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala index 94d8ede..272c5cc 100644 --- a/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala +++ b/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala @@ -157,7 +157,7 @@ Some("https://docs.github.com/en/rest/reference/repos#create-or-update-file-contents") ) case _ => - val (commitId, blobId) = commitFile( + commitFile( repository, branch, path, @@ -170,12 +170,12 @@ data.committer.map(_.name).getOrElse(loginAccount.fullName), data.committer.map(_.email).getOrElse(loginAccount.mailAddress), context.settings - ) - - blobId match { - case None => + ) match { + case Left(error) => + ApiError(s"Failed to commit a file: ${error}", None) + case Right((_, None)) => ApiError("Failed to commit a file.", None) - case Some(blobId) => + case Right((commitId, Some(blobId))) => Map( "content" -> ApiContents( "file", diff --git a/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala b/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala index ccb4d11..590ca59 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala @@ -36,10 +36,10 @@ settings: SystemSettings )( f: (Git, ObjectId, DirCacheBuilder, ObjectInserter) => Unit - )(implicit s: Session, c: JsonFormat.Context): ObjectId = { + )(implicit s: Session, c: JsonFormat.Context): Either[String, ObjectId] = { _createFiles(repository, branch, message, loginAccount, loginAccount.fullName, loginAccount.mailAddress, settings)( f - )._1 + ).map(_._1) } /** @@ -58,7 +58,7 @@ commit: String, loginAccount: Account, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): (ObjectId, Option[ObjectId]) = { + )(implicit s: Session, c: JsonFormat.Context): Either[String, (ObjectId, Option[ObjectId])] = { commitFile( repository, branch, @@ -92,7 +92,7 @@ committerName: String, committerMailAddress: String, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): (ObjectId, Option[ObjectId]) = { + )(implicit s: Session, c: JsonFormat.Context): Either[String, (ObjectId, Option[ObjectId])] = { val newPath = newFileName.map { newFileName => if (path.length == 0) newFileName else s"${path}/${newFileName}" @@ -141,7 +141,7 @@ settings: SystemSettings )( f: (Git, ObjectId, DirCacheBuilder, ObjectInserter) => R - )(implicit s: Session, c: JsonFormat.Context): (ObjectId, R) = { + )(implicit s: Session, c: JsonFormat.Context): Either[String, (ObjectId, R)] = { LockUtil.lock(s"${repository.owner}/${repository.name}") { Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git => @@ -177,11 +177,11 @@ error match { case Some(error) => // commit is rejected - // TODO Notify commit failure to edited user val refUpdate = git.getRepository.updateRef(headName) refUpdate.setNewObjectId(headTip) refUpdate.setForceUpdate(true) refUpdate.update() + Left(error) case None => // update refs @@ -242,8 +242,8 @@ ) } } + Right((commitId, result)) } - (commitId, result) } } } diff --git a/src/main/scala/gitbucket/core/util/StringUtil.scala b/src/main/scala/gitbucket/core/util/StringUtil.scala index 4a11b56..724ab98 100644 --- a/src/main/scala/gitbucket/core/util/StringUtil.scala +++ b/src/main/scala/gitbucket/core/util/StringUtil.scala @@ -74,6 +74,11 @@ def urlDecode(value: String): String = URLDecoder.decode(value, "UTF-8") + /** + * URL encode except '/'. + */ + def encodeRefName(value: String): String = urlEncode(value).replace("%2F", "/") + def splitWords(value: String): Array[String] = value.split("[ \\t　]+") def isInteger(value: String): Boolean = allCatch opt { value.toInt } map (_ => true) getOrElse (false) diff --git a/src/main/scala/gitbucket/core/view/helpers.scala b/src/main/scala/gitbucket/core/view/helpers.scala index d471c73..e686481 100644 --- a/src/main/scala/gitbucket/core/view/helpers.scala +++ b/src/main/scala/gitbucket/core/view/helpers.scala @@ -276,7 +276,7 @@ /** * URL encode except '/'. */ - def encodeRefName(value: String): String = StringUtil.urlEncode(value).replace("%2F", "/") + def encodeRefName(value: String): String = StringUtil.encodeRefName(value) def urlEncode(value: String): String = StringUtil.urlEncode(value)