bug: nil pointer dereference in flow/retriever/router when Router config is not provided
- Dominant language
- Go
- Stars
- 13k
- Forks
- 1.1k
- Avg merge
- 4h 6m
- Merged PRs (30d)
- 41
Description
## Bug Description
In `flow/retriever/router/router.go`, the `NewRetriever` function builds a default router function when `config.Router` is `nil`, but then accidentally stores `config.Router` (which is still `nil`) into the `routerRetriever` struct instead of the local `router` variable. This causes a **nil pointer dereference panic** when `Retrieve()` is called without a custom `Router` configured.
## Affected File
`flow/retriever/router/router.go`
## Root Cause
```go
func NewRetriever(ctx context.Context, config *Config) (retriever.Retriever, error) {
// ...
router := config.Router
if router == nil {
// correctly build a default router
var retrieverSet []string
for k := range config.Retrievers {
retrieverSet = append(retrieverSet, k)
}
router = func(ctx context.Context, query string) ([]string, error) {
return retrieverSet, nil
}
}
// ...
return &routerRetriever{
retrievers: config.Retrievers,
router: config.Router, // BUG: should be `router`, not `config.Router`
fusionFunc: fusion,
}, nil
}
```
When `config.Router` is `nil` (the common case where the user relies on the default all-retrievers router), the struct field `routerRetriever.router` is also `nil`. The `Retrieve()` method then calls `e.router(routeCtx, query)`, which panics with a nil pointer dereference.
## Steps to Reproduce
```go
retrieverA := /* some retriever.Retriever */
retrieverB := /* some retriever.Retriever */
// config.Router is intentionally omitted — expecting the default "all retrievers" behavior
rr, err := router.NewRetriever(ctx, &router.Config{
Retrievers: map[string]retriever.Retriever{
"a": retrieverA,
"b": retrieverB,
},
// Router not set, FusionFunc not set
})
if err != nil {
log.Fatal(err)
}
// PANICS: runtime error: invalid memory address or nil pointer dereference
docs, err := rr.Retrieve(ctx, "some query")
```
## Expected Behavior
`NewRetriever` should store the resolved (possibly default) `router` variable into the struct, not `config.Router`. When `config.Router` is `nil`, all registered retrievers should be queried (the intended default behavior).
## Fix
Change line in `NewRetriever`:
```go
// Before (buggy)
return &routerRetriever{
retrievers: config.Retrievers,
router: config.Router,
fusionFunc: fusion,
}, nil
// After (fixed)
return &routerRetriever{
retrievers: config.Retrievers,
router: router,
fusionFunc: fusion,
}, nil
```
## Impact
Any caller that omits `Config.Router` (relying on the documented default behavior of querying all retrievers) will encounter a nil pointer dereference panic at runtime instead of the intended behavior.
Contributor guide
Research direction
Read flow/retriever/router/router.go, starting with NewRetriever and then Retrieve. Reproduce the issue with Config.Router omitted and registered retrievers, then verify the default configuration no longer panics and queries the configured retrievers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100