Skip to content

Commit 2a1079f

Browse files
committed
Added Oversized Response Guard
Signed-off-by: Effi-S <effi.szt@gmail.com>
1 parent 5019e00 commit 2a1079f

2 files changed

Lines changed: 88 additions & 0 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ 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+
6970
go q.queryByID(ctx, keys, ch, startingBlock, evicted)
7071

7172
return ch, nil
@@ -117,6 +118,13 @@ func (q *DeliveryScanQueryByID) queryByID(ctx context.Context, keys []driver.PKe
117118

118119
continue
119120
}
121+
if len(values) != len(keys) {
122+
logger.Errorf("peer returned %d values for %d keys in ns [%s]; falling back to block scan",
123+
len(values), len(keys), ns)
124+
startDelivery = true
125+
126+
continue // treat as a per-namespace failure (=> fall back to the slow block scan)
127+
}
120128
found := make([]KeyInfo, 0, len(values))
121129
var notFound []string
122130
for i, value := range values {

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,86 @@ 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")
216+
}
217+
218+
// A namespace holding several keys where only some resolve must deliver the resolved ones and
219+
// fall back to the block scan for the rest, keeping the still-missing keys grouped under their
220+
// namespace (the notFound branch that rewrites keysByNS rather than deleting it).
221+
func TestQueryByID_PartialResolutionWithinNamespace(t *testing.T) {
222+
// One value present, one absent. Key<->value pairing follows the (map-random) request order,
223+
// so assert on the shape rather than a fixed key.
224+
querier := &fakeQuerier{results: map[driver.Namespace]querierResult{
225+
"ns1": {raw: values(t, []byte("v"), []byte{})},
226+
}}
227+
scanner := &fakeScanner{}
228+
229+
evicted := map[driver.PKey][]events.ListenerEntry[lookup.KeyInfo]{
230+
"k1": {&fakeEntry{ns: "ns1"}},
231+
"k2": {&fakeEntry{ns: "ns1"}},
232+
}
233+
234+
ch, err := newQuery(querier, scanner).QueryByID(t.Context(), 100, evicted)
235+
require.NoError(t, err)
236+
237+
got := drain(ch)
238+
require.Len(t, got, 1, "exactly the resolved key must be delivered")
239+
assert.Equal(t, driver.Namespace("ns1"), got[0].Namespace)
240+
assert.Contains(t, []driver.PKey{"k1", "k2"}, got[0].Key)
241+
assert.Equal(t, []byte("v"), got[0].Value)
242+
assert.True(t, scanner.called, "the unresolved key must fall back to the block scan")
243+
assert.Equal(t, uint64(90), scanner.startBlock)
244+
}
245+
246+
// An empty batch has nothing to query and nothing to scan: the channel simply closes empty.
247+
func TestQueryByID_EmptyEvicted(t *testing.T) {
248+
querier := &fakeQuerier{results: map[driver.Namespace]querierResult{}}
249+
scanner := &fakeScanner{}
250+
251+
ch, err := newQuery(querier, scanner).QueryByID(t.Context(), 100, nil)
252+
require.NoError(t, err)
253+
254+
assert.Empty(t, drain(ch))
255+
assert.Empty(t, querier.queried, "no namespace should be queried")
256+
assert.False(t, scanner.called, "an empty batch must not start a block scan")
257+
}
258+
259+
// A panic in the background goroutine (here: a listener entry with no namespace source) must be
260+
// recovered so it degrades to an empty, closed channel instead of crashing the node process.
261+
func TestQueryByID_PanicIsRecovered(t *testing.T) {
262+
querier := &fakeQuerier{results: map[driver.Namespace]querierResult{}}
263+
scanner := &fakeScanner{}
264+
265+
// An empty listener slice makes the namespace lookup index out of range and panic.
266+
evicted := map[driver.PKey][]events.ListenerEntry[lookup.KeyInfo]{
267+
"k1": {},
268+
}
269+
270+
ch, err := newQuery(querier, scanner).QueryByID(t.Context(), 100, evicted)
271+
require.NoError(t, err)
272+
273+
assert.Empty(t, drain(ch), "the channel must close cleanly after the recovered panic")
274+
assert.False(t, scanner.called)
275+
}
276+
197277
// The fallback block scan must never start from an underflowed block number: on a chain younger
198278
// than NumberPastBlocks it starts from FirstBlock instead of wrapping around to a block near
199279
// MaxUint64, which would silently find nothing and surface no error.

0 commit comments

Comments
 (0)