From 783ddfffa3d17341bc199ac8cebf9bdf116835d6 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 26 Nov 2023 21:02:40 +0900 Subject: [PATCH] Make max files and lines limit in showing diff configurable (#3433) --- .../scala/gitbucket/core/api/ApiCommit.scala | 2 +- .../controller/PullRequestsController.scala | 17 ++++++++------ .../RepositoryViewerController.scala | 10 ++++++++- .../controller/SystemSettingsController.scala | 4 +++- .../core/controller/WikiController.scala | 22 +++++++++++++++++-- .../gitbucket/core/service/MergeService.scala | 5 +++-- .../core/service/PullRequestService.scala | 22 ++++++++++++++----- .../core/service/SystemSettingsService.scala | 10 +++++++-- .../core/servlet/GitRepositoryServlet.scala | 3 ++- .../scala/gitbucket/core/util/JGitUtil.scala | 10 +++++---- .../core/admin/settings_system.scala.html | 20 +++++++++++++++++ .../core/service/ServiceSpecBase.scala | 4 +++- .../core/view/AvatarImageProviderSpec.scala | 4 +++- 13 files changed, 105 insertions(+), 28 deletions(-) diff --git a/src/main/scala/gitbucket/core/api/ApiCommit.scala b/src/main/scala/gitbucket/core/api/ApiCommit.scala index 0f9d8dea6..12d986d86 100644 --- a/src/main/scala/gitbucket/core/api/ApiCommit.scala +++ b/src/main/scala/gitbucket/core/api/ApiCommit.scala @@ -29,7 +29,7 @@ case class ApiCommit( object ApiCommit { def apply(git: Git, repositoryName: RepositoryName, commit: CommitInfo): ApiCommit = { - val diffs = JGitUtil.getDiffs(git, None, commit.id, false, false) + val diffs = JGitUtil.getDiffs(git = git, from = None, to = commit.id, fetchContent = false, makePatch = false) ApiCommit( id = commit.id, message = commit.fullMessage, diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 988faa6bc..5fd39ed9a 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -128,7 +128,8 @@ trait PullRequestsControllerBase extends ControllerBase { pullreq.commitIdFrom, repository.owner, repository.name, - pullreq.commitIdTo + pullreq.commitIdTo, + context.settings ) html.conversation( @@ -165,7 +166,8 @@ trait PullRequestsControllerBase extends ControllerBase { pullreq.commitIdFrom, repository.owner, repository.name, - pullreq.commitIdTo + pullreq.commitIdTo, + context.settings ) val commitsWithStatus = commits.map { day => @@ -197,7 +199,8 @@ trait PullRequestsControllerBase extends ControllerBase { pullreq.commitIdFrom, repository.owner, repository.name, - pullreq.commitIdTo + pullreq.commitIdTo, + context.settings ) html.files( @@ -225,9 +228,8 @@ trait PullRequestsControllerBase extends ControllerBase { conflictMessage = conflictMessage, commitStatuses = getCommitStatuses(repository.owner, repository.name, pullreq.commitIdTo), branchProtection = branchProtection, - branchIsOutOfDate = JGitUtil.getShaByRef(repository.owner, repository.name, pullreq.branch) != Some( - pullreq.commitIdFrom - ), + branchIsOutOfDate = + !JGitUtil.getShaByRef(repository.owner, repository.name, pullreq.branch).contains(pullreq.commitIdFrom), needStatusCheck = context.loginAccount.forall { u => branchProtection.needStatusCheck(u.userName) }, @@ -458,7 +460,8 @@ trait PullRequestsControllerBase extends ControllerBase { oldId.getName, forkedRepository.owner, forkedRepository.name, - newId.getName + newId.getName, + context.settings ) val title = if (commits.flatten.length == 1) { diff --git a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala index fa545d931..1dab85760 100644 --- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala @@ -786,7 +786,15 @@ trait RepositoryViewerControllerBase extends ControllerBase { try { Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git => val revCommit = JGitUtil.getRevCommitFromId(git, git.getRepository.resolve(id)) - val diffs = JGitUtil.getDiffs(git, None, id, true, false) + val diffs = JGitUtil.getDiffs( + git = git, + from = None, + to = id, + fetchContent = true, + makePatch = false, + maxFiles = context.settings.repositoryViewer.maxDiffFiles, + maxLines = context.settings.repositoryViewer.maxDiffLines + ) val oldCommitId = JGitUtil.getParentCommitId(git, id) html.commit( diff --git a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala index 3be3b0860..dd734305a 100644 --- a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala @@ -121,7 +121,9 @@ trait SystemSettingsControllerBase extends AccountManagementControllerBase { "largeTimeout" -> trim(label("Timeout for large file", long(required))) )(Upload.apply), "repositoryViewer" -> mapping( - "maxFiles" -> trim(label("Max files", number(required))) + "maxFiles" -> trim(label("Max files", number(required))), + "maxDiffFiles" -> trim(label("Max diff files", number(required))), + "maxDiffLines" -> trim(label("Max diff lines", number(required))) )(RepositoryViewerSettings.apply), "defaultBranch" -> trim(label("Default branch", text(required))) )(SystemSettings.apply).verifying { settings => diff --git a/src/main/scala/gitbucket/core/controller/WikiController.scala b/src/main/scala/gitbucket/core/controller/WikiController.scala index 67b5e248b..de1e0971e 100644 --- a/src/main/scala/gitbucket/core/controller/WikiController.scala +++ b/src/main/scala/gitbucket/core/controller/WikiController.scala @@ -121,7 +121,17 @@ trait WikiControllerBase extends ControllerBase { Some(pageName), from, to, - JGitUtil.getDiffs(git, Some(from), to, true, false).filter(_.newPath == pageName + ".md"), + JGitUtil + .getDiffs( + git = git, + from = Some(from), + to = to, + fetchContent = true, + makePatch = false, + maxFiles = context.settings.repositoryViewer.maxDiffFiles, + maxLines = context.settings.repositoryViewer.maxDiffLines + ) + .filter(_.newPath == pageName + ".md"), repository, isEditable(repository), flash.get("info") @@ -137,7 +147,15 @@ trait WikiControllerBase extends ControllerBase { None, from, to, - JGitUtil.getDiffs(git, Some(from), to, true, false), + JGitUtil.getDiffs( + git = git, + from = Some(from), + to = to, + fetchContent = true, + makePatch = false, + maxFiles = context.settings.repositoryViewer.maxDiffFiles, + maxLines = context.settings.repositoryViewer.maxDiffLines + ), repository, isEditable(repository), flash.get("info") diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 5bb61c3dd..94187ab6b 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -356,7 +356,8 @@ trait MergeService { pullRequest.commitIdFrom, pullRequest.requestUserName, pullRequest.requestRepositoryName, - pullRequest.commitIdTo + pullRequest.commitIdTo, + settings ) // merge git repository @@ -384,7 +385,7 @@ trait MergeService { "merge" ) createComment(repository.owner, repository.name, loginAccount.userName, issueId, "Close", "close") - updateClosed(repository.owner, repository.name, issueId, true) + updateClosed(repository.owner, repository.name, issueId, closed = true) // record activity val mergeInfo = diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index 445c9bfc0..7cf6f138a 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -280,7 +280,8 @@ trait PullRequestService { pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo, - commitIdTo + commitIdTo, + settings ) // Update commit id in the PULL_REQUEST table @@ -400,10 +401,12 @@ trait PullRequestService { userName: String, repositoryName: String, oldCommitId: String, - newCommitId: String + newCommitId: String, + settings: SystemSettings )(implicit s: Session): Unit = { - val (_, diffs) = getRequestCompareInfo(userName, repositoryName, oldCommitId, userName, repositoryName, newCommitId) + val (_, diffs) = + getRequestCompareInfo(userName, repositoryName, oldCommitId, userName, repositoryName, newCommitId, settings) val patchs = positions.map { case (file, _) => diffs @@ -503,7 +506,8 @@ trait PullRequestService { branch: String, requestUserName: String, requestRepositoryName: String, - requestCommitId: String + requestCommitId: String, + settings: SystemSettings ): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = Using.resources( Git.open(getRepositoryDir(userName, repositoryName)), @@ -526,7 +530,15 @@ trait PullRequestService { } // TODO Isolate to an another method? - val diffs = JGitUtil.getDiffs(newGit, Some(oldId.getName), newId.getName, true, false) + val diffs = JGitUtil.getDiffs( + git = newGit, + from = Some(oldId.getName), + to = newId.getName, + fetchContent = true, + makePatch = false, + maxFiles = settings.repositoryViewer.maxDiffFiles, + maxLines = settings.repositoryViewer.maxDiffLines + ) (commits, diffs) } diff --git a/src/main/scala/gitbucket/core/service/SystemSettingsService.scala b/src/main/scala/gitbucket/core/service/SystemSettingsService.scala index a135aa250..17b9f0301 100644 --- a/src/main/scala/gitbucket/core/service/SystemSettingsService.scala +++ b/src/main/scala/gitbucket/core/service/SystemSettingsService.scala @@ -90,6 +90,8 @@ trait SystemSettingsService { props.setProperty(UploadLargeMaxFileSize, settings.upload.largeMaxFileSize.toString) props.setProperty(UploadLargeTimeout, settings.upload.largeTimeout.toString) props.setProperty(RepositoryViewerMaxFiles, settings.repositoryViewer.maxFiles.toString) + props.setProperty(RepositoryViewerMaxDiffFiles, settings.repositoryViewer.maxDiffFiles.toString) + props.setProperty(RepositoryViewerMaxDiffLines, settings.repositoryViewer.maxDiffLines.toString) props.setProperty(DefaultBranch, settings.defaultBranch) Using.resource(new java.io.FileOutputStream(GitBucketConf)) { out => @@ -205,7 +207,9 @@ trait SystemSettingsService { getValue(props, UploadLargeTimeout, 3 * 10000) ), RepositoryViewerSettings( - getValue(props, RepositoryViewerMaxFiles, 0) + getValue(props, RepositoryViewerMaxFiles, 0), + getValue(props, RepositoryViewerMaxDiffFiles, 100), + getValue(props, RepositoryViewerMaxDiffLines, 1000) ), getValue(props, DefaultBranch, "main") ) @@ -384,7 +388,7 @@ object SystemSettingsService { case class Upload(maxFileSize: Long, timeout: Long, largeMaxFileSize: Long, largeTimeout: Long) - case class RepositoryViewerSettings(maxFiles: Int) + case class RepositoryViewerSettings(maxFiles: Int, maxDiffFiles: Int, maxDiffLines: Int) val GenericSshUser = "git" val PublicSshPort = 22 @@ -450,6 +454,8 @@ object SystemSettingsService { private val UploadLargeMaxFileSize = "upload.largeMaxFileSize" private val UploadLargeTimeout = "upload.largeTimeout" private val RepositoryViewerMaxFiles = "repository_viewer_max_files" + private val RepositoryViewerMaxDiffFiles = "repository_viewer_max_diff_files" + private val RepositoryViewerMaxDiffLines = "repository_viewer_max_diff_lines" private val DefaultBranch = "default_branch" private def getValue[A: ClassTag](props: java.util.Properties, key: String, default: A): A = { diff --git a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala index a49d2cbe8..3e8df696e 100644 --- a/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala +++ b/src/main/scala/gitbucket/core/servlet/GitRepositoryServlet.scala @@ -498,7 +498,8 @@ class WikiCommitHook(owner: String, repository: String, pusher: String, baseUrl: commitIds.foreach { case (oldCommitId, newCommitId) => val commits = Using.resource(Git.open(Directory.getWikiRepositoryDir(owner, repository))) { git => JGitUtil.getCommitLog(git, oldCommitId, newCommitId).flatMap { commit => - val diffs = JGitUtil.getDiffs(git, None, commit.id, false, false) + val diffs = + JGitUtil.getDiffs(git = git, from = None, to = commit.id, fetchContent = false, makePatch = false) diffs.collect { case diff if diff.newPath.toLowerCase.endsWith(".md") => val action = mapToAction(diff.changeType) diff --git a/src/main/scala/gitbucket/core/util/JGitUtil.scala b/src/main/scala/gitbucket/core/util/JGitUtil.scala index eeefd6d1f..1f4e5013d 100644 --- a/src/main/scala/gitbucket/core/util/JGitUtil.scala +++ b/src/main/scala/gitbucket/core/util/JGitUtil.scala @@ -762,11 +762,13 @@ object JGitUtil { from: Option[String], to: String, fetchContent: Boolean, - makePatch: Boolean + makePatch: Boolean, + maxFiles: Int = 100, + maxLines: Int = 1000 ): List[DiffInfo] = { val diffs = getDiffEntries(git, from, to) diffs.map { diff => - if (diffs.size > 100) { // Don't show diff if there are more than 100 files + if (maxFiles > 0 && diffs.size > maxFiles) { // Don't show diff if there are more than maxFiles DiffInfo( changeType = diff.getChangeType, oldPath = diff.getOldPath, @@ -787,8 +789,8 @@ object JGitUtil { val newIsImage = FileUtil.isImage(diff.getNewPath) val patch = if (oldIsImage || newIsImage) None else Some(makePatchFromDiffEntry(git, diff)) // TODO use DiffFormatter - val tooLarge = - patch.exists(_.count(_ == '\n') > 1000) // Don't show diff if the file has more than 1000 lines diff + val tooLarge = maxLines > 0 && + patch.exists(_.count(_ == '\n') > maxLines) // Don't show diff if the file has more than maxLines of diff val includeContent = tooLarge || !fetchContent || oldIsImage || newIsImage DiffInfo( changeType = diff.getChangeType, diff --git a/src/main/twirl/gitbucket/core/admin/settings_system.scala.html b/src/main/twirl/gitbucket/core/admin/settings_system.scala.html index 267959d59..bda508164 100644 --- a/src/main/twirl/gitbucket/core/admin/settings_system.scala.html +++ b/src/main/twirl/gitbucket/core/admin/settings_system.scala.html @@ -321,6 +321,26 @@

+
+ +
+ + +

+ Avoid showing diff to prevent browser freezes if number of files in diff is bigger than this value. 0 or negative number means no limitation. +

+
+
+
+ +
+ + +

+ Avoid showing diff to prevent browser freezes if lines of diff in a file is bigger than this value. 0 or negative number means no limitation. +

+
+