Skip to content

Commit 9127cfc

Browse files
committed
Added Oversized Response Guard and goroutine defer wrap
Signed-off-by: Effi-S <effi.szt@gmail.com>
1 parent 591b1b9 commit 9127cfc

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

token/services/network/fabric/lookup/deliveryqs.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,18 @@ func (q *DeliveryScanQueryByID) QueryByID(ctx context.Context, startingBlock dri
6666
// Keys are supposed to be unique
6767
keys := collections.Keys(evicted) // These are the state keys we are looking for
6868
ch := make(chan []KeyInfo, len(keys))
69-
go q.queryByID(ctx, keys, ch, startingBlock, evicted)
69+
70+
go func() {
71+
// Defense in depth: any unforeseen panic in this background path must degrade to a
72+
// failed request rather than crashing the whole node process, since nothing else in
73+
// this goroutine's call chain has a recover().
74+
defer func() {
75+
if r := recover(); r != nil {
76+
logger.Errorf("recovered from panic in queryByID: %v", r)
77+
}
78+
}()
79+
q.queryByID(ctx, keys, ch, startingBlock, evicted)
80+
}()
7081

7182
return ch, nil
7283
}
@@ -117,6 +128,13 @@ func (q *DeliveryScanQueryByID) queryByID(ctx context.Context, keys []driver.PKe
117128

118129
continue
119130
}
131+
if len(values) != len(keys) {
132+
logger.Errorf("peer returned %d values for %d keys in ns [%s]; falling back to block scan",
133+
len(values), len(keys), ns)
134+
startDelivery = true
135+
136+
continue // treat as a per-namespace failure (=> fall back to the slow block scan)
137+
}
120138
found := make([]KeyInfo, 0, len(values))
121139
var notFound []string
122140
for i, value := range values {

token/services/network/fabric/lookup/deliveryqs_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,25 @@ func TestQueryByID_MissingValueTriggersScan(t *testing.T) {
194194
assert.True(t, scanner.called)
195195
}
196196

197+
// A peer that returns more values than keys requested
198+
// must not index the keys slice out of range and crash the process.
199+
// (From Issue #2055.)
200+
func TestQueryByID_OversizedResponse(t *testing.T) {
201+
// 2 values returned, but only 1 key requested.
202+
querier := &fakeQuerier{results: map[driver.Namespace]querierResult{
203+
"ns1": {raw: values(t, []byte("v1"), []byte("v2-unexpected"))},
204+
}}
205+
scanner := &fakeScanner{}
206+
207+
ch, err := newQuery(querier, scanner).QueryByID(t.Context(), 100, evictedFor(map[driver.Namespace]driver.PKey{
208+
"ns1": "k1",
209+
}))
210+
require.NoError(t, err)
211+
212+
// Nothing is delivered for the oversized namespace.
213+
// It should fall back to the block scan rather than crashing.
214+
assert.Empty(t, drain(ch))
215+
assert.True(t, scanner.called, "the oversized response must fall back to the block scan")
197216
// The fallback block scan must never start from an underflowed block number: on a chain younger
198217
// than NumberPastBlocks it starts from FirstBlock instead of wrapping around to a block near
199218
// MaxUint64, which would silently find nothing and surface no error.

0 commit comments

Comments
 (0)