Router in runtime breaks Context for any following middlewares

[mjuraga]

Describe the bug

I have some logic where I need to log the request after the authentication and authorization, and in my auth functions I set something like:

ctx := context.WithValue(r.Context(), types.AuthErrCtxKey, retErr)
*r = *r.WithContext(ctx)

And after the request is executed, my context is gone due to the following line:

runtime/middleware/router.go

Line 67 in f4f99b4

next.ServeHTTP(rw, rCtx)

		if _, rCtx, ok := ctx.RouteInfo(r); ok {
			next.ServeHTTP(rw, rCtx)
			return
		}

This creates a new rCtx and passes it up to the downstream handlers, but never sets it to the r variable, which means everything done to this request object, most importantly Context, never gets to the upstream handlers after this return.

Steps to Reproduce

If you add a middleware before your router, and one after. Add some Context value in the one after, and try to read it in the one before, it will be gone.

Expected behavior

I would expect anything done with request.Context survives for the lifetime of the request.

Actual behavior

From this point in the request lifetime anything added or changed will not be reflected in the chain before.

Environment
Not-applicable

Additional context

I would be happy to provide an upstream fix for this.

[fredbi]

Interesting. I’ll try to catch the bug over the week end.

Feel free to drop by any repro code here, as it would really help the resolution

[fredbi]

@mjuraga I can't reproduce your issue.

Regarding your comment
`

This creates a new rCtx and passes it up to the downstream handlers, but never sets it to the r variable, which means everything done to this request object, most importantly Context, never gets to the upstream handlers after this return.

I don't really get your point. RouteInfo() builds a context from the request context to add some matched route context info.
The returned *http.Request inherits the context from the original request.

Further, I've tried to write a small repro test based on your observations and indeed, I can verify that the request context is maintained. Perhaps there is something I didn't quite get from your explanation.

func TestRouterContext_Issue375(t *testing.T) {
        spec, api := petstore.NewAPI(t)
        spec.Spec().BasePath = "/api/"
        context := NewContext(spec, api, nil)

        type authCtxKey uint8
        const authCtx authCtxKey = iota + 1
        authCtxErr := stderrors.New("test error in context")

        mw := NewRouter(context, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                // check context after API middleware
                value := r.Context().Value(authCtx)
                assert.NotNilf(t, value, "end of middleware chain: expected to find authCtx in request context")
                if value == nil {
                        w.WriteHeader(http.StatusInternalServerError)
                }

                errAuth, ok := value.(error)
                assert.Truef(t, ok, "expected authCtx to be an error, but got: %T", value)
                if !ok {
                        w.WriteHeader(http.StatusInternalServerError)
                }

                t.Logf("got context from request: %v", errAuth)
                fmt.Fprintf(w, "%v", errAuth)
                w.WriteHeader(http.StatusOK)
        }))

        start := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                authContext := stdcontext.WithValue(r.Context(), authCtx, authCtxErr)

                t.Logf("calling API router with context: %v", authCtxErr)
                mw.ServeHTTP(w, r.WithContext(authContext))
                // *r = *r.WithContext(authContext)
                // mw.ServeHTTP(w, r)
        })

        recorder := httptest.NewRecorder()
        request, err := http.NewRequestWithContext(stdcontext.Background(), http.MethodGet, "/api/pets/123", nil)
        require.NoError(t, err)

        start.ServeHTTP(recorder, request)
        assert.Equal(t, http.StatusOK, recorder.Code)

        res := recorder.Result()
        require.NotNil(t, res.Body)
        msg, err := io.ReadAll(res.Body)
        require.NoError(t, err)
        t.Logf("response message: %q", string(msg))
}

go test -v -run 375
=== RUN   TestRouterContext_Issue375
    router_test.go:290: calling API router with context: test error in context
    router_test.go:282: got context from request: test error in context
    router_test.go:307: response message: "test error in context"
--- PASS: TestRouterContext_Issue375 (0.02s)
PASS
ok  	github.com/go-openapi/runtime/middleware	0.021s