Skip to content

Commit db67dd8

Browse files
nvxbugclaude
andcommitted
extproc: allow negative server window increments per spec
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LK7JqPusvaySyrJFUPbsH
1 parent 7b7f666 commit db67dd8

2 files changed

Lines changed: 31 additions & 17 deletions

File tree

internal/xds/httpfilter/extproc/ext_proc.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,25 +2040,26 @@ func (cs *clientStream) waitForTrailerProcessing(recvErr error) error {
20402040

20412041
// applyServerWindowUpdate adds a flow control window increment received from
20422042
// the external processor to window, waking a blocked acquirer if the window
2043-
// becomes positive. The increment is server-controlled, so it must be
2044-
// non-negative and must not overflow the int64 window; an increment that
2045-
// violates either is a protocol error and returns one instead of wrapping the
2046-
// accounting, matching how HTTP/2 flow control rejects a window update that
2047-
// exceeds the maximum. This recv loop is the only writer that grows the
2048-
// window, so the compare-and-swap only contends with a concurrent deduction in
2043+
// becomes positive. The increment may be negative: the ext_proc spec allows a
2044+
// server to shrink the window, in which case senders block until later
2045+
// updates bring it back above zero. The increment is server-controlled
2046+
// though, so one that would wrap the int64 window in either direction is a
2047+
// protocol error and returns one instead of corrupting the accounting. This
2048+
// recv loop is the only writer that grows the window, so the compare-and-swap
2049+
// only contends with a concurrent deduction in
20492050
// acquire{Downstream,Upstream}ToSidestreamWindow.
20502051
func applyServerWindowUpdate(window *atomic.Int64, positiveUpdateCh chan struct{}, delta int64) error {
20512052
if delta == 0 {
20522053
return nil
20532054
}
2054-
if delta < 0 {
2055-
return fmt.Errorf("negative window increment %d", delta)
2056-
}
20572055
for {
20582056
previousQuota := window.Load()
2059-
if previousQuota > math.MaxInt64-delta {
2057+
if delta > 0 && previousQuota > math.MaxInt64-delta {
20602058
return fmt.Errorf("window increment %d overflows the current window %d", delta, previousQuota)
20612059
}
2060+
if delta < 0 && previousQuota < math.MinInt64-delta {
2061+
return fmt.Errorf("window increment %d underflows the current window %d", delta, previousQuota)
2062+
}
20622063
newQuota := previousQuota + delta
20632064
if !window.CompareAndSwap(previousQuota, newQuota) {
20642065
continue

internal/xds/httpfilter/extproc/ext_proc_test.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ import (
2727
)
2828

2929
// TestApplyServerWindowUpdate verifies that a flow control window increment
30-
// from the external processor is applied only when it is a non-negative value
31-
// that does not overflow the window. An out-of-range increment must be
32-
// rejected without mutating the window, so the accounting cannot wrap to a
33-
// value that permanently blocks acquireDownstreamToSidestreamWindow.
30+
// from the external processor is applied whenever it does not wrap the int64
31+
// window, including negative increments, which the ext_proc spec allows. An
32+
// increment that would overflow or underflow must be rejected without
33+
// mutating the window, so the accounting cannot wrap to a value that
34+
// permanently blocks acquireDownstreamToSidestreamWindow.
3435
func TestApplyServerWindowUpdate(t *testing.T) {
3536
tests := []struct {
3637
name string
@@ -67,11 +68,23 @@ func TestApplyServerWindowUpdate(t *testing.T) {
6768
wantWindow: iextproc.DefaultFlowControlWindowSize,
6869
},
6970
{
70-
name: "negative increment is rejected and leaves the window unchanged",
71+
name: "negative increment shrinks the window",
7172
start: iextproc.DefaultFlowControlWindowSize,
72-
delta: -1,
73+
delta: -1024,
74+
wantWindow: iextproc.DefaultFlowControlWindowSize - 1024,
75+
},
76+
{
77+
name: "negative increment can drive the window below zero",
78+
start: 1024,
79+
delta: -2048,
80+
wantWindow: -1024,
81+
},
82+
{
83+
name: "underflowing increment is rejected and leaves the window unchanged",
84+
start: math.MinInt64 + 10,
85+
delta: -1024,
7386
wantErr: true,
74-
wantWindow: iextproc.DefaultFlowControlWindowSize,
87+
wantWindow: math.MinInt64 + 10,
7588
},
7689
}
7790
for _, test := range tests {

0 commit comments

Comments
 (0)