When a filter is configured to conditionally forward, and it is configured to handle FORWARD dispatches as well, and it prevents infinite forward loops by either extending OncePerRequestFilter or otherwise using request attributes, this can result in infinite forward loops in WebClient tests using MockMvcWebConnection. Because in this case request attributes are not propagated from the original request to the forwards.

Example RequireSettingFilter:

@Component
public class RequireSettingFilter extends OncePerRequestFilter {

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        if (System.getProperty("mysetting") == null) {
            request.getRequestDispatcher("/error/missingsetting").forward(request, response);
            return;
        }
        filterChain.doFilter(request, response);
    }
}

RedirectTest:

@WebMvcTest
public class RedirectTest {
    @Autowired
    WebClient webClient;

    @Test
    void testForwardToError() throws FailingHttpStatusCodeException, MalformedURLException, IOException {
        HtmlPage page = webClient.getPage("/demo.html");
        assertEquals("A demo", page.asNormalizedText());
    }
}

Suggested fix: In org.springframework.test.web.servlet.htmlunit.MockMvcWebConnection.getResponse(WebRequest) forwards are handled. Limit this to e.g. 100 forwards and afterwards throw an exception that the page is not forwarding properly. Infinite loops are very bad, because they can make the build system hang.

Note that just copying request attributes from the original request to the new one wouldn't help for the case of OncePerRequestFilter, because it clears the attributes when exiting doFilterInternal(...).

Affects: 5.3.23

Comment From: simonbasle

Once the forward happens, if the filter also apply to the forward destination it is applied on a distinct request. I'm not entirely sure if this is specific to MockMvc's MockRequestDispatchHandler or if it is true of more RequestDispatchers 🤔

Unlike other MockMVC parts, MockMvcWebConnection is really trying to get to some HTML for the benefit of HtmlUnit and as such does follow these forwards. So it can end up in an infinite loop if the forwarding filter applies to both original request and forwarded request. I agree that being defensive there is an enhancement.

Perhaps there is more area of improvement (MockRequestDispatcherHandler not setting the DispatcherType to FORWARD for instance) , but this is a good first step.

Comment From: svschouw-bb

Setting the dispatch type to FORWARD might help, but I think you would need to handle all the forward attributes then as well. If you would really like to do it correctly you'd need a RequestDispatcher that actually handles the forward instead of setting a forwardedUrl parameter. But I think this is where the MockMvc case really differs from the MockMvcWebConnection. With MockMvc you want to do a single request and see that it did a forward. With MockMvcWebConnection you want to see the entire result of the request.

But in my opinion this ticket is mostly just about avoiding the infinite loop. If you really want to do some advanced integration testing, it's probably better to use Spring Boot's @SpringBootTest(webEnvironment=RANDOM_PORT) or something.

Comment From: simonbasle

Superseded by gh-29557