azkaban-developers

diff --git a/azkaban-common/src/main/java/azkaban/utils/Utils.java b/azkaban-common/src/main/java/azkaban/utils/Utils.java
index ca53c76..48ed0ab 100644
--- a/azkaban-common/src/main/java/azkaban/utils/Utils.java
+++ b/azkaban-common/src/main/java/azkaban/utils/Utils.java
@@ -28,6 +28,7 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Collection;
+import java.util.Date;
 import java.util.Enumeration;
 import java.util.Random;
 import java.util.TimeZone;
@@ -489,7 +490,25 @@ public class Utils {
     } else return null;
   }
 
-  public static boolean isCronExpressionValid(String cronExpression) {
-    return CronExpression.isValidExpression(cronExpression);
+  /**
+   *
+   * @param cronExpression
+   * @param timezone
+   * @return if the cronExpression is valid or not.
+   */
+  public static boolean isCronExpressionValid(String cronExpression, DateTimeZone timezone) {
+    if (!CronExpression.isValidExpression(cronExpression)) {
+      return false;
+    }
+
+    /*
+     * The below code is aimed at checking some cases that the above code can not identify,
+     * e.g. <0 0 3 ? * * 22> OR <0 0 3 ? * 8>. Under these cases, the below code is able to tell.
+     */
+    CronExpression cronExecutionTime = parseCronExpression(cronExpression, timezone);
+    if (cronExecutionTime == null || cronExecutionTime.getNextValidTimeAfter(new Date()) == null) {
+      return false;
+    }
+    return true;
   }
 }

diff --git a/azkaban-common/src/test/java/azkaban/utils/UtilsTest.java b/azkaban-common/src/test/java/azkaban/utils/UtilsTest.java
index 4ea0930..526cc36 100644
--- a/azkaban-common/src/test/java/azkaban/utils/UtilsTest.java
+++ b/azkaban-common/src/test/java/azkaban/utils/UtilsTest.java
@@ -16,6 +16,7 @@
 
 package azkaban.utils;
 
+import org.joda.time.DateTimeZone;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -52,4 +53,29 @@ public class UtilsTest {
     Assert.assertTrue(Utils.isValidPort(3030));
     Assert.assertTrue(Utils.isValidPort(1045));
   }
+
+  /* Test CronExpression valid cases*/
+  @Test
+  public void testValidCronExpressionV() {
+
+    DateTimeZone timezone = DateTimeZone.getDefault();
+    Assert.assertTrue(Utils.isCronExpressionValid("0 0 3 ? * *", timezone));
+    Assert.assertTrue(Utils.isCronExpressionValid("0 0 3 ? * * 2017", timezone));
+    Assert.assertTrue(Utils.isCronExpressionValid("0 0 * ? * *", timezone));
+    Assert.assertTrue(Utils.isCronExpressionValid("0 0 * ? * FRI", timezone));
+
+    // This is a bug from Quartz Cron. It looks like Quartz will parse the preceding 7 fields of a String.
+    Assert.assertTrue(Utils.isCronExpressionValid("0 0 3 ? * * 2017 22", timezone));
+  }
+
+  /* Test CronExpression invalid cases*/
+  @Test
+  public void testInvalidCronExpression() {
+
+    DateTimeZone timezone = DateTimeZone.getDefault();
+    Assert.assertFalse(Utils.isCronExpressionValid("0 0 3 * * *", timezone));
+    Assert.assertFalse(Utils.isCronExpressionValid("0 66 * ? * *", timezone));
+    Assert.assertFalse(Utils.isCronExpressionValid("0 * * ? * 8", timezone));
+    Assert.assertFalse(Utils.isCronExpressionValid("0 * 25 ? * FRI", timezone));
+  }
 }

diff --git a/azkaban-web-server/src/main/java/azkaban/webapp/servlet/ScheduleServlet.java b/azkaban-web-server/src/main/java/azkaban/webapp/servlet/ScheduleServlet.java
index 14d5b89..32a4f4a 100644
--- a/azkaban-web-server/src/main/java/azkaban/webapp/servlet/ScheduleServlet.java
+++ b/azkaban-web-server/src/main/java/azkaban/webapp/servlet/ScheduleServlet.java
@@ -731,7 +731,7 @@ public class ScheduleServlet extends LoginAbstractAzkabanServlet {
         // everything in Azkaban functions is at the minute granularity, so we add 0 here
         // to let the expression to be complete.
         cronExpression = getParam(req, "cronExpression");
-        if(azkaban.utils.Utils.isCronExpressionValid(cronExpression) == false) {
+        if(azkaban.utils.Utils.isCronExpressionValid(cronExpression, timezone) == false) {
           ret.put("error", "This expression <" + cronExpression + "> can not be parsed to quartz cron.");
           return;
         }