Do not override requestContextAttribute with null in UrlBasedViewResolvers

Prior to this commit, if a subclass of
org.springframework.web.servlet.view.AbstractView or
org.springframework.web.reactive.result.view.AbstractUrlBasedView
configured a custom value for the requestContextAttribute, that value
was overwritten with null whenever the View was dynamically
instantiated by a UrlBasedViewResolver, and this could lead to
confusing behavior for users of the View.

This commit addresses this issue by ensuring that the
UrlBasedViewResolvers in spring-webmvc and spring-webflux do not
override the requestContextAttribute in a View if the
UrlBasedViewResolver has not been explicitly configured with a custom
requestContextAttribute value.

Closes gh-23129

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/UrlBasedViewResolver.java

 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -30,8 +30,8 @@ import org.springframework.util.Assert;
 import org.springframework.util.PatternMatchUtils;
 
 /**
- * A {@link ViewResolver} that allow direct resolution of symbolic view names
- * to URLs without explicit mapping definition. This is useful if symbolic names
+ * A {@link ViewResolver} that allows direct resolution of symbolic view names
+ * to URLs without explicit mapping definitions. This is useful if symbolic names
  * match the names of view resources in a straightforward manner (i.e. the
  * symbolic name is the unique part of the resource's filename), without the need
  * for a dedicated mapping to be defined for each view.
@@ -59,6 +59,7 @@ import org.springframework.util.PatternMatchUtils;
  * @author Rossen Stoyanchev
  * @author Sebastien Deleuze
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 5.0
  */
 public class UrlBasedViewResolver extends ViewResolverSupport
@@ -266,7 +267,7 @@ public class UrlBasedViewResolver extends ViewResolverSupport
 
 	/**
 	 * Creates a new View instance of the specified view class and configures it.
-	 * Does <i>not</i> perform any lookup for pre-defined View instances.
+	 * <p>Does <i>not</i> perform any lookup for pre-defined View instances.
 	 * <p>Spring lifecycle methods as defined by the bean container do not have to
 	 * be called here: They will be automatically applied afterwards, provided
 	 * that an {@link #setApplicationContext ApplicationContext} is available.
@@ -281,10 +282,14 @@ public class UrlBasedViewResolver extends ViewResolverSupport
 
 		AbstractUrlBasedView view = (AbstractUrlBasedView) BeanUtils.instantiateClass(viewClass);
 		view.setSupportedMediaTypes(getSupportedMediaTypes());
-		view.setRequestContextAttribute(getRequestContextAttribute());
 		view.setDefaultCharset(getDefaultCharset());
 		view.setUrl(getPrefix() + viewName + getSuffix());
 
+		String requestContextAttribute = getRequestContextAttribute();
+		if (requestContextAttribute != null) {
+			view.setRequestContextAttribute(requestContextAttribute);
+		}
+
 		return view;
 	}
 

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/UrlBasedViewResolverTests.java

@@ -37,20 +37,60 @@ import static org.assertj.core.api.Assertions.assertThat;
  *
  * @author Rossen Stoyanchev
  * @author Sebastien Deleuze
+ * @author Sam Brannen
  */
 public class UrlBasedViewResolverTests {
 
-	private UrlBasedViewResolver resolver;
+	private final UrlBasedViewResolver resolver = new UrlBasedViewResolver();
 
 
 	@Before
 	public void setup() {
 		StaticApplicationContext context = new StaticApplicationContext();
 		context.refresh();
-		this.resolver = new UrlBasedViewResolver();
 		this.resolver.setApplicationContext(context);
 	}
 
+	@Test
+	public void urlBasedViewResolverOverridesCustomRequestContextAttributeWithNonNullValue() throws Exception {
+		assertThat(new TestView().getRequestContextAttribute())
+			.as("requestContextAttribute when instantiated directly")
+			.isEqualTo("testRequestContext");
+
+		this.resolver.setViewClass(TestView.class);
+		this.resolver.setRequestContextAttribute("viewResolverRequestContext");
+
+		Mono<View> mono = this.resolver.resolveViewName("example", Locale.getDefault());
+		StepVerifier.create(mono)
+				.consumeNextWith(view -> {
+					assertThat(view).isInstanceOf(TestView.class);
+					assertThat(((TestView) view).getRequestContextAttribute())
+						.as("requestContextAttribute when instantiated dynamically by UrlBasedViewResolver")
+						.isEqualTo("viewResolverRequestContext");
+				})
+				.expectComplete()
+				.verify(Duration.ZERO);
+	}
+
+	@Test
+	public void urlBasedViewResolverDoesNotOverrideCustomRequestContextAttributeWithNull() throws Exception {
+		assertThat(new TestView().getRequestContextAttribute())
+			.as("requestContextAttribute when instantiated directly")
+			.isEqualTo("testRequestContext");
+
+		this.resolver.setViewClass(TestView.class);
+
+		Mono<View> mono = this.resolver.resolveViewName("example", Locale.getDefault());
+		StepVerifier.create(mono)
+				.consumeNextWith(view -> {
+					assertThat(view).isInstanceOf(TestView.class);
+					assertThat(((TestView) view).getRequestContextAttribute())
+						.as("requestContextAttribute when instantiated dynamically by UrlBasedViewResolver")
+						.isEqualTo("testRequestContext");
+				})
+				.expectComplete()
+				.verify(Duration.ZERO);
+	}
 
 	@Test
 	public void viewNames() throws Exception {
@@ -98,6 +138,10 @@ public class UrlBasedViewResolverTests {
 
 	private static class TestView extends AbstractUrlBasedView {
 
+		public TestView() {
+			setRequestContextAttribute("testRequestContext");
+		}
+
 		@Override
 		public boolean checkResourceExists(Locale locale) throws Exception {
 			return true;

spring-webmvc/src/main/java/org/springframework/web/servlet/view/UrlBasedViewResolver.java

 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -34,7 +34,7 @@ import org.springframework.web.servlet.View;
 /**
  * Simple implementation of the {@link org.springframework.web.servlet.ViewResolver}
  * interface, allowing for direct resolution of symbolic view names to URLs,
- * without explicit mapping definition. This is useful if your symbolic names
+ * without explicit mapping definitions. This is useful if your symbolic names
  * match the names of your view resources in a straightforward manner
  * (i.e. the symbolic name is the unique part of the resource's filename),
  * without the need for a dedicated mapping to be defined for each view.
@@ -65,14 +65,15 @@ import org.springframework.web.servlet.View;
  * a symbolic view name to different resources depending on the current locale.
  *
  * <p><b>Note:</b> When chaining ViewResolvers, a UrlBasedViewResolver will check whether
- * the {@link AbstractUrlBasedView#checkResource specified resource actually exists}.
+ * the {@linkplain AbstractUrlBasedView#checkResource specified resource actually exists}.
  * However, with {@link InternalResourceView}, it is not generally possible to
  * determine the existence of the target resource upfront. In such a scenario,
- * a UrlBasedViewResolver will always return View for any given view name;
+ * a UrlBasedViewResolver will always return a View for any given view name;
  * as a consequence, it should be configured as the last ViewResolver in the chain.
  *
  * @author Juergen Hoeller
  * @author Rob Harrop
+ * @author Sam Brannen
  * @since 13.12.2003
  * @see #setViewClass
  * @see #setPrefix
@@ -550,14 +551,17 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 
 		AbstractUrlBasedView view = (AbstractUrlBasedView) BeanUtils.instantiateClass(viewClass);
 		view.setUrl(getPrefix() + viewName + getSuffix());
+		view.setAttributesMap(getAttributesMap());
 
 		String contentType = getContentType();
 		if (contentType != null) {
 			view.setContentType(contentType);
 		}
 
-		view.setRequestContextAttribute(getRequestContextAttribute());
-		view.setAttributesMap(getAttributesMap());
+		String requestContextAttribute = getRequestContextAttribute();
+		if (requestContextAttribute != null) {
+			view.setRequestContextAttribute(requestContextAttribute);
+		}
 
 		Boolean exposePathVariables = getExposePathVariables();
 		if (exposePathVariables != null) {