[trafficcontrol] branch master updated: Fix TM nil pointer panic (#3330)

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mitchell852 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new 81f2f8b  Fix TM nil pointer panic (#3330)
81f2f8b is described below

commit 81f2f8beef6b00d068f4f2567d279d73d540616d
Author: Robert Butts <ro...@users.noreply.github.com>
AuthorDate: Fri Feb 15 10:20:34 2019 -0700

    Fix TM nil pointer panic (#3330)
---
 traffic_monitor/ds/stat.go      | 32 ++++++++++++++++++++++++++++++++
 traffic_monitor/ds/stat_test.go |  7 +++++++
 2 files changed, 39 insertions(+)

diff --git a/traffic_monitor/ds/stat.go b/traffic_monitor/ds/stat.go
index 237876a..124210f 100644
--- a/traffic_monitor/ds/stat.go
+++ b/traffic_monitor/ds/stat.go
@@ -20,6 +20,7 @@ package ds
  */
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -159,6 +160,10 @@ const BytesPerKilobit = 125
 // Note this mutates lastData, adding the new stat to it.
 // Also note that lastData may be mutated, even if an error occurs. Specifically, if the new stat is less than the last stat, it will still be set, so that the per-second stats will be properly computed on the next poll.
 func addLastStat(lastData *dsdata.LastStatData, newStat int64, newStatTime time.Time) error {
+	if lastData == nil {
+		return errors.New("nil lastData")
+	}
+
 	if lastData.Time == newStatTime {
 		return nil // TODO fix callers to not pass the same stat twice
 	}
@@ -199,6 +204,14 @@ func addLastStats(lastData *dsdata.LastStatsData, newStats *dsdata.StatCacheStat
 // addLastStatsToStatCacheStats adds the given LastStatsData to the given StatCacheStats.
 // Note s is mutated, with l being added to it.
 func addLastStatsToStatCacheStats(s *dsdata.StatCacheStats, l *dsdata.LastStatsData) {
+	if s == nil {
+		log.Errorln("ds.addLastStatsToStatCacheStats got nil StatCacheStats - skipping!")
+		return
+	}
+	if l == nil {
+		log.Errorln("ds.addLastStatsToStatCacheStats got nil LastStatsData - skipping!")
+		return
+	}
 	s.Kbps.Value = l.Bytes.PerSec / BytesPerKilobit
 	s.Tps2xx.Value = l.Status2xx.PerSec
 	s.Tps3xx.Value = l.Status3xx.PerSec
@@ -255,6 +268,11 @@ func addDSPerSecStats(lastStats *dsdata.LastStats, dsStats *dsdata.Stats, dsName
 		lastStats.DeliveryServices[dsName] = lastStat
 	}
 	for cacheName, cacheStats := range stat.Caches {
+		if cacheStats == nil {
+			log.Errorln("ds.addDSPerSecStats - stat.Caches[" + cacheName + "] exists, but unexpected nil! Setting new!")
+			stat.Caches[cacheName] = &dsdata.StatCacheStats{}
+		}
+
 		lastStatCache := lastStat.Caches[cacheName]
 		if lastStatCache == nil {
 			lastStatCache = &dsdata.LastStatsData{}
@@ -272,9 +290,23 @@ func addDSPerSecStats(lastStats *dsdata.LastStats, dsStats *dsdata.Stats, dsName
 	addLastDSStatTotals(lastStat, stat.CommonStats.CachesReporting, serverCachegroups, serverTypes)
 
 	for cacheGroup, cacheGroupStat := range lastStat.CacheGroups {
+		if cacheGroupStat == nil {
+			log.Errorln("ds.addDSPerSecStats - lastStats.DeliveryServices[" + string(dsName) + "].CacheGroups[" + string(cacheGroup) + "] exists, but unexpected nil! Setting new!")
+			lastStat.CacheGroups[cacheGroup] = &dsdata.LastStatsData{}
+		}
+		if stat.CacheGroups[cacheGroup] == nil {
+			stat.CacheGroups[cacheGroup] = &dsdata.StatCacheStats{}
+		}
 		addLastStatsToStatCacheStats(stat.CacheGroups[cacheGroup], cacheGroupStat)
 	}
 	for cacheType, cacheTypeStat := range lastStat.Type {
+		if cacheTypeStat == nil {
+			log.Errorln("ds.addDSPerSecStats - lastStat.DeliveryServices[" + string(dsName) + "].Type[" + string(cacheType) + "] exists, but unexpected nil! Setting new!")
+			lastStat.Type[cacheType] = &dsdata.LastStatsData{}
+		}
+		if stat.Types[cacheType] == nil {
+			stat.Types[cacheType] = &dsdata.StatCacheStats{}
+		}
 		addLastStatsToStatCacheStats(stat.Types[cacheType], cacheTypeStat)
 	}
 	addLastStatsToStatCacheStats(&stat.TotalStats, &lastStat.Total)
diff --git a/traffic_monitor/ds/stat_test.go b/traffic_monitor/ds/stat_test.go
index 269b91d..a7e05f9 100644
--- a/traffic_monitor/ds/stat_test.go
+++ b/traffic_monitor/ds/stat_test.go
@@ -398,3 +398,10 @@ func randStr() string {
 	}
 	return s
 }
+
+func TestAddLastStatsToStatCacheStatsNilVals(t *testing.T) {
+	// test that addLastStatsToStatCacheStats doesn't panic with nil values
+	addLastStatsToStatCacheStats(nil, nil)
+	addLastStatsToStatCacheStats(&dsdata.StatCacheStats{}, nil)
+	addLastStatsToStatCacheStats(nil, &dsdata.LastStatsData{})
+}