mcp: prevent MemoryEventStore.After panic on index past the end (#1132)
## Summary
`MemoryEventStore.After` guarded only the lower bound of the requested
index:
```go
start := index + 1
if dl.first > start { // too old → ErrEventsPurged
return nil, fmt.Errorf(... ErrEventsPurged)
}
return slices.Clone(dl.data[start-dl.first:]), nil
```
There was no *upper*-bound guard, so an `index` greater than the highest
stored event made the final slice expression panic with `slice bounds
out of range`. The boundary `start-dl.first == len(dl.data)` (resume
from the last event seen) is a valid empty replay, so only
strictly-greater indices faulted.
This is reachable from untrusted input: `parseEventID` accepts any
non-negative index from the client-supplied `Last-Event-ID` header, and
`serveGET` passes it straight to `eventStore.After`. Direct callers of
the exported `EventStore.After` API crash outright.
Fixes #1131
## Change
Return an empty replay for an index at or beyond the latest stored event
— there is nothing after it — consistent with the already-valid `index
== last` case. `ErrEventsPurged` would be misleading here, since those
events were never sent rather than purged.
## Testing
Extended `TestMemoryEventStoreAfter` with indices past the end of a
stream (`3`, `100`, and one past a second stream), asserting an empty
replay with no error.
Verified the new cases **panic on the pre-fix code** (`panic: runtime
error: slice bounds out of range [3:2]`) and **pass with the fix**. `go
vet ./mcp/` and the full `go test ./mcp/` suite pass.
---------
Co-authored-by: Yaroslav Shevchuk <yarolegovich@gmail.com>
This commit is contained in:
parent
fe27fa9151
commit
ce0b198202
2 changed files with 10 additions and 3 deletions
|
|
@ -337,12 +337,15 @@ func (s *MemoryEventStore) After(_ context.Context, sessionID, streamID string,
|
|||
if !ok {
|
||||
return nil, fmt.Errorf("MemoryEventStore.After: unknown stream ID %v in session %q", streamID, sessionID)
|
||||
}
|
||||
start := index + 1
|
||||
if dl.first > start {
|
||||
start := (index + 1) - dl.first
|
||||
if start < 0 {
|
||||
return nil, fmt.Errorf("MemoryEventStore.After: index %d, stream ID %v, session %q: %w",
|
||||
index, streamID, sessionID, ErrEventsPurged)
|
||||
}
|
||||
return slices.Clone(dl.data[start-dl.first:]), nil
|
||||
if start >= len(dl.data) {
|
||||
return nil, nil
|
||||
}
|
||||
return slices.Clone(dl.data[start:]), nil
|
||||
}
|
||||
|
||||
return func(yield func([]byte, error) bool) {
|
||||
|
|
|
|||
|
|
@ -299,7 +299,11 @@ func TestMemoryEventStoreAfter(t *testing.T) {
|
|||
{"S1", "1", 0, []string{"d2", "d3"}, ""},
|
||||
{"S1", "1", 1, []string{"d3"}, ""},
|
||||
{"S1", "1", 2, nil, ""},
|
||||
{"S1", "1", 3, nil, ""},
|
||||
{"S1", "1", 100, nil, ""}, // past the end of stream "1" (highest is 2)
|
||||
{"S1", "2", 0, nil, ""},
|
||||
{"S1", "2", 1, nil, ""},
|
||||
{"S1", "2", 100, nil, ""},
|
||||
{"S1", "3", 0, nil, "unknown stream ID"},
|
||||
{"S2", "0", 0, nil, "unknown session ID"},
|
||||
} {
|
||||
|
|
|
|||
Loading…
Reference in a new issue