Skip to content

Commit

Permalink
fix: endless matching loop (#199)
Browse files Browse the repository at this point in the history
This happens for example with the proxy_protocol matcher and handler. If we receive a proxy proto header from a not allowed source the matcher matches, but the handler does not consume any of the prefetched bytes. Since the handler is not terminal we jump back to the beginning of the matching loop and match the same matcher/handler combo again. It never stops because we never call cx.prefetch again. Since `isFirstPrefetch = false` is unreachable in this situation (the statement is at the wrong place). It always must be set false after the first iteration.
This bug was caused by the fix for #194
  • Loading branch information
ydylla authored Jun 28, 2024
1 parent ce9789f commit ca3e2f3
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions layer4/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,14 @@ func (routes RouteList) Compile(logger *zap.Logger, matchingTimeout time.Duratio
if err != nil {
return err
}
isFirstPrefetch := true
for { // retry prefetching and matching routes until timeout

for i := 0; i < 10000; i++ { // retry prefetching and matching routes until timeout

// Do not call prefetch if this is the first loop iteration and there already is some data available,
// since this means we are at the start of a subroute handler and previous prefetch calls likely already fetched all bytes available from the client.
// Which means it would block the subroute handler. In the second iteration (if no subroute routes match) blocking is the correct behaviour.
if !isFirstPrefetch || cx.buf == nil || len(cx.buf[cx.offset:]) == 0 {
if i != 0 || cx.buf == nil || len(cx.buf[cx.offset:]) == 0 {
err = cx.prefetch()
isFirstPrefetch = false
if err != nil {
logFunc := logger.Error
if errors.Is(err, os.ErrDeadlineExceeded) {
Expand Down Expand Up @@ -184,5 +183,8 @@ func (routes RouteList) Compile(logger *zap.Logger, matchingTimeout time.Duratio
}
}
}

logger.Error("matching connection", zap.String("remote", cx.RemoteAddr().String()), zap.Error(errors.New("number of prefetch calls exhausted")))
return nil
})
}

0 comments on commit ca3e2f3

Please sign in to comment.