Skip to content
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

SEC-830: SavedRequestAwareWrapper and ParameterMethodNameResolver #1091

Closed
spring-projects-issues opened this issue May 13, 2008 · 5 comments
Closed
Milestone

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented May 13, 2008

from SEC-830) said:

This is a very simple use case, Its strange how this was ignored. I will try to create a sample war file and get it. in the mean while..

Spring : 2.5.1 (code in 2.0)
Acegi : 1.0.4

lest say we have Acegi security and a multi action controller method resolved on “action” parameter.

lets say, we try to access
/mod/order.html?action=view
and acegi saves the request into SavedRequestAwareWrapper and forwards to /login.html
After login, it goes to OrderController.view() method and forwards to view.jsp

similarly
/mod/order.html?action=viewDetail (This has more logic and data building)
to OrderController.viewDetail() to viewdetail.jsp

This is fine so far

but I have a condition in view.jsp (say for certain cust)
if (cust priv > ordinary)

now, if i add this jsp include, and access this page with a customer satisfying the condition, a new request to
/mod/order.html?action=view
keeps looping after login and there is a stack over flow.

Reason:
After Login, the request /mod/order.html?action=view goes to call the
OrderController.view(), once it dispatches to jsp, the jsp include is kicked off, and the method resolver tries to get the parameter action from the request. However, the SavedRequestAwareWrapper.getParameter returns it from the savedRequest which is “/mod/order.html?action=view” .. it keeps looping there.

I am eager to find out how you would solve this.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 15, 2008

Luke Taylor said:

Do you actually need to use SavedRequestAwareWrapper (or indeed SecurityContextHolderAwareRequestFilter)?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 15, 2008

Luke Taylor said:

I think we can get round this issue by consulting the wrapped request first for the parameter.

The problem occurs because, during a request dispatcher call, the parameters from the query string are merged with those of the original request. The query string ones should take precedence over the original request ones. SavedRequestWrapper calls the saved request if it exists. It should probably always allow parameters in the wrapped request to take precedence (as these may be modified by the container during a forward or include).

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 15, 2008

Sarath said:

Right, but only when the request is an include. The current logic of using the savedRequest for getParameters is right for other cases.

```
if (savedRequest == null) {
values = super.getParameterValues(name);
} else {
values = savedRequest.getParameterValues(name);
}
```

could be something like

```
if (savedRequest == null || WebUtils.isRequestInclude(this)) {
values = super.getParameterValues(name);
} else {
values = savedRequest.getParameterValues(name);
}
```

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 15, 2008

Luke Taylor said:

I don’t think it matters if the request is an include or not. The SavedRequest will only be used if it exactly matches the incoming request. Hence it should already contain the same query string as the original request. It is useful because it supplements the information that can be sent this way (e.g. with POST parameters). So checking the original request should not produce different values (unless they have been supplemented or modified by a RequestDispatcher call, which is then desirable).

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 25, 2008

Luke Taylor said:

I’ve changed SavedRequestAwareWrapper so that the wrapped request parameters take precedence over the saved request ones for calls to getParameter. This appears to fix the problem reported here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant