From 648245b2002f1ba2a51da2ed9817730e1224fac2 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 11 Feb 2014 23:41:09 +0100 Subject: [PATCH] MarshallingView should not close response OutputStream after copying to it Also throws IllegalStateException instead of ServletException now, consistent with other Spring MVC classes. Issue: SPR-11411 --- .../web/servlet/view/xml/MarshallingView.java | 89 +++++++++---------- .../view/xml/MarshallingViewTests.java | 21 +++-- 2 files changed, 54 insertions(+), 56 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xml/MarshallingView.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xml/MarshallingView.java index c08daa0a85..b7e2717067 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xml/MarshallingView.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xml/MarshallingView.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,26 +18,24 @@ package org.springframework.web.servlet.view.xml; import java.io.ByteArrayOutputStream; import java.util.Map; - -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.xml.transform.stream.StreamResult; -import org.springframework.beans.BeansException; import org.springframework.oxm.Marshaller; import org.springframework.util.Assert; -import org.springframework.util.FileCopyUtils; +import org.springframework.util.StreamUtils; import org.springframework.web.servlet.View; import org.springframework.web.servlet.view.AbstractView; /** - * Spring-MVC {@link View} that allows for response context to be rendered as the result of marshalling by a {@link - * Marshaller}. + * Spring-MVC {@link View} that allows for response context to be rendered as the result + * of marshalling by a {@link Marshaller}. * - *

The Object to be marshalled is supplied as a parameter in the model and then {@linkplain - * #locateToBeMarshalled(Map) detected} during response rendering. Users can either specify a specific entry in the - * model via the {@link #setModelKey(String) sourceKey} property or have Spring locate the Source object. + *

The Object to be marshalled is supplied as a parameter in the model and then + * {@linkplain #locateToBeMarshalled(Map) detected} during response rendering. Users can + * either specify a specific entry in the model via the {@link #setModelKey(String) sourceKey} + * property or have Spring locate the Source object. * * @author Arjen Poutsma * @since 3.0 @@ -49,13 +47,15 @@ public class MarshallingView extends AbstractView { */ public static final String DEFAULT_CONTENT_TYPE = "application/xml"; + private Marshaller marshaller; private String modelKey; + /** - * Constructs a new {@code MarshallingView} with no {@link Marshaller} set. The marshaller must be set after - * construction by invoking {@link #setMarshaller(Marshaller)}. + * Constructs a new {@code MarshallingView} with no {@link Marshaller} set. + * The marshaller must be set after construction by invoking {@link #setMarshaller}. */ public MarshallingView() { setContentType(DEFAULT_CONTENT_TYPE); @@ -66,24 +66,22 @@ public class MarshallingView extends AbstractView { * Constructs a new {@code MarshallingView} with the given {@link Marshaller} set. */ public MarshallingView(Marshaller marshaller) { - Assert.notNull(marshaller, "'marshaller' must not be null"); - setContentType(DEFAULT_CONTENT_TYPE); - this.marshaller = marshaller; - setExposePathVariables(false); + this(); + setMarshaller(marshaller); } + /** * Sets the {@link Marshaller} to be used by this view. */ public void setMarshaller(Marshaller marshaller) { - Assert.notNull(marshaller, "'marshaller' must not be null"); + Assert.notNull(marshaller, "Marshaller must not be null"); this.marshaller = marshaller; } /** - * Set the name of the model key that represents the object to be marshalled. If not specified, the model map will be - * searched for a supported value type. - * + * Set the name of the model key that represents the object to be marshalled. + * If not specified, the model map will be searched for a supported value type. * @see Marshaller#supports(Class) */ public void setModelKey(String modelKey) { @@ -91,55 +89,56 @@ public class MarshallingView extends AbstractView { } @Override - protected void initApplicationContext() throws BeansException { - Assert.notNull(marshaller, "Property 'marshaller' is required"); + protected void initApplicationContext() { + Assert.notNull(this.marshaller, "Property 'marshaller' is required"); } + @Override - protected void renderMergedOutputModel(Map model, - HttpServletRequest request, - HttpServletResponse response) throws Exception { + protected void renderMergedOutputModel(Map model, HttpServletRequest request, + HttpServletResponse response) throws Exception { + Object toBeMarshalled = locateToBeMarshalled(model); if (toBeMarshalled == null) { - throw new ServletException("Unable to locate object to be marshalled in model: " + model); + throw new IllegalStateException("Unable to locate object to be marshalled in model: " + model); } ByteArrayOutputStream bos = new ByteArrayOutputStream(2048); - marshaller.marshal(toBeMarshalled, new StreamResult(bos)); + this.marshaller.marshal(toBeMarshalled, new StreamResult(bos)); setResponseContentType(request, response); response.setContentLength(bos.size()); - FileCopyUtils.copy(bos.toByteArray(), response.getOutputStream()); + StreamUtils.copy(bos.toByteArray(), response.getOutputStream()); } /** - * Locates the object to be marshalled. The default implementation first attempts to look under the configured - * {@linkplain #setModelKey(String) model key}, if any, before attempting to locate an object of {@linkplain - * Marshaller#supports(Class) supported type}. - * + * Locates the object to be marshalled. The default implementation first attempts to look + * under the configured {@linkplain #setModelKey(String) model key}, if any, before attempting + * to locate an object of {@linkplain Marshaller#supports(Class) supported type}. * @param model the model Map * @return the Object to be marshalled (or {@code null} if none found) - * @throws ServletException if the model object specified by the {@linkplain #setModelKey(String) model key} is not - * supported by the marshaller + * @throws IllegalStateException if the model object specified by the + * {@linkplain #setModelKey(String) model key} is not supported by the marshaller * @see #setModelKey(String) */ - protected Object locateToBeMarshalled(Map model) throws ServletException { + protected Object locateToBeMarshalled(Map model) throws IllegalStateException { if (this.modelKey != null) { - Object o = model.get(this.modelKey); - if (o == null) { - throw new ServletException("Model contains no object with key [" + modelKey + "]"); + Object obj = model.get(this.modelKey); + if (obj == null) { + throw new IllegalStateException("Model contains no object with key [" + this.modelKey + "]"); } - if (!this.marshaller.supports(o.getClass())) { - throw new ServletException("Model object [" + o + "] retrieved via key [" + modelKey + - "] is not supported by the Marshaller"); + if (!this.marshaller.supports(obj.getClass())) { + throw new IllegalStateException("Model object [" + obj + "] retrieved via key [" + + this.modelKey + "] is not supported by the Marshaller"); } - return o; + return obj; } - for (Object o : model.values()) { - if (o != null && this.marshaller.supports(o.getClass())) { - return o; + for (Object obj : model.values()) { + if (obj != null && this.marshaller.supports(obj.getClass())) { + return obj; } } return null; } + } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MarshallingViewTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MarshallingViewTests.java index 6b767c7f78..4b6cddcda6 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MarshallingViewTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/xml/MarshallingViewTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,12 +18,11 @@ package org.springframework.web.servlet.view.xml; import java.util.HashMap; import java.util.Map; - -import javax.servlet.ServletException; import javax.xml.transform.stream.StreamResult; import org.junit.Before; import org.junit.Test; + import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.oxm.Marshaller; @@ -93,9 +92,9 @@ public class MarshallingViewTests { try { view.render(model, request, response); - fail("ServletException expected"); + fail("IllegalStateException expected"); } - catch (ServletException ex) { + catch (IllegalStateException ex) { // expected } assertEquals("Invalid content length", 0, response.getContentLength()); @@ -112,9 +111,9 @@ public class MarshallingViewTests { try { view.render(model, request, response); - fail("ServletException expected"); + fail("IllegalStateException expected"); } - catch (ServletException ex) { + catch (IllegalStateException ex) { // expected } assertEquals("Invalid content length", 0, response.getContentLength()); @@ -135,9 +134,9 @@ public class MarshallingViewTests { try { view.render(model, request, response); - fail("ServletException expected"); + fail("IllegalStateException expected"); } - catch (ServletException ex) { + catch (IllegalStateException ex) { // expected } } @@ -174,9 +173,9 @@ public class MarshallingViewTests { try { view.render(model, request, response); - fail("ServletException expected"); + fail("IllegalStateException expected"); } - catch (ServletException ex) { + catch (IllegalStateException ex) { // expected } }