From 3fbf8cac2e83ea3ed7dc4f366d60111381c815eb Mon Sep 17 00:00:00 2001
From: Matthias Hanel <mh@synadia.com>
Date: Mon, 17 Feb 2020 12:57:21 -0500
Subject: [PATCH] parse ping_interval as duration, else assume seconds and warn

Signed-off-by: Matthias Hanel <mh@synadia.com>
---
 server/configs/test.conf |  2 +-
 server/opts.go           | 49 +++++++++++++++++++++-------------------
 server/opts_test.go      | 30 ++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/server/configs/test.conf b/server/configs/test.conf
index c5ce25e4..7a32658f 100644
--- a/server/configs/test.conf
+++ b/server/configs/test.conf
@@ -41,7 +41,7 @@ max_control_line: 2048
 max_payload: 65536
 
 # ping interval and no pong threshold
-ping_interval: 60
+ping_interval: "60s"
 ping_max: 3
 
 # how long server can block on a socket write to a client
diff --git a/server/opts.go b/server/opts.go
index e8803059..9ffe4cab 100644
--- a/server/opts.go
+++ b/server/opts.go
@@ -638,7 +638,7 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error
 	case "max_subscriptions", "max_subs":
 		o.MaxSubs = int(v.(int64))
 	case "ping_interval":
-		o.PingInterval = time.Duration(int(v.(int64))) * time.Second
+		o.PingInterval = parseDuration("ping_interval", tk, v, errors, warnings)
 	case "ping_max":
 		o.MaxPingsOut = int(v.(int64))
 	case "tls":
@@ -655,28 +655,7 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error
 		o.TLSTimeout = tc.Timeout
 		o.TLSMap = tc.Map
 	case "write_deadline":
-		wd, ok := v.(string)
-		if ok {
-			dur, err := time.ParseDuration(wd)
-			if err != nil {
-				err := &configErr{tk, fmt.Sprintf("error parsing write_deadline: %v", err)}
-				*errors = append(*errors, err)
-				return
-			}
-			o.WriteDeadline = dur
-		} else {
-			// Backward compatible with old type, assume this is the
-			// number of seconds.
-			o.WriteDeadline = time.Duration(v.(int64)) * time.Second
-			err := &configWarningErr{
-				field: k,
-				configErr: configErr{
-					token:  tk,
-					reason: "write_deadline should be converted to a duration",
-				},
-			}
-			*warnings = append(*warnings, err)
-		}
+		o.WriteDeadline = parseDuration("write_deadline", tk, v, errors, warnings)
 	case "lame_duck_duration":
 		dur, err := time.ParseDuration(v.(string))
 		if err != nil {
@@ -832,6 +811,30 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error
 	}
 }
 
+func parseDuration(field string, tk token, v interface{}, errors *[]error, warnings *[]error) time.Duration {
+	if wd, ok := v.(string); ok {
+		if dur, err := time.ParseDuration(wd); err != nil {
+			err := &configErr{tk, fmt.Sprintf("error parsing %s: %v", field, err)}
+			*errors = append(*errors, err)
+			return 0
+		} else {
+			return dur
+		}
+	} else {
+		// Backward compatible with old type, assume this is the
+		// number of seconds.
+		err := &configWarningErr{
+			field: field,
+			configErr: configErr{
+				token:  tk,
+				reason: field + " should be converted to a duration",
+			},
+		}
+		*warnings = append(*warnings, err)
+		return time.Duration(v.(int64)) * time.Second
+	}
+}
+
 func trackExplicitVal(opts *Options, pm *map[string]bool, name string, val bool) {
 	m := *pm
 	if m == nil {
diff --git a/server/opts_test.go b/server/opts_test.go
index 894bfb43..822d7fe6 100644
--- a/server/opts_test.go
+++ b/server/opts_test.go
@@ -1155,6 +1155,36 @@ func TestPanic(t *testing.T) {
 	}
 }
 
+func TestPingIntervalOld(t *testing.T) {
+	conf := createConfFile(t, []byte(`ping_interval: 5`))
+	defer os.Remove(conf)
+	opts := &Options{}
+	err := opts.ProcessConfigFile(conf)
+	if err == nil {
+		t.Fatalf("expected an error")
+	} else if err, ok := err.(*processConfigErr); !ok {
+		t.Fatalf("expected an error of type processConfigErr")
+	} else if len(err.warnings) != 1 {
+		t.Fatalf("expected processConfigErr to have one warning")
+	} else if len(err.errors) != 0 {
+		t.Fatalf("expected processConfigErr to have no error")
+	} else if opts.PingInterval != 5*time.Second {
+		t.Fatalf("expected ping interval to be 5 seconds")
+	}
+}
+
+func TestPingIntervalNew(t *testing.T) {
+	conf := createConfFile(t, []byte(`ping_interval: "5m"`))
+	defer os.Remove(conf)
+	opts := &Options{}
+	err := opts.ProcessConfigFile(conf)
+	if err != nil {
+		t.Fatalf("expected no error")
+	} else if opts.PingInterval != 5*time.Minute {
+		t.Fatalf("expected ping interval to be 5 minutes")
+	}
+}
+
 func TestOptionsProcessConfigFile(t *testing.T) {
 	// Create options with default values of Debug and Trace
 	// that are the opposite of what is in the config file.