From 51325afbcc021ede4fe9cb020f72aa5edcee936d Mon Sep 17 00:00:00 2001
From: Arjen Poutsma <apoutsma@pivotal.io>
Date: Thu, 3 May 2018 09:41:14 +0200
Subject: [PATCH] Clean up path variables after non match

This commit makes sure the nested path variables are only commited to
the attributes when all predicates match.

Issue: SPR-16692
---
 .../function/server/RequestPredicates.java    | 23 +++++++++-------
 .../server/NestedRouteIntegrationTests.java   | 27 ++++++++++++++-----
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
index 72e0a5dc72..4959658542 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java
@@ -362,10 +362,7 @@ public abstract class RequestPredicates {
 		@Override
 		public Optional<ServerRequest> nest(ServerRequest request) {
 			return Optional.ofNullable(this.pattern.matchStartOfPath(request.pathContainer()))
-					.map(info -> {
-						mergeTemplateVariables(request, info.getUriVariables());
-						return new SubPathServerRequestWrapper(request, info);
-					});
+					.map(info -> new SubPathServerRequestWrapper(request, info));
 		}
 
 		private void mergeTemplateVariables(ServerRequest request, Map<String, String> variables) {
@@ -475,10 +472,21 @@ public abstract class RequestPredicates {
 
 		private final PathContainer subPathContainer;
 
+		private final Map<String, String> pathVariables;
 
 		public SubPathServerRequestWrapper(ServerRequest request, PathPattern.PathRemainingMatchInfo info) {
 			this.request = request;
 			this.subPathContainer = new SubPathContainer(info.getPathRemaining());
+
+			this.pathVariables = mergePathVariables(request, info.getUriVariables());
+		}
+
+		private static Map<String, String> mergePathVariables(ServerRequest request,
+				Map<String, String> pathVariables) {
+
+			Map<String, String> result = new LinkedHashMap<>(request.pathVariables());
+			result.putAll(pathVariables);
+			return Collections.unmodifiableMap(result);
 		}
 
 		@Override
@@ -581,14 +589,9 @@ public abstract class RequestPredicates {
 			return this.request.queryParams();
 		}
 
-		@Override
-		public String pathVariable(String name) {
-			return this.request.pathVariable(name);
-		}
-
 		@Override
 		public Map<String, String> pathVariables() {
-			return this.request.pathVariables();
+			return this.pathVariables;
 		}
 
 		@Override
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java
index 29e62edbcf..087d5430d5 100644
--- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java
+++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -17,7 +17,6 @@
 package org.springframework.web.reactive.function.server;
 
 import org.junit.Test;
-import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import org.springframework.http.HttpStatus;
@@ -46,7 +45,8 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
 						.andRoute(GET("/baz"), nestedHandler::baz))
 				.andNest(GET("/{foo}"),
 						nest(GET("/{bar}"),
-								route(GET("/{baz}"), nestedHandler::variables)));
+								route(GET("/{baz}"), nestedHandler::variables)))
+				.andRoute(GET("/{qux}/quux"), nestedHandler::variables);
 	}
 
 
@@ -74,7 +74,18 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
 				restTemplate.getForEntity("http://localhost:" + port + "/1/2/3", String.class);
 
 		assertEquals(HttpStatus.OK, result.getStatusCode());
-		assertEquals("1-2-3", result.getBody());
+		assertEquals("{foo=1, bar=2, baz=3}", result.getBody());
+	}
+
+	// SPR 16692
+	@Test
+	public void removeFailedPathVariables() throws Exception {
+		ResponseEntity<String> result =
+				restTemplate.getForEntity("http://localhost:" + port + "/qux/quux", String.class);
+
+		assertEquals(HttpStatus.OK, result.getStatusCode());
+		assertEquals("{qux=qux}", result.getBody());
+
 	}
 
 
@@ -89,11 +100,13 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati
 		}
 
 		public Mono<ServerResponse> variables(ServerRequest request) {
-			Flux<String> responseBody =
-					Flux.just(request.pathVariable("foo"), "-", request.pathVariable("bar"), "-",
-							request.pathVariable("baz"));
+			assertEquals(request.pathVariables(),
+					request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE));
+
+			Mono<String> responseBody = Mono.just(request.pathVariables().toString());
 			return ServerResponse.ok().body(responseBody, String.class);
 		}
+
 	}
 
 }