Skip to content

Commit ab90e9d

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

2 files changed

Lines changed: 40 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
@@ -63,7 +63,18 @@ func (q *DeliveryScanQueryByID) QueryByID(ctx context.Context, startingBlock dri
6363
// Keys are supposed to be unique
6464
keys := collections.Keys(evicted) // These are the state keys we are looking for
6565
ch := make(chan []KeyInfo, len(keys))
66-
go q.queryByID(ctx, keys, ch, startingBlock, evicted)
66+
67+
go func() {
68+
// Defense in depth: any unforeseen panic in this background path must degrade to a
69+
// failed request rather than crashing the whole node process, since nothing else in
70+
// this goroutine's call chain has a recover().
71+
defer func() {
72+
if r := recover(); r != nil {
73+
logger.Errorf("recovered from panic in queryByID: %v", r)
74+
}
75+
}()
76+
q.queryByID(ctx, keys, ch, startingBlock, evicted)
77+
}()
6778

6879
return ch, nil
6980
}
@@ -114,6 +125,13 @@ func (q *DeliveryScanQueryByID) queryByID(ctx context.Context, keys []driver.PKe
114125

115126
continue
116127
}
128+
if len(values) != len(keys) {
129+
logger.Errorf("peer returned %d values for %d keys in ns [%s]; falling back to block scan",
130+
len(values), len(keys), ns)
131+
startDelivery = true
132+
133+
continue // treat as a per-namespace failure (=> fall back to the slow block scan)
134+
}
117135
found := make([]KeyInfo, 0, len(values))
118136
var notFound []string
119137
for i, value := range values {

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,24 @@ func TestQueryByID_MissingValueTriggersScan(t *testing.T) {
193193
assert.Empty(t, drain(ch))
194194
assert.True(t, scanner.called)
195195
}
196+
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")
216+
}

0 commit comments

Comments
 (0)