From ca3e2f38f6e558e4b8120671d6d2336993d09e38 Mon Sep 17 00:00:00 2001 From: ydylla <17772145+ydylla@users.noreply.github.com> Date: Fri, 28 Jun 2024 18:36:18 +0200 Subject: [PATCH] fix: endless matching loop (#199) 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 https://github.com/mholt/caddy-l4/pull/194 --- layer4/routes.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/layer4/routes.go b/layer4/routes.go index f4bd312..cfa4003 100644 --- a/layer4/routes.go +++ b/layer4/routes.go @@ -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) { @@ -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 }) }