Skip to content

Restrict forwards in MockMvcWebConnection to 100 #29557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

manthanb
Copy link
Contributor

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. This change will restrict the maximum number of forwards in MockMvcWebConnection to 100.

Closes gh-29483

Verified

This commit was signed with the committer’s verified signature.
aalmiray Andres Almiray
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. This
change will restrict the maximum number of forwards in
MockMvcWebConnection to 100.

Closes spring-projectsgh-29483
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 23, 2022
@manthanb
Copy link
Contributor Author

manthanb commented Jan 6, 2023

Hey @sbrannen,
I am not sure if you are the right person to reach out to, and I am very sorry if you are not.
I was wondering whenever you have some time, if you can take a quick look at this request and see if it fixes the issue of potential infinite loop with MockMvcWebConnection - #29483.

@simonbasle simonbasle self-assigned this Jan 20, 2023
Copy link
Contributor

@simonbasle simonbasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manthanb thanks for the contribution. I think that throwing an exception as proposed in the original issue would be better, wdyt? The message can mention that there is a probable infinite loop and that it forwarded 100 times.

@simonbasle simonbasle added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 20, 2023
@simonbasle
Copy link
Contributor

@manthanb are you still able to work on this? (cf review above)

@manthanb
Copy link
Contributor Author

Hi @simonbasle Very sorry I missed your last comment. Yes, I will make the changes and commit today.

@simonbasle
Copy link
Contributor

(note: to be backported in 5.3.x, as tracked by gh-29866)

Copy link
Contributor

@simonbasle simonbasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and made the adjustments to throw rather than ignore

@simonbasle simonbasle merged commit 89c170b into spring-projects:main Feb 7, 2023
@simonbasle simonbasle modified the milestones: 6.0.6, 6.0.5 Feb 7, 2023
simonbasle added a commit that referenced this pull request Feb 7, 2023
This change restricts the maximum number of forwards in MockMvcWebConnection to 100,
in case a forward is configured in a way that causes a loop. This is necessary in HtmlUnit
backed tests, unlike in classic MockMvc tests in which the forwards are not actually resolved.

See gh-29557
Closes gh-29866

Co-authored-by: Simon Baslé <sbasle@vmware.com>
@simonbasle simonbasle added the status: backported An issue that has been backported to maintenance branches label Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible infinite forward loop with MockMvcWebConnection
3 participants