Skip to content

Commit 10eea0e

Browse files
committed
Grails would sometimes attempt to render the wrong view, particularly
during error dispatch. This was because certain information about the view to render was stored in the request, and that was affecting whatever was forwarded to. So while Grails would try to render the view associated with an error action, suddenly it would end up rendering the original one. This fix clears certain request attributes before that request is forwarded. This was already being done for includes, so I refactored to share the code with the forwarding methods.
1 parent d0d4856 commit 10eea0e

File tree

3 files changed

+77
-31
lines changed

3 files changed

+77
-31
lines changed

src/java/org/codehaus/groovy/grails/web/servlet/GrailsDispatcherServlet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ public Locale getLocale() {
301301

302302
mv = super.processHandlerException(processedRequest, response, mappedHandler, e);
303303
handlerException = e;
304-
render(mv, processedRequest, response);
304+
if (mv != null) render(mv, processedRequest, response);
305305
}
306306
else {
307307
request.removeAttribute(GrailsApplicationAttributes.RENDERING_ERROR_ATTRIBUTE);

src/java/org/codehaus/groovy/grails/web/servlet/mvc/SimpleGrailsControllerHelper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,13 @@ public Object handleAction(GroovyObject controller,Closure action, HttpServletRe
388388
public ModelAndView handleActionResponse( GroovyObject controller,Object returnValue,String closurePropertyName, String viewName) {
389389
boolean viewNameBlank = (viewName == null || viewName.length() == 0);
390390
// reset the metaclass
391-
ModelAndView explicityModelAndView = (ModelAndView)controller.getProperty(ControllerDynamicMethods.MODEL_AND_VIEW_PROPERTY);
391+
ModelAndView explicitModelAndView = (ModelAndView)controller.getProperty(ControllerDynamicMethods.MODEL_AND_VIEW_PROPERTY);
392392

393393
if(!webRequest.isRenderView()) {
394394
return null;
395395
}
396-
else if(explicityModelAndView != null) {
397-
return explicityModelAndView;
396+
else if(explicitModelAndView != null) {
397+
return explicitModelAndView;
398398
}
399399
else if (returnValue == null) {
400400
if (viewNameBlank) {

src/java/org/codehaus/groovy/grails/web/util/WebUtils.java

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,12 @@ public static String forwardRequestForUrlMappingInfo(HttpServletRequest request,
297297
//populateParamsForMapping(info);
298298
RequestDispatcher dispatcher = request.getRequestDispatcher(forwardUrl);
299299

300+
// Clear the request attributes that affect view rendering. Otherwise
301+
// whatever we forward to may render the wrong thing! Note that we
302+
// don't care about the return value because we're delegating
303+
// responsibility for rendering the response.
304+
saveAndResetWebRequest(request, info);
305+
300306
exposeForwardRequestAttributes(request);
301307
exposeRequestAttributes(request, model);
302308
dispatcher.forward(request, response);
@@ -318,42 +324,82 @@ public static IncludedContent includeForUrlMappingInfo(HttpServletRequest reques
318324
String includeUrl = buildDispatchUrlForMapping(info, true);
319325

320326
final GrailsWebRequest webRequest = GrailsWebRequest.lookup(request);
321-
322-
String currentController = null;
323-
String currentAction = null;
324-
String currentId = null;
325-
ModelAndView currentMv = null;
326-
Map currentParams = null;
327-
if (webRequest!=null) {
328-
currentController = webRequest.getControllerName();
329-
currentAction = webRequest.getActionName();
330-
currentId = webRequest.getId();
331-
currentParams = new HashMap();
332-
currentParams.putAll(webRequest.getParameterMap());
333-
currentMv = (ModelAndView)webRequest.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
334-
}
327+
InternalSavedRequest savedRequest = null;
328+
335329
try {
336-
if (webRequest!=null) {
337-
webRequest.getParameterMap().clear();
338-
info.configure(webRequest);
339-
webRequest.getParameterMap().putAll(info.getParameters());
340-
webRequest.removeAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
341-
}
330+
savedRequest = saveAndResetWebRequest(request, info);
342331
return includeForUrl(includeUrl, request, response, model);
343332
}
344333
finally {
345-
if (webRequest!=null) {
334+
if (savedRequest != null) {
346335
webRequest.getParameterMap().clear();
347-
webRequest.getParameterMap().putAll(currentParams);
348-
webRequest.setId(currentId);
349-
webRequest.setControllerName(currentController);
350-
webRequest.setActionName(currentAction);
351-
if(currentMv != null) {
352-
webRequest.setAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, currentMv, 0);
336+
webRequest.getParameterMap().putAll(savedRequest.getParameterMap());
337+
webRequest.setId(savedRequest.getId());
338+
webRequest.setControllerName(savedRequest.getController());
339+
webRequest.setActionName(savedRequest.getAction());
340+
if (savedRequest.getModelAndView() != null) {
341+
webRequest.setAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, savedRequest.getModelAndView(), 0);
353342
}
354343
}
355344
}
356345
}
346+
347+
/**
348+
* Saves the details of the current web request in an {@link InternalSavedRequest}
349+
* instance, clears information related to view rendering from the request
350+
* attributes, and returns the saved request.
351+
* @param request The underlying HTTP request to process.
352+
* @param info The URL mapping that should be applied to the request after
353+
* the attributes have been cleared.
354+
* @return The saved web request details.
355+
*/
356+
public static InternalSavedRequest saveAndResetWebRequest(HttpServletRequest request, UrlMappingInfo info) {
357+
final GrailsWebRequest webRequest = GrailsWebRequest.lookup(request);
358+
InternalSavedRequest savedRequest = null;
359+
if (webRequest != null) {
360+
savedRequest = new InternalSavedRequest(
361+
webRequest.getControllerName(),
362+
webRequest.getActionName(),
363+
webRequest.getId(),
364+
webRequest.getParameterMap(),
365+
(ModelAndView) webRequest.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0));
366+
webRequest.getParameterMap().clear();
367+
info.configure(webRequest);
368+
webRequest.getParameterMap().putAll(info.getParameters());
369+
webRequest.removeAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
370+
}
371+
372+
return savedRequest;
373+
}
374+
375+
/**
376+
* Simple class that stores a subset of information about a request, including
377+
* the target controller and action, and the request parameters. Also stores
378+
* information about any current ModelAndView that will be used to render the
379+
* response.
380+
*/
381+
private static class InternalSavedRequest {
382+
private String controller;
383+
private String action;
384+
private String id;
385+
private HashMap parameters;
386+
private ModelAndView modelAndView;
387+
388+
public InternalSavedRequest(String controllerName, String actionName, String id, Map parameterMap, ModelAndView mv) {
389+
this.controller = controllerName;
390+
this.action = actionName;
391+
this.id = id;
392+
this.parameters = new HashMap(parameterMap);
393+
this.modelAndView = mv;
394+
}
395+
396+
public String getController() { return controller; }
397+
public String getAction() { return action; }
398+
public String getId() { return id; }
399+
public Map getParameterMap() { return parameters; }
400+
public ModelAndView getModelAndView() { return modelAndView; }
401+
}
402+
357403

358404
/**
359405
* Includes the given URL returning the resulting content as a String

0 commit comments

Comments
 (0)