From 4fc2d7f14b66ec2bca5f7e3cc164582c1eead5d3 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Mon, 18 Nov 2013 06:15:55 +0200 Subject: [PATCH] GRAILS-9056 Unify content type setting in RenderDynamicMethod, fix rendering file --- .../web/metaclass/RenderDynamicMethod.java | 359 +++++++++--------- .../web/servlet/RenderMethodTests.groovy | 2 +- 2 files changed, 188 insertions(+), 173 deletions(-) diff --git a/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/web/metaclass/RenderDynamicMethod.java b/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/web/metaclass/RenderDynamicMethod.java index 205e0dcc326..958f8f74d9d 100644 --- a/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/web/metaclass/RenderDynamicMethod.java +++ b/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/web/metaclass/RenderDynamicMethod.java @@ -25,6 +25,7 @@ import groovy.lang.MissingMethodException; import groovy.lang.Writable; import groovy.text.Template; +import groovy.util.slurpersupport.GPathResult; import groovy.xml.StreamingMarkupBuilder; import java.io.ByteArrayInputStream; @@ -96,6 +97,7 @@ public class RenderDynamicMethod extends AbstractDynamicMethodInvocation { private static final String BUILDER_TYPE_JSON = "json"; private static final String TEXT_HTML = "text/html"; + private static final String APPLICATION_XML = "application/xml"; public static final String DISPOSITION_HEADER_PREFIX = "attachment;filename="; private String gspEncoding = DEFAULT_ENCODING; private static final String DEFAULT_ENCODING = "utf-8"; @@ -124,207 +126,211 @@ public Object invoke(Object target, String methodName, Object[] arguments) { boolean renderView = true; GroovyObject controller = (GroovyObject) target; - if (arguments[0] instanceof CharSequence) { - setContentType(response, TEXT_HTML, DEFAULT_ENCODING,true); - CharSequence text = (CharSequence)arguments[0]; + final Object renderArgument = arguments[0]; + if (renderArgument instanceof Writable) { + applyContentType(response, null, renderArgument); + Writable writable = (Writable)renderArgument; + renderView = renderWritable(writable, response); + } else if (renderArgument instanceof CharSequence) { + applyContentType(response, null, renderArgument); + CharSequence text = (CharSequence)renderArgument; renderView = renderText(text, response); } - else if (arguments[0] instanceof Closure) { - setContentType(response, TEXT_HTML, gspEncoding, true); - Closure closure = (Closure) arguments[arguments.length - 1]; - renderView = renderMarkup(closure, response); - } - else if (arguments[0] instanceof Map) { - Map argMap = (Map) arguments[0]; - boolean hasContentType = argMap.containsKey(ARGUMENT_CONTENT_TYPE); - - Writer out = null; - if(hasContentType) { - out = getWriterForConfiguredContentType(response,argMap, hasContentType); - webRequest.setOut(out); - } - if (argMap.containsKey(ARGUMENT_LAYOUT)) { - webRequest.getCurrentRequest().setAttribute(GrailsLayoutDecoratorMapper.LAYOUT_ATTRIBUTE, argMap.get(ARGUMENT_LAYOUT)); + else { + final Object renderObject = arguments[arguments.length - 1]; + if (renderArgument instanceof Closure) { + setContentType(response, TEXT_HTML, DEFAULT_ENCODING, true); + Closure closure = (Closure) renderObject; + renderView = renderMarkup(closure, response); } + else if (renderArgument instanceof Map) { + Map argMap = (Map) renderArgument; + + if (argMap.containsKey(ARGUMENT_LAYOUT)) { + webRequest.getCurrentRequest().setAttribute(GrailsLayoutDecoratorMapper.LAYOUT_ATTRIBUTE, argMap.get(ARGUMENT_LAYOUT)); + } - boolean statusSet = false; - if (argMap.containsKey(ARGUMENT_STATUS)) { - Object statusObj = argMap.get(ARGUMENT_STATUS); - if (statusObj != null) { - try { - response.setStatus(Integer.parseInt(statusObj.toString())); - statusSet = true; - } - catch (NumberFormatException e) { - throw new ControllerExecutionException( - "Argument [status] of method [render] must be a valid integer."); + boolean statusSet = false; + if (argMap.containsKey(ARGUMENT_STATUS)) { + Object statusObj = argMap.get(ARGUMENT_STATUS); + if (statusObj != null) { + try { + final int statusCode = statusObj instanceof Number ? ((Number)statusObj).intValue() : Integer.parseInt(statusObj.toString()); + response.setStatus(statusCode); + statusSet = true; + } + catch (NumberFormatException e) { + throw new ControllerExecutionException( + "Argument [status] of method [render] must be a valid integer."); + } } } - } - - if (arguments[arguments.length - 1] instanceof Closure) { - Closure callable = (Closure) arguments[arguments.length - 1]; - if (BUILDER_TYPE_JSON.equals(argMap.get(ARGUMENT_BUILDER)) || isJSONResponse(response)) { - renderView = renderJSON(callable, response); + + if (renderObject instanceof Writable) { + Writable writable = (Writable) renderObject; + applyContentType(response, argMap, renderObject); + renderView = renderWritable(writable, response); + } + else if (renderObject instanceof Closure) { + Closure callable = (Closure) renderObject; + applyContentType(response, argMap, renderObject); + if (BUILDER_TYPE_JSON.equals(argMap.get(ARGUMENT_BUILDER)) || isJSONResponse(response)) { + renderView = renderJSON(callable, response); + } + else { + renderView = renderMarkup(callable, response); + } } - else { - renderView = renderMarkup(callable, response); + else if (renderObject instanceof CharSequence) { + applyContentType(response, argMap, renderObject); + CharSequence text = (CharSequence) renderObject; + renderView = renderText(text, response); } - } - else if (arguments[arguments.length - 1] instanceof CharSequence) { - if(out == null) { - out = getWriterForConfiguredContentType(response, argMap, hasContentType); - webRequest.setOut(out); + else if (argMap.containsKey(ARGUMENT_TEXT)) { + Object textArg = argMap.get(ARGUMENT_TEXT); + applyContentType(response, argMap, textArg); + if (textArg instanceof Writable) { + Writable writable = (Writable) textArg; + renderView = renderWritable(writable, response); + } else { + CharSequence text = (textArg instanceof CharSequence) ? ((CharSequence)textArg) : textArg.toString(); + renderView = renderText(text, response); + } } - - CharSequence text = (CharSequence) arguments[arguments.length - 1]; - renderView = renderText(text, out); - } - else if (argMap.containsKey(ARGUMENT_TEXT)) { - if(out == null) { - out = getWriterForConfiguredContentType(response, argMap, hasContentType); - webRequest.setOut(out); + else if (argMap.containsKey(ARGUMENT_VIEW)) { + renderView(webRequest, argMap, target, controller); } - - Object textArg = argMap.get(ARGUMENT_TEXT); - CharSequence text = (textArg instanceof CharSequence) ? ((CharSequence)textArg) : textArg.toString(); - renderView = renderText(text, out); - } - else if (argMap.containsKey(ARGUMENT_VIEW)) { - renderView(webRequest, argMap, target, controller, hasContentType); - } - else if (argMap.containsKey(ARGUMENT_TEMPLATE)) { - if(out == null) { - out = getWriterForConfiguredContentType(response, argMap, hasContentType); - webRequest.setOut(out); + else if (argMap.containsKey(ARGUMENT_TEMPLATE)) { + applyContentType(response, argMap, null, false); + renderView = renderTemplate(target, controller, webRequest, argMap); } - - renderView = renderTemplate(target, controller, webRequest, argMap, out); - } - else if (argMap.containsKey(ARGUMENT_FILE)) { - renderView = false; - - Object o = argMap.get(ARGUMENT_FILE); - Object fnO = argMap.get(ARGUMENT_FILE_NAME); - String fileName = fnO != null ? fnO.toString() : ((o instanceof File) ? ((File)o).getName(): null ); - if (o != null) { - if (fileName != null) { - detectContentTypeFromFileName(webRequest, response, argMap, fileName, hasContentType); - if (fnO != null) { - response.setHeader(HttpHeaders.CONTENT_DISPOSITION, DISPOSITION_HEADER_PREFIX + fileName); - } - } - else if (!hasContentType) { - throw new ControllerExecutionException( - "Argument [file] of render method specified without valid [contentType] argument"); - } - - InputStream input = null; - try { - if (o instanceof File) { - File f = (File) o; - input = FileUtils.openInputStream(f); - } - else if (o instanceof InputStream) { - input = (InputStream) o; - } - else if (o instanceof byte[]) { - input = new ByteArrayInputStream((byte[])o); + else if (argMap.containsKey(ARGUMENT_FILE)) { + renderView = false; + + Object o = argMap.get(ARGUMENT_FILE); + Object fnO = argMap.get(ARGUMENT_FILE_NAME); + String fileName = fnO != null ? fnO.toString() : ((o instanceof File) ? ((File)o).getName(): null ); + if (o != null) { + boolean hasContentType = applyContentType(response, argMap, null, false); + if (fileName != null) { + if(!hasContentType) { + hasContentType = detectContentTypeFromFileName(webRequest, response, argMap, fileName); + } + if (fnO != null) { + response.setHeader(HttpHeaders.CONTENT_DISPOSITION, DISPOSITION_HEADER_PREFIX + fileName); + } } - else { - input = FileUtils.openInputStream(new File(o.toString())); + if (!hasContentType) { + throw new ControllerExecutionException( + "Argument [file] of render method specified without valid [contentType] argument"); } - IOUtils.copy(input, response.getOutputStream()); - } catch (IOException e) { - throw new ControllerExecutionException( - "I/O error copying file to response: " + e.getMessage(), e); - } - finally { - if (input != null) { - try { - input.close(); - } catch (IOException e) { - // ignore + InputStream input = null; + try { + if (o instanceof File) { + File f = (File) o; + input = FileUtils.openInputStream(f); + } + else if (o instanceof InputStream) { + input = (InputStream) o; + } + else if (o instanceof byte[]) { + input = new ByteArrayInputStream((byte[])o); + } + else { + input = FileUtils.openInputStream(new File(o.toString())); + } + IOUtils.copy(input, response.getOutputStream()); + } catch (IOException e) { + throw new ControllerExecutionException( + "I/O error copying file to response: " + e.getMessage(), e); + + } + finally { + if (input != null) { + try { + input.close(); + } catch (IOException e) { + // ignore + } } } } } - } - else if (statusSet) { - // GRAILS-6711 nothing to render, just setting status code, so don't render the map - renderView = false; - } - else { - Object object = arguments[0]; - if (object instanceof JSONElement) { - renderView = renderJSON((JSONElement)object, response); - } - else{ - out = getWriterForConfiguredContentType(response, argMap, hasContentType); - webRequest.setOut(out); - - renderView = renderObject(object, out); + else if (statusSet) { + // GRAILS-6711 nothing to render, just setting status code, so don't render the map + renderView = false; } - } - try { - if (!renderView) { - if (out != null) { - out.flush(); + else { + Object object = renderArgument; + if (object instanceof JSONElement) { + renderView = renderJSON((JSONElement)object, response); + } + else{ + try { + renderView = renderObject(object, response.getWriter()); + } + catch (IOException e) { + // ignore + } } } } - catch (IOException e) { - throw new ControllerExecutionException("I/O error executing render method for arguments [" + - argMap + "]: " + e.getMessage(), e); + else { + throw new MissingMethodException(METHOD_SIGNATURE, target.getClass(), arguments); } } - else { - throw new MissingMethodException(METHOD_SIGNATURE, target.getClass(), arguments); - } webRequest.setRenderView(renderView); return null; } - private Writer getWriterForConfiguredContentType(HttpServletResponse response, Map argMap, boolean hasContentType) { - Writer out; - String contentType = hasContentType ? argMap.get(ARGUMENT_CONTENT_TYPE).toString() : null; - if (hasContentType && argMap.containsKey(ARGUMENT_ENCODING)) { - String encoding = argMap.get(ARGUMENT_ENCODING).toString(); - setContentType(response, contentType, encoding); - out = GSPResponseWriter.getInstance(response); - } - else if (hasContentType) { - setContentType(response, contentType, DEFAULT_ENCODING); - out = GSPResponseWriter.getInstance(response); + private String resolveContentTypeBySourceType(final Object renderArgument, String defaultEncoding) { + return renderArgument instanceof GPathResult ? APPLICATION_XML : defaultEncoding; + } + + private boolean applyContentType(HttpServletResponse response, Map argMap, Object renderArgument) { + return applyContentType(response, argMap, renderArgument, true); + } + + private boolean applyContentType(HttpServletResponse response, Map argMap, Object renderArgument, boolean useDefault) { + boolean contentTypeIsDefault = true; + String contentType = resolveContentTypeBySourceType(renderArgument, useDefault ? TEXT_HTML : null); + String encoding = DEFAULT_ENCODING; + if (argMap != null) { + if(argMap.containsKey(ARGUMENT_CONTENT_TYPE)) { + contentType = argMap.get(ARGUMENT_CONTENT_TYPE).toString(); + contentTypeIsDefault = false; + } + if(argMap.containsKey(ARGUMENT_ENCODING)) { + encoding = argMap.get(ARGUMENT_ENCODING).toString(); + contentTypeIsDefault = false; + } } - else { - setContentType(response, TEXT_HTML, DEFAULT_ENCODING, true); - out = GSPResponseWriter.getInstance(response); + if(contentType != null) { + setContentType(response, contentType, encoding, contentTypeIsDefault); + return true; } - return out; + return false; } private boolean renderJSON(JSONElement object, HttpServletResponse response) { - response.setContentType(GrailsWebUtil.getContentType("application/json", gspEncoding)); + response.setContentType(GrailsWebUtil.getContentType("application/json", DEFAULT_ENCODING)); return renderText(object.toString(), response); } - private void detectContentTypeFromFileName(GrailsWebRequest webRequest, HttpServletResponse response, Map argMap, String fileName, boolean hasContentType) { - if (hasContentType) return; - String contentType; + private boolean detectContentTypeFromFileName(GrailsWebRequest webRequest, HttpServletResponse response, Map argMap, String fileName) { MimeUtility mimeUtility = lookupMimeUtility(webRequest); if (mimeUtility != null) { MimeType mimeType = mimeUtility.getMimeTypeForExtension(FilenameUtils.getExtension(fileName)); if (mimeType != null) { - contentType = mimeType.getName(); + String contentType = mimeType.getName(); Object encodingObj = argMap.get(ARGUMENT_ENCODING); String encoding = encodingObj != null ? encodingObj.toString() : DEFAULT_ENCODING; setContentType(response, contentType, encoding); - } else { - throw new ControllerExecutionException("Content type could not be determined for file: " + fileName); + return true; } } + return false; } private MimeUtility lookupMimeUtility(GrailsWebRequest webRequest) { @@ -336,10 +342,16 @@ private MimeUtility lookupMimeUtility(GrailsWebRequest webRequest) { } return mimeUtility; } + + private GSPResponseWriter createResponseWriter(GrailsWebRequest webRequest) { + GSPResponseWriter out = GSPResponseWriter.getInstance(webRequest.getResponse()); + webRequest.setOut(out); + return out; + } private boolean renderTemplate(Object target, GroovyObject controller, GrailsWebRequest webRequest, - Map argMap, Writer out) { - + Map argMap) { + GSPResponseWriter out = createResponseWriter(webRequest); boolean renderView; boolean hasModel = argMap.containsKey(ARGUMENT_MODEL); Object modelObject = null; @@ -403,6 +415,7 @@ else if (hasModel) { Writable w = t.make(new BeanMap(target)); w.writeTo(out); } + out.flush(); renderView = false; } catch (GroovyRuntimeException gre) { @@ -454,8 +467,8 @@ private void setContentType(HttpServletResponse response, String contentType, St } private void setContentType(HttpServletResponse response, String contentType, String encoding, boolean contentTypeIsDefault) { - if (response.getContentType()==null || !contentTypeIsDefault) { - response.setContentType(GrailsWebUtil.getContentType(contentType,encoding)); + if (!contentTypeIsDefault || response.getContentType()==null) { + response.setContentType(GrailsWebUtil.getContentType(contentType, encoding)); } } @@ -515,7 +528,7 @@ private void renderTemplateForBean(Template template, Map binding, Object bean, template.make(binding).writeTo(out); } - private void renderView(GrailsWebRequest webRequest, Map argMap, Object target, GroovyObject controller, boolean hasContentType) { + private void renderView(GrailsWebRequest webRequest, Map argMap, Object target, GroovyObject controller) { String viewName = argMap.get(ARGUMENT_VIEW).toString(); String viewUri = webRequest.getAttributes().getNoSuffixViewURI((GroovyObject) target, viewName); Object modelObject = argMap.get(ARGUMENT_MODEL); @@ -528,9 +541,8 @@ private void renderView(GrailsWebRequest webRequest, Map argMap, Object target, } if (isPromise) return; } - - - getWriterForConfiguredContentType(webRequest.getResponse(),argMap,hasContentType); + + applyContentType(webRequest.getResponse(), argMap, null); Map model; if (modelObject instanceof Map) { @@ -556,28 +568,31 @@ private boolean renderJSON(Closure callable, HttpServletResponse response) { } private boolean renderMarkup(Closure closure, HttpServletResponse response) { - boolean renderView; StreamingMarkupBuilder b = new StreamingMarkupBuilder(); b.setEncoding(response.getCharacterEncoding()); Writable markup = (Writable) b.bind(closure); + return renderWritable(markup, response); + } + + private boolean renderText(CharSequence text, HttpServletResponse response) { try { - markup.writeTo(response.getWriter()); + PrintWriter writer = response.getWriter(); + return renderText(text, writer); } catch (IOException e) { - throw new ControllerExecutionException("I/O error executing render method for arguments [" + closure + "]: " + e.getMessage(), e); + throw new ControllerExecutionException(e.getMessage(), e); } - renderView = false; - return renderView; } - - private boolean renderText(CharSequence text, HttpServletResponse response) { + + private boolean renderWritable(Writable writable, HttpServletResponse response) { try { PrintWriter writer = response.getWriter(); - return renderText(text, writer); + writable.writeTo(writer); } catch (IOException e) { throw new ControllerExecutionException(e.getMessage(), e); } + return false; } private boolean renderText(CharSequence text, Writer writer) { diff --git a/grails-test-suite-uber/src/test/groovy/org/codehaus/groovy/grails/web/servlet/RenderMethodTests.groovy b/grails-test-suite-uber/src/test/groovy/org/codehaus/groovy/grails/web/servlet/RenderMethodTests.groovy index 5f257856c87..6512938ab86 100644 --- a/grails-test-suite-uber/src/test/groovy/org/codehaus/groovy/grails/web/servlet/RenderMethodTests.groovy +++ b/grails-test-suite-uber/src/test/groovy/org/codehaus/groovy/grails/web/servlet/RenderMethodTests.groovy @@ -181,7 +181,7 @@ class RenderMethodTests extends AbstractGrailsControllerTests { def response = mockController.response - assertEquals "text/html;charset=utf-8", response.contentType + assertEquals "text/html;charset=UTF-8", response.contentType assertEquals "hello world!", response.contentAsString }