Merge branch 'implement-668' of https://github.com/beraboris/gitbucket into beraboris-implement-668

src/main/scala/gitbucket/core/view/helpers.scala

@@ -4,7 +4,7 @@ import java.text.SimpleDateFormat
 import java.util.{Date, Locale, TimeZone}
 
 import gitbucket.core.controller.Context
-import gitbucket.core.model.CommitState
+import gitbucket.core.model.{CommitState, Repository}
 import gitbucket.core.plugin.{RenderRequest, PluginRegistry, Renderer}
 import gitbucket.core.service.{RepositoryService, RequestCache}
 import gitbucket.core.util.{FileUtil, JGitUtil, StringUtil}
@@ -165,10 +165,28 @@ object helpers extends AvatarImageProvider with LinkConverter with RequestCache
    */
   def encodeRefName(value: String): String = StringUtil.urlEncode(value).replace("%2F", "/")
 
+  /**
+   * Url encode except '/' and ':'
+   */
+  def encodeCompareBranch(value: String) =
+    StringUtil.urlEncode(value).replace("%2F", "/").replace("%3A", ":")
+
   def urlEncode(value: String): String = StringUtil.urlEncode(value)
 
   def urlEncode(value: Option[String]): String = value.map(urlEncode).getOrElse("")
 
+  /**
+   * The default origin (branch or remote:branch pair) branches are compared to.
+   *
+   * There are two cases: when the repo is a fork and when the repo is not a
+   * fork.
+   *
+   * For a fork, the default ref is parentUserName:defaultBranch.
+   * For a non fork, the default ref is defaultBranch.
+   */
+  def repositoryDefaultCompareOrigin(repo: Repository): String =
+    repo.parentUserName.map(n => s"$n:${repo.defaultBranch}").getOrElse(repo.defaultBranch)
+
   /**
    * Generates the url to the repository.
    */

src/main/twirl/gitbucket/core/repo/branches.scala.html

@@ -30,9 +30,9 @@
                   }
                 }.getOrElse{
                   @if(context.loginAccount.isDefined){
-                    <a href="@url(repository)/compare/@{encodeRefName(repository.repository.defaultBranch)}...@{encodeRefName(branch.name)}?expand=1" class="btn btn-small">New Pull Request</a>
+                    <a href="@url(repository)/compare/@{encodeCompareBranch(repositoryDefaultCompareOrigin(repository.repository))}...@{encodeCompareBranch(branch.name)}?expand=1" class="btn btn-small">New Pull Request</a>
                   }else{
-                    <a href="@url(repository)/compare/@{encodeRefName(repository.repository.defaultBranch)}...@{encodeRefName(branch.name)}" class="btn btn-small">Compare</a>
+                    <a href="@url(repository)/compare/@{encodeCompareBranch(repositoryDefaultCompareOrigin(repository.repository))}...@{encodeCompareBranch(branch.name)}" class="btn btn-small">Compare</a>
                   }
                 }
                 @if(hasWritePermission){

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

@@ -0,0 +1,51 @@
+package gitbucket.core.view
+
+import org.specs2.mutable._
+import gitbucket.core.model.Repository
+import java.util.Date
+
+class HelpersSpec extends Specification {
+  def repository(defaultBranch: String, parentUserName: Option[String]) =
+    Repository(
+      userName = "some-user",
+      repositoryName = "some-repo",
+      isPrivate = false,
+      description = None,
+      defaultBranch = defaultBranch,
+      parentUserName = parentUserName,
+      parentRepositoryName = Some("some-repo"),
+      registeredDate = new Date(),
+      updatedDate = new Date(),
+      lastActivityDate = new Date(),
+      originUserName = Some("some-other-user"),
+      originRepositoryName = Some("some-repo")
+    )
+
+  "repositoryDefaultCompareOrigin" should {
+    "return default branch when not fork" in {
+      val repo = repository("master", None)
+
+      helpers.repositoryDefaultCompareOrigin(repo) mustEqual "master"
+    }
+
+    "return [upstream]:[branch] when a fork" in {
+      val repo = repository("some-branch", Some("parent-user"))
+
+      helpers.repositoryDefaultCompareOrigin(repo) mustEqual "parent-user:some-branch"
+    }
+  }
+
+  "encodeCompareBranch" should {
+    "not uri encode /" in {
+      helpers.encodeCompareBranch("foo/bar#baz") mustEqual "foo/bar%23baz"
+    }
+
+    "not uri encode :" in {
+      helpers.encodeCompareBranch("foo:bar#baz") mustEqual "foo:bar%23baz"
+    }
+
+    "uri encode special characters" in {
+      helpers.encodeCompareBranch("!#$&'()+,;=?@[]") mustEqual "%21%23%24%26%27%28%29%2B%2C%3B%3D%3F%40%5B%5D"
+    }
+  }
+}