Prevent external mutation of data passed into widgets.

Fixes #174.

CHANGELOG.md

@@ -31,6 +31,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   not just received them.
 - Container focus tracker now correctly tracks focus changes in enlarged areas,
   i.e. when the terminal size increased.
+- The BarChart, LineChart and SegmentDisplay widgets now protect against
+  external mutation of the values passed into them by copying the data they
+  receive.
 
 #### Breaking API changes
 

widgets/barchart/barchart.go

@@ -264,14 +264,17 @@ func (bc *BarChart) Values(values []int, max int, opts ...Option) error {
 	bc.mu.Lock()
 	defer bc.mu.Unlock()
 
-	if err := validateValues(values, max); err != nil {
+	// Copy to avoid external modifications. See #174.
+	v := make([]int, len(values))
+	copy(v, values)
+	if err := validateValues(v, max); err != nil {
 		return err
 	}
 
 	for _, opt := range opts {
 		opt.set(bc.opts)
 	}
-	bc.values = values
+	bc.values = v
 	bc.max = max
 	return nil
 }

widgets/barchart/barchart_test.go

@@ -579,6 +579,61 @@ func TestBarChart(t *testing.T) {
 			},
 			wantCapacity: 3,
 		},
+		{
+			desc: "regression for #174, protects against external data mutation",
+			opts: []Option{
+				Char('o'),
+				Labels([]string{
+					"1",
+					"2",
+					"3",
+				}),
+			},
+			update: func(bc *BarChart) error {
+				values := []int{1, 2, 5, 10}
+				if err := bc.Values(values, 10); err != nil {
+					return err
+				}
+				values[0] = 100
+				return nil
+			},
+			canvas: image.Rect(0, 0, 7, 11),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustRectangle(c, image.Rect(0, 9, 1, 10),
+					draw.RectChar('o'),
+					draw.RectCellOpts(cell.BgColor(DefaultBarColor)),
+				)
+				testdraw.MustRectangle(c, image.Rect(2, 8, 3, 10),
+					draw.RectChar('o'),
+					draw.RectCellOpts(cell.BgColor(DefaultBarColor)),
+				)
+				testdraw.MustRectangle(c, image.Rect(4, 5, 5, 10),
+					draw.RectChar('o'),
+					draw.RectCellOpts(cell.BgColor(DefaultBarColor)),
+				)
+				testdraw.MustRectangle(c, image.Rect(6, 0, 7, 10),
+					draw.RectChar('o'),
+					draw.RectCellOpts(cell.BgColor(DefaultBarColor)),
+				)
+
+				// Labels.
+				testdraw.MustText(c, "1", image.Point{0, 10}, draw.TextCellOpts(
+					cell.FgColor(DefaultLabelColor),
+				))
+				testdraw.MustText(c, "2", image.Point{2, 10}, draw.TextCellOpts(
+					cell.FgColor(DefaultLabelColor),
+				))
+				testdraw.MustText(c, "3", image.Point{4, 10}, draw.TextCellOpts(
+					cell.FgColor(DefaultLabelColor),
+				))
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+			wantCapacity: 4,
+		},
 	}
 
 	for _, tc := range tests {

widgets/barchart/options.go

@@ -143,7 +143,9 @@ func LabelColors(colors []cell.Color) Option {
 // label.
 func Labels(labels []string) Option {
 	return option(func(opts *options) {
-		opts.labels = labels
+		// Copy to avoid external modifications. See #174.
+		opts.labels = make([]string, len(labels))
+		copy(opts.labels, labels)
 	})
 }
 

widgets/linechart/internal/axes/scale.go

@@ -68,6 +68,11 @@ type YScale struct {
 	brailleHeight int
 }
 
+// String implements fmt.Stringer.
+func (ys *YScale) String() string {
+	return fmt.Sprintf("YScale{Min:%v, Max:%v, Step:%v, GraphHeight:%v}", ys.Min, ys.Max, ys.Step, ys.GraphHeight)
+}
+
 // NewYScale calculates the scale of the Y axis, given the boundary values and
 // the height of the graph. The nonZeroDecimals dictates rounding of the
 // calculated scale, see NewValue for details.

widgets/linechart/linechart.go

@@ -52,9 +52,13 @@ type seriesValues struct {
 
 // newSeriesValues returns a new seriesValues instance.
 func newSeriesValues(values []float64) *seriesValues {
-	min, max := numbers.MinMax(values)
+	// Copy to avoid external modifications. See #174.
+	v := make([]float64, len(values))
+	copy(v, values)
+
+	min, max := numbers.MinMax(v)
 	return &seriesValues{
-		values: values,
+		values: v,
 		min:    min,
 		max:    max,
 	}
@@ -146,7 +150,12 @@ func SeriesCellOpts(co ...cell.Option) SeriesOption {
 func SeriesXLabels(labels map[int]string) SeriesOption {
 	return seriesOption(func(opts *seriesValues) {
 		opts.xLabelsSet = true
-		opts.xLabels = labels
+
+		// Copy to avoid external modifications. See #174.
+		opts.xLabels = make(map[int]string, len(labels))
+		for pos, label := range labels {
+			opts.xLabels[pos] = label
+		}
 	})
 }
 
@@ -409,21 +418,21 @@ func (lc *LineChart) drawSeries(cvs *canvas.Canvas, xd *axes.XDetails, yd *axes.
 
 			startX, err := xdZoomed.Scale.ValueToPixel(i - 1)
 			if err != nil {
-				return nil, fmt.Errorf("failure for series %v[%d], xdZoomed.Scale.ValueToPixel => %v", name, i-1, err)
+				return nil, fmt.Errorf("failure for series %v[%d] on scale %v, xdZoomed.Scale.ValueToPixel(%v) => %v", name, i-1, xdZoomed.Scale, i-1, err)
 			}
 			endX, err := xdZoomed.Scale.ValueToPixel(i)
 			if err != nil {
-				return nil, fmt.Errorf("failure for series %v[%d], xdZoomed.Scale.ValueToPixel => %v", name, i, err)
+				return nil, fmt.Errorf("failure for series %v[%d] on scale %v, xdZoomed.Scale.ValueToPixel(%v) => %v", name, i, xdZoomed.Scale, i, err)
 			}
 
 			startY, err := yd.Scale.ValueToPixel(prev)
 			if err != nil {
-				return nil, fmt.Errorf("failure for series %v[%d], yd.Scale.ValueToPixel => %v", name, i-1, err)
+				return nil, fmt.Errorf("failure for series %v[%d] on scale %v, yd.Scale.ValueToPixel(%v) => %v", name, i-1, yd.Scale, prev, err)
 			}
 			v := sv.values[i]
 			endY, err := yd.Scale.ValueToPixel(v)
 			if err != nil {
-				return nil, fmt.Errorf("failure for series %v[%d], yd.Scale.ValueToPixel => %v", name, i, err)
+				return nil, fmt.Errorf("failure for series %v[%d] on scale %v, yd.Scale.ValueToPixel(%v) => %v", name, i, yd.Scale, v, err)
 			}
 
 			if err := draw.BrailleLine(bc,

widgets/linechart/linechart_test.go

@@ -1479,6 +1479,53 @@ func TestLineChartDraws(t *testing.T) {
 				return ft
 			},
 		},
+		{
+			desc:   "regression for #174, protects against external data mutation",
+			canvas: image.Rect(0, 0, 20, 10),
+			writes: func(lc *LineChart) error {
+				values := []float64{0, 100}
+				labels := map[int]string{
+					0: "start",
+					1: "end",
+				}
+				if err := lc.Series("first", values, SeriesXLabels(labels)); err != nil {
+					return err
+				}
+
+				// Modify the values after they were passed in.
+				// Increase above the previous maximum to run out of the Y axis.
+				// The linechart should not use the original values.
+				values[1] = 1000
+				labels[0] = "bad"
+				return nil
+			},
+			wantCapacity: 28,
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				// Y and X axis.
+				lines := []draw.HVLine{
+					{Start: image.Point{5, 0}, End: image.Point{5, 8}},
+					{Start: image.Point{5, 8}, End: image.Point{19, 8}},
+				}
+				testdraw.MustHVLines(c, lines)
+
+				// Value labels.
+				testdraw.MustText(c, "0", image.Point{4, 7})
+				testdraw.MustText(c, "51.68", image.Point{0, 3})
+				testdraw.MustText(c, "start", image.Point{6, 9})
+
+				// Braille line.
+				graphAr := image.Rect(6, 0, 20, 8)
+				bc := testbraille.MustNew(graphAr)
+				testdraw.MustBrailleLine(bc, image.Point{0, 31}, image.Point{26, 0})
+				testbraille.MustCopyTo(bc, c)
+
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+		},
 	}
 
 	for _, tc := range tests {

widgets/segmentdisplay/segmentdisplay_test.go

@@ -731,6 +731,40 @@ func TestSegmentDisplay(t *testing.T) {
 					mustDrawChar(cvs, tc.char, tc.area)
 				}
 
+				testcanvas.MustApply(cvs, ft)
+				return ft
+			},
+		},
+		{
+			desc: "regression for #174, protects against external data mutation",
+			opts: []Option{
+				GapPercent(0),
+			},
+			canvas: image.Rect(0, 0, sixteen.MinCols*3, sixteen.MinRows),
+			update: func(sd *SegmentDisplay) error {
+				chunks := []*TextChunk{NewChunk("123")}
+				if err := sd.Write(chunks); err != nil {
+					return err
+				}
+				// Mutates and adds unsupported characters.
+				chunks[0] = NewChunk("\r\t")
+				return nil
+			},
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				cvs := testcanvas.MustNew(ft.Area())
+
+				for _, tc := range []struct {
+					char rune
+					area image.Rectangle
+				}{
+					{'1', image.Rect(0, 0, sixteen.MinCols, sixteen.MinRows)},
+					{'2', image.Rect(sixteen.MinCols, 0, sixteen.MinCols*2, sixteen.MinRows)},
+					{'3', image.Rect(sixteen.MinCols*2, 0, sixteen.MinCols*3, sixteen.MinRows)},
+				} {
+					mustDrawChar(cvs, tc.char, tc.area)
+				}
+
 				testcanvas.MustApply(cvs, ft)
 				return ft
 			},

widgets/sparkline/sparkline_test.go

@@ -426,6 +426,30 @@ func TestSparkLine(t *testing.T) {
 			},
 			wantCapacity: 2,
 		},
+		{
+			desc: "regression for #174, protects against external data mutation",
+			update: func(sl *SparkLine) error {
+				values := []int{0, 1, 2, 3, 4, 5, 6, 7, 8}
+				if err := sl.Add(values); err != nil {
+					return err
+				}
+				// Mutation should have no effect.
+				values[0] = 8
+				return nil
+			},
+			canvas: image.Rect(0, 0, 9, 1),
+			want: func(size image.Point) *faketerm.Terminal {
+				ft := faketerm.MustNew(size)
+				c := testcanvas.MustNew(ft.Area())
+
+				testdraw.MustText(c, "▁▂▃▄▅▆▇█", image.Point{1, 0}, draw.TextCellOpts(
+					cell.FgColor(DefaultColor),
+				))
+				testcanvas.MustApply(c, ft)
+				return ft
+			},
+			wantCapacity: 9,
+		},
 	}
 
 	for _, tc := range tests {