Merge pull request #153 from xuwei-k/number-format-exception

avoid NumberFormatException

src/main/scala/app/IssuesController.scala

@@ -228,13 +228,13 @@ trait IssuesControllerBase extends ControllerBase {
   })
 
   post("/:owner/:repository/issues/batchedit/label")(collaboratorsOnly { repository =>
-    defining(params("value").toInt){ labelId =>
+    params("value").toIntOpt.map{ labelId =>
       executeBatch(repository) { issueId =>
         getIssueLabel(repository.owner, repository.name, issueId, labelId) getOrElse {
           registerIssueLabel(repository.owner, repository.name, issueId, labelId)
         }
       }
-    }
+    } getOrElse NotFound
   })
 
   post("/:owner/:repository/issues/batchedit/assign")(collaboratorsOnly { repository =>
@@ -254,7 +254,7 @@ trait IssuesControllerBase extends ControllerBase {
   })
 
   val assignedUserName = (key: String) => params.get(key) filter (_.trim != "")
-  val milestoneId      = (key: String) => params.get(key) collect { case x if x.trim != "" => x.toInt }
+  val milestoneId: String => Option[Int] = (key: String) => params.get(key).flatMap(_.toIntOpt)
 
   private def isEditable(owner: String, repository: String, author: String)(implicit context: app.Context): Boolean =
     hasWritePermission(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName

src/main/scala/app/MilestonesController.scala

@@ -4,6 +4,7 @@ import jp.sf.amateras.scalatra.forms._
 
 import service._
 import util.{CollaboratorsAuthenticator, ReferrerAuthenticator}
+import util.Implicits._
 
 class MilestonesController extends MilestonesControllerBase
   with MilestonesService with RepositoryService with AccountService
@@ -39,34 +40,44 @@ trait MilestonesControllerBase extends ControllerBase {
   })
 
   get("/:owner/:repository/issues/milestones/:milestoneId/edit")(collaboratorsOnly { repository =>
-    issues.milestones.html.edit(getMilestone(repository.owner, repository.name, params("milestoneId").toInt), repository)
+    params("milestoneId").toIntOpt.map{ milestoneId =>
+      issues.milestones.html.edit(getMilestone(repository.owner, repository.name, milestoneId), repository)
+    } getOrElse NotFound
   })
 
   post("/:owner/:repository/issues/milestones/:milestoneId/edit", milestoneForm)(collaboratorsOnly { (form, repository) =>
-    getMilestone(repository.owner, repository.name, params("milestoneId").toInt).map { milestone =>
-      updateMilestone(milestone.copy(title = form.title, description = form.description, dueDate = form.dueDate))
-      redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+    params("milestoneId").toIntOpt.flatMap{ milestoneId =>
+      getMilestone(repository.owner, repository.name, milestoneId).map { milestone =>
+        updateMilestone(milestone.copy(title = form.title, description = form.description, dueDate = form.dueDate))
+        redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+      }
     } getOrElse NotFound
   })
 
   get("/:owner/:repository/issues/milestones/:milestoneId/close")(collaboratorsOnly { repository =>
-    getMilestone(repository.owner, repository.name, params("milestoneId").toInt).map { milestone =>
-      closeMilestone(milestone)
-      redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+    params("milestoneId").toIntOpt.flatMap{ milestoneId =>
+      getMilestone(repository.owner, repository.name, milestoneId).map { milestone =>
+        closeMilestone(milestone)
+        redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+      }
     } getOrElse NotFound
   })
 
   get("/:owner/:repository/issues/milestones/:milestoneId/open")(collaboratorsOnly { repository =>
-    getMilestone(repository.owner, repository.name, params("milestoneId").toInt).map { milestone =>
-      openMilestone(milestone)
-      redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+    params("milestoneId").toIntOpt.flatMap{ milestoneId =>
+      getMilestone(repository.owner, repository.name, milestoneId).map { milestone =>
+        openMilestone(milestone)
+        redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+      }
     } getOrElse NotFound
   })
 
   get("/:owner/:repository/issues/milestones/:milestoneId/delete")(collaboratorsOnly { repository =>
-    getMilestone(repository.owner, repository.name, params("milestoneId").toInt).map { milestone =>
-      deleteMilestone(repository.owner, repository.name, milestone.milestoneId)
-      redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+    params("milestoneId").toIntOpt.flatMap{ milestoneId =>
+      getMilestone(repository.owner, repository.name, milestoneId).map { milestone =>
+        deleteMilestone(repository.owner, repository.name, milestone.milestoneId)
+        redirect(s"/${repository.owner}/${repository.name}/issues/milestones")
+      }
     } getOrElse NotFound
   })
 

src/main/scala/app/PullRequestsController.scala

@@ -63,7 +63,9 @@ trait PullRequestsControllerBase extends ControllerBase {
   })
 
   get("/:owner/:repository/pull/:id")(referrersOnly { repository =>
-    defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) =>
+    params("id").toIntOpt.flatMap{ issueId =>
+      val owner = repository.owner
+      val name = repository.name
       getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
         using(Git.open(getRepositoryDir(owner, name))){ git =>
           val (commits, diffs) =
@@ -71,7 +73,7 @@ trait PullRequestsControllerBase extends ControllerBase {
 
           pulls.html.pullreq(
             issue, pullreq,
-            getComments(owner, name, issueId.toInt),
+            getComments(owner, name, issueId),
             (getCollaborators(owner, name) :+ owner).sorted,
             getMilestonesWithIssueCount(owner, name),
             commits,
@@ -79,23 +81,27 @@ trait PullRequestsControllerBase extends ControllerBase {
             hasWritePermission(owner, name, context.loginAccount),
             repository)
         }
-      } getOrElse NotFound
-    }
+      }
+    } getOrElse NotFound
   })
 
   ajaxGet("/:owner/:repository/pull/:id/mergeguide")(collaboratorsOnly { repository =>
-    defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) =>
+    params("id").toIntOpt.flatMap{ issueId =>
+      val owner = repository.owner
+      val name = repository.name
       getPullRequest(owner, name, issueId) map { case(issue, pullreq) =>
         pulls.html.mergeguide(
           checkConflict(owner, name, pullreq.branch, owner, name, pullreq.requestBranch),
           pullreq,
           s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git")
-      } getOrElse NotFound()
-    }
+      }
+    } getOrElse NotFound
   })
 
   post("/:owner/:repository/pull/:id/merge", mergeForm)(collaboratorsOnly { (form, repository) =>
-    defining(repository.owner, repository.name, params("id").toInt){ case (owner, name, issueId) =>
+    params("id").toIntOpt.flatMap{ issueId =>
+      val owner = repository.owner
+      val name = repository.name
       LockUtil.lock(s"${owner}/${name}/merge"){
         getPullRequest(owner, name, issueId).map { case (issue, pullreq) =>
           val remote = getRepositoryDir(owner, name)
@@ -160,9 +166,9 @@ trait PullRequestsControllerBase extends ControllerBase {
               redirect(s"/${owner}/${name}/pull/${issueId}")
             }
           }
-        } getOrElse NotFound
+        }
       }
-    }
+    } getOrElse NotFound
   })
 
   get("/:owner/:repository/compare")(referrersOnly { forkedRepository =>

src/main/scala/app/RepositoryViewerController.scala

@@ -56,7 +56,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
    */
   get("/:owner/:repository/commits/*")(referrersOnly { repository =>
     val (branchName, path) = splitPath(repository, multiParams("splat").head)
-    val page = params.getOrElse("page", "1").toInt
+    val page = params.get("page").flatMap(_.toIntOpt).getOrElse(1)
 
     using(Git.open(getRepositoryDir(repository.owner, repository.name))){ git =>
       JGitUtil.getCommitLog(git, branchName, page, 30, path) match {
@@ -257,4 +257,4 @@ trait RepositoryViewerControllerBase extends ControllerBase {
     }
   }
   
-}
+}

src/main/scala/service/IssuesService.scala

@@ -350,10 +350,10 @@ object IssuesService {
     def apply(request: HttpServletRequest): IssueSearchCondition =
       IssueSearchCondition(
         param(request, "labels").map(_.split(" ").toSet).getOrElse(Set.empty),
-        param(request, "milestone").map(_ match {
+        param(request, "milestone").map{
           case "none" => None
-          case x      => Some(x.toInt)
-        }),
+          case x      => x.toIntOpt
+        },
         param(request, "for"),
         param(request, "state",     Seq("open", "closed")).getOrElse("open"),
         param(request, "sort",      Seq("created", "comments", "updated")).getOrElse("created"),

src/main/scala/util/Implicits.scala

@@ -41,6 +41,12 @@ object Implicits {
       }
       sb.toString
     }
+
+    def toIntOpt: Option[Int] = try {
+      Option(Integer.parseInt(value))
+    } catch {
+      case e: NumberFormatException => None
+    }
   }
 
   implicit class RichRequest(request: HttpServletRequest){
@@ -69,4 +75,4 @@ object Implicits {
     }
   }
 
-}
+}