[JENKINS-72653] fix timezone handling (#301)

* [JENKINS-72653] improve time zone handling

* [JENKINS-72653] fix timezone handling

the implementation to prefill the scheduled time with the global value
was not working properly, when the timezone of the controller was
different from the timezone that was configured. The implementation used
java.util.Date which has only limited capabilites to work with
timeszones.
This change replaces all date/time handling with new classes from the
java.time packages.
The timezone is no longer a free text field but a dropdown that gets
filled with all available zone ids.
The datetime field to schedule a build is no longer locale dependent but
uses a fixed format based on 24h format (see [JENKINS-72645]).
It is optional to enter seconds.
The actual timezone that will be used is shown as field description.
The time input on global config allows to specify am/pm but also 24h
formats, seconds are optional.

* fix security alerts

* update pics and readme

* fix typo

* allow system_read in doCHeck and doFill

* apply spotless

* fix checkurl

* fix merge error

* fix help url

* allow AM/PM

* Format with spotless

---------

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>

README.md

@@ -42,7 +42,7 @@ Add to your yaml file:
 ```yaml
 unclassified:
   scheduleBuild:
-    defaultScheduleTime: "11:00:00 PM"
+    defaultStartTime: "11:00:00 PM"
     timeZone: "Europe/Paris"
 ```
 

src/main/java/org/jenkinsci/plugins/schedulebuild/ScheduleBuildAction.java

@@ -6,22 +6,22 @@
 import hudson.model.Job;
 import hudson.model.ParametersDefinitionProperty;
 import hudson.util.FormValidation;
-import java.io.IOException;
-import java.text.DateFormat;
-import java.text.ParseException;
-import java.util.Date;
+import java.time.LocalDateTime;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
+import java.time.temporal.ChronoUnit;
 import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import javax.servlet.ServletException;
-import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
 import org.jenkins.ui.icon.IconSpec;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.QueryParameter;
-import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerProxy;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.interceptor.RequirePOST;
@@ -31,8 +31,13 @@ public class ScheduleBuildAction implements Action, StaplerProxy, IconSpec {
     private static final Logger LOGGER = Logger.getLogger(ScheduleBuildAction.class.getName());
 
     private final Job<?, ?> target;
-    private static final long SECURITY_MARGIN = 120 * 1000;
-    private static final long ONE_DAY = 24 * 3600 * 1000;
+    private static final long SECURITY_MARGIN = 120;
+
+    private static final String DATE_TIME_PATTERN = "dd-MM-yyyy HH:mm:ss";
+
+    private static final DateTimeFormatter[] FORMATTERS = {
+        DateTimeFormatter.ofPattern("d-M-y H:m[:s]"), DateTimeFormatter.ofPattern("d-M-y h:m[:s] a", Locale.ROOT),
+    };
 
     private long quietperiod;
 
@@ -78,41 +83,17 @@ public Object getTarget() {
         return this;
     }
 
-    public String getIconPath() {
-        Jenkins instance = Jenkins.getInstanceOrNull();
-        if (instance != null) {
-            String rootUrl = instance.getRootUrl();
-
-            if (rootUrl != null) {
-                return rootUrl + "plugin/schedule-build/";
-            }
-        }
-        throw new IllegalStateException("couldn't load rootUrl");
-    }
-
     public String getDefaultDate() {
-        return dateFormat().format(getDefaultDateObject());
+        return getDefaultDateObject().format(DateTimeFormatter.ofPattern(DATE_TIME_PATTERN));
     }
 
-    public Date getDefaultDateObject() {
-        Date buildtime = new Date(),
-                now = new Date(),
-                defaultScheduleTime = new ScheduleBuildGlobalConfiguration().getDefaultScheduleTimeObject();
-        DateFormat dateFormat = dateFormat();
-        try {
-            now = dateFormat.parse(dateFormat.format(now));
-        } catch (ParseException e) {
-            LOGGER.log(Level.WARNING, "Error while parsing date", e);
-        }
-        buildtime.setHours(defaultScheduleTime.getHours());
-        buildtime.setMinutes(defaultScheduleTime.getMinutes());
-        buildtime.setSeconds(defaultScheduleTime.getSeconds());
-
-        if (now.getTime() > buildtime.getTime()) {
-            buildtime.setTime(buildtime.getTime() + ScheduleBuildAction.ONE_DAY);
+    public ZonedDateTime getDefaultDateObject() {
+        ZonedDateTime zdt = new ScheduleBuildGlobalConfiguration().getDefaultScheduleTimeObject();
+        ZonedDateTime now = ZonedDateTime.now();
+        if (now.isAfter(zdt)) {
+            zdt = zdt.plusDays(1);
         }
-
-        return buildtime;
+        return zdt;
     }
 
     @RequirePOST
@@ -122,73 +103,78 @@ public FormValidation doCheckDate(@QueryParameter String value, @AncestorInPath
         }
         // User requesting a build needs permission to start the build
         item.checkPermission(Item.BUILD);
-        Date ddate, now = new Date();
-        DateFormat dateFormat = dateFormat();
+        ZonedDateTime now = ZonedDateTime.now();
+        ZonedDateTime ddate;
         try {
-            ddate = dateFormat.parse(value);
-            now = dateFormat.parse(dateFormat.format(now));
-        } catch (ParseException ex) {
+            ddate = parseDateTime(value.trim())
+                    .atZone(new ScheduleBuildGlobalConfiguration().getZoneId())
+                    .plusSeconds(SECURITY_MARGIN);
+        } catch (DateTimeParseException ex) {
             return FormValidation.error(Messages.ScheduleBuildAction_ParsingError());
         }
-
-        if (now.getTime() > ddate.getTime() + ScheduleBuildAction.SECURITY_MARGIN) {
+        if (now.isAfter(ddate)) {
             return FormValidation.error(Messages.ScheduleBuildAction_DateInPastError());
         }
 
         return FormValidation.ok();
     }
 
     public long getQuietPeriodInSeconds() {
-        return quietperiod / 1000;
+        return quietperiod;
     }
 
     @RequirePOST
-    public HttpResponse doNext(StaplerRequest req, @AncestorInPath Item item)
-            throws FormException, ServletException, IOException {
+    public HttpResponse doNext(@QueryParameter String date, @AncestorInPath Item item) {
         if (item == null) {
             return FormValidation.ok();
         }
         // User requesting a build needs permission to start the build
         item.checkPermission(Item.BUILD);
-        // Deprecated function StructureForm.get()
-        // JSONObject param = StructuredForm.get(req);
-        JSONObject param = req.getSubmittedForm();
-        Date ddate = getDefaultDateObject(), now = new Date();
-        DateFormat dateFormat = dateFormat();
-        try {
-            now = dateFormat.parse(dateFormat.format(now));
-        } catch (ParseException e) {
-            LOGGER.log(Level.WARNING, "Error while parsing date", e);
-        }
+        ZonedDateTime ddate, now = ZonedDateTime.now();
 
-        if (param.containsKey("date")) {
-            try {
-                ddate = dateFormat().parse(param.getString("date"));
-            } catch (ParseException ex) {
-                return HttpResponses.redirectTo("error");
-            }
+        final String time = date.trim();
+        try {
+            ddate = parseDateTime(time).atZone(new ScheduleBuildGlobalConfiguration().getZoneId());
+        } catch (DateTimeParseException ex) {
+            LOGGER.log(Level.INFO, ex, () -> "Error parsing " + time);
+            return HttpResponses.redirectTo("error");
         }
-
-        quietperiod = ddate.getTime() - now.getTime();
+        quietperiod = ChronoUnit.SECONDS.between(now, ddate);
+        LOGGER.log(Level.FINER, () -> "Quietperiod: " + quietperiod);
         if (quietperiod + ScheduleBuildAction.SECURITY_MARGIN < 0) { // 120 sec security margin
+            LOGGER.log(Level.INFO, () -> "Error security margin" + quietperiod);
             return HttpResponses.redirectTo("error");
         }
         return HttpResponses.forwardToView(this, "redirect");
     }
 
+    private LocalDateTime parseDateTime(String time) {
+        DateTimeParseException exception = null;
+        for (DateTimeFormatter formatter : FORMATTERS) {
+            try {
+                return LocalDateTime.parse(time.toUpperCase(Locale.ROOT), formatter);
+            } catch (DateTimeParseException dtex) {
+                exception = dtex;
+                LOGGER.log(Level.FINE, dtex, () -> "Did not parse '" + time + "' with formatter " + formatter);
+            }
+        }
+        throw exception;
+    }
+
     public boolean isJobParameterized() {
         ParametersDefinitionProperty paramDefinitions = target.getProperty(ParametersDefinitionProperty.class);
         return paramDefinitions != null
                 && paramDefinitions.getParameterDefinitions() != null
                 && paramDefinitions.getParameterDefinitions().size() > 0;
     }
 
-    private DateFormat dateFormat() {
-        Locale locale = Stapler.getCurrentRequest() != null
-                ? Stapler.getCurrentRequest().getLocale()
-                : Locale.getDefault();
-        DateFormat df = DateFormat.getDateTimeInstance(DateFormat.SHORT, DateFormat.MEDIUM, locale);
-        df.setTimeZone(new ScheduleBuildGlobalConfiguration().getTimeZoneObject());
-        return df;
+    @Restricted(NoExternalUse.class)
+    public String getDateTimeFormatting() {
+        return DATE_TIME_PATTERN;
+    }
+
+    @Restricted(NoExternalUse.class)
+    public String getTimeZone() {
+        return new ScheduleBuildGlobalConfiguration().getTimeZone();
     }
 }