fix(toxics/bandwidth): don't hang or panic when rate <= 0

toxics/bandwidth.go

@@ -38,8 +38,11 @@ func (t *BandwidthToxic) Pipe(stub *ToxicStub) {
 			} else {
 				sleep += time.Duration(len(p.Data)) * time.Millisecond / time.Duration(t.Rate)
 			}
-			// If the rate is low enough, split the packet up and send in 100 millisecond intervals
-			for int64(len(p.Data)) > t.Rate*100 {
+			// If the rate is low enough, split the packet up and send in 100 millisecond intervals.
+			// Guard on t.Rate > 0: a non-positive rate disables throttling (see above), and
+			// t.Rate*100 would otherwise be a zero split size (infinite loop emitting empty
+			// chunks) or a negative slice bound (panic).
+			for t.Rate > 0 && int64(len(p.Data)) > t.Rate*100 {
 				select {
 				case <-time.After(100 * time.Millisecond):
 					stub.Output <- &stream.StreamChunk{

toxics/bandwidth_test.go

@@ -2,12 +2,14 @@ package toxics_test
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"net"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/Shopify/toxiproxy/v2/stream"
 	"github.com/Shopify/toxiproxy/v2/testhelper"
 	"github.com/Shopify/toxiproxy/v2/toxics"
 )
@@ -93,3 +95,54 @@ func BenchmarkBandwidthToxic100MB(b *testing.B) {
 		b.Error("Failed to close TCP connection", err)
 	}
 }
+
+// TestBandwidthToxicNonPositiveRate verifies that a bandwidth toxic configured
+// with a non-positive rate (rate <= 0) forwards data unthrottled instead of
+// hanging or panicking. The Pipe already special-cases `t.Rate <= 0` to disable
+// throttling (sleep = 0), so the intent is that such a toxic is a no-op.
+//
+// Regression test: with rate == 0 the packet-splitting loop
+// (`for len(p.Data) > t.Rate*100`) spins forever emitting empty chunks
+// (p.Data[:0]) and never forwards the payload; with rate < 0 the same loop
+// slices p.Data[:negative] and panics with "slice bounds out of range".
+func TestBandwidthToxicNonPositiveRate(t *testing.T) {
+	for _, rate := range []int64{0, -1} {
+		rate := rate
+		t.Run(fmt.Sprintf("rate_%d", rate), func(t *testing.T) {
+			toxic := &toxics.BandwidthToxic{Rate: rate}
+
+			input := make(chan *stream.StreamChunk)
+			output := make(chan *stream.StreamChunk, 100)
+			stub := toxics.NewToxicStub(input, output)
+
+			payload := []byte("hello world")
+
+			panicked := make(chan interface{}, 1)
+			go func() {
+				defer func() { panicked <- recover() }()
+				toxic.Pipe(stub)
+			}()
+
+			go func() { input <- &stream.StreamChunk{Data: payload} }()
+
+			select {
+			case chunk := <-output:
+				if !bytes.Equal(chunk.Data, payload) {
+					t.Errorf("rate=%d: expected full payload forwarded unthrottled, "+
+						"got %d bytes (%q)", rate, len(chunk.Data), chunk.Data)
+				}
+			case p := <-panicked:
+				t.Fatalf("rate=%d: BandwidthToxic.Pipe panicked: %v", rate, p)
+			case <-time.After(2 * time.Second):
+				t.Fatalf("rate=%d: timed out; BandwidthToxic.Pipe did not forward "+
+					"the chunk (infinite split loop)", rate)
+			}
+
+			// Unblock and stop the Pipe goroutine if it is still running.
+			select {
+			case stub.Interrupt <- struct{}{}:
+			case <-time.After(time.Second):
+			}
+		})
+	}
+}