Make max files and lines limit in showing diff configurable (#3433)

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,

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) {

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(

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 =>

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")

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 =

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)
     }

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 = {

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)

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,

src/main/twirl/gitbucket/core/admin/settings_system.scala.html

@@ -321,6 +321,26 @@
       </p>
     </div>
   </div>
+  <div class="form-group">
+    <label class="control-label col-md-2" for="repositoryViewerMaxDiffFiles">Max files in diff</label>
+    <div class="col-md-10">
+      <input type="text" name="repositoryViewer.maxDiffFiles" id="repositoryViewerMaxDiffFiles" class="form-control" value="@context.settings.repositoryViewer.maxDiffFiles"/>
+      <span id="error-repositoryViewerMaxDiffFiles" class="error"></span>
+      <p class="muted">
+        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.
+      </p>
+    </div>
+  </div>
+  <div class="form-group">
+    <label class="control-label col-md-2" for="repositoryViewerMaxDiffLines">Max lines in diff</label>
+    <div class="col-md-10">
+      <input type="text" name="repositoryViewer.maxDiffLines" id="repositoryViewerMaxDiffLines" class="form-control" value="@context.settings.repositoryViewer.maxDiffLines"/>
+      <span id="error-repositoryViewerMaxDiffLines" class="error"></span>
+      <p class="muted">
+        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.
+      </p>
+    </div>
+  </div>
 </fieldset>
 
 <script>

src/test/scala/gitbucket/core/service/ServiceSpecBase.scala

@@ -80,7 +80,9 @@ trait ServiceSpecBase {
         largeTimeout = 30 * 10000
       ),
       repositoryViewer = RepositoryViewerSettings(
-        maxFiles = 0
+        maxFiles = 0,
+        maxDiffFiles = 100,
+        maxDiffLines = 1000
       ),
       defaultBranch = "main"
     )

src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala

@@ -192,7 +192,9 @@ class AvatarImageProviderSpec extends AnyFunSpec {
         largeTimeout = 30 * 10000
       ),
       repositoryViewer = RepositoryViewerSettings(
-        maxFiles = 0
+        maxFiles = 0,
+        maxDiffFiles = 100,
+        maxDiffLines = 1000
       ),
       "main"
     )