From 271885cdf43ad41410fa69fc4c522a05af0bde8f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 7 Jul 2019 18:26:59 +0200 Subject: [PATCH] Support empty target request path in FlashMap Prior to this commit, if the user configured an empty path for the targetRequestPath property of a FlashMap, the FlashMapManager threw a StringIndexOutOfBoundsException when saving the output FlashMap for the next request. This commit fixes this by skipping the decoding and normalization of an empty target request path. An empty target request path is therefore effectively treated as the root path. Fixes gh-23240 --- .../support/AbstractFlashMapManager.java | 5 +- .../servlet/support/FlashMapManagerTests.java | 59 ++++++++----------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java index cf38517888..11c1a30287 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java @@ -1,5 +1,5 @@ /* - * 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. @@ -41,6 +41,7 @@ import org.springframework.web.util.UrlPathHelper; * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sam Brannen * @since 3.1.1 */ public abstract class AbstractFlashMapManager implements FlashMapManager { @@ -228,7 +229,7 @@ public abstract class AbstractFlashMapManager implements FlashMapManager { @Nullable private String decodeAndNormalizePath(@Nullable String path, HttpServletRequest request) { - if (path != null) { + if (path != null && !path.isEmpty()) { path = getUrlPathHelper().decodeRequestString(request, path); if (path.charAt(0) != '/') { String requestUri = getUrlPathHelper().getRequestUri(request); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java index e0b135e1fd..0dfa9ee8ad 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/FlashMapManagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -16,19 +16,15 @@ package org.springframework.web.servlet.support; -import static org.junit.Assert.*; - import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.junit.Before; import org.junit.Test; import org.springframework.mock.web.test.MockHttpServletRequest; @@ -36,27 +32,21 @@ import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.web.servlet.FlashMap; import org.springframework.web.util.WebUtils; +import static org.junit.Assert.*; /** * Test fixture for testing {@link AbstractFlashMapManager} methods. * * @author Rossen Stoyanchev + * @author Sam Brannen */ public class FlashMapManagerTests { - private TestFlashMapManager flashMapManager; + private final TestFlashMapManager flashMapManager = new TestFlashMapManager(); - private MockHttpServletRequest request; + private final MockHttpServletRequest request = new MockHttpServletRequest(); - private MockHttpServletResponse response; - - - @Before - public void setup() { - this.flashMapManager = new TestFlashMapManager(); - this.request = new MockHttpServletRequest(); - this.response = new MockHttpServletResponse(); - } + private final MockHttpServletResponse response = new MockHttpServletResponse(); @Test @@ -73,9 +63,7 @@ public class FlashMapManagerTests { assertEquals(flashMap, inputFlashMap); } - // SPR-8779 - - @Test + @Test // SPR-8779 public void retrieveAndUpdateMatchByOriginatingPath() { FlashMap flashMap = new FlashMap(); flashMap.put("key", "value"); @@ -133,9 +121,7 @@ public class FlashMapManagerTests { assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); } - // SPR-8798 - - @Test + @Test // SPR-8798 public void retrieveAndUpdateMatchWithMultiValueParam() { FlashMap flashMap = new FlashMap(); flashMap.put("name", "value"); @@ -180,7 +166,7 @@ public class FlashMapManagerTests { } @Test - public void retrieveAndUpdateRemoveExpired() throws InterruptedException { + public void retrieveAndUpdateRemoveExpired() { List flashMaps = new ArrayList<>(); for (int i = 0; i < 5; i++) { FlashMap expiredFlashMap = new FlashMap(); @@ -195,7 +181,7 @@ public class FlashMapManagerTests { } @Test - public void saveOutputFlashMapEmpty() throws InterruptedException { + public void saveOutputFlashMapEmpty() { FlashMap flashMap = new FlashMap(); this.flashMapManager.saveOutputFlashMap(flashMap, this.request, this.response); @@ -205,7 +191,7 @@ public class FlashMapManagerTests { } @Test - public void saveOutputFlashMap() throws InterruptedException { + public void saveOutputFlashMap() { FlashMap flashMap = new FlashMap(); flashMap.put("name", "value"); @@ -219,7 +205,7 @@ public class FlashMapManagerTests { } @Test - public void saveOutputFlashMapDecodeTargetPath() throws InterruptedException { + public void saveOutputFlashMapDecodeTargetPath() { FlashMap flashMap = new FlashMap(); flashMap.put("key", "value"); @@ -230,7 +216,7 @@ public class FlashMapManagerTests { } @Test - public void saveOutputFlashMapNormalizeTargetPath() throws InterruptedException { + public void saveOutputFlashMapNormalizeTargetPath() { FlashMap flashMap = new FlashMap(); flashMap.put("key", "value"); @@ -265,11 +251,19 @@ public class FlashMapManagerTests { assertEquals("/once/only", flashMap.getTargetRequestPath()); } - // SPR-9657, SPR-11504 + @Test // gh-23240 + public void saveOutputFlashMapAndNormalizeEmptyTargetPath() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); - @Test + flashMap.setTargetRequestPath(""); + this.flashMapManager.saveOutputFlashMap(flashMap, this.request, this.response); + + assertEquals("", flashMap.getTargetRequestPath()); + } + + @Test // SPR-9657, SPR-11504 public void saveOutputFlashMapDecodeParameters() throws Exception { - FlashMap flashMap = new FlashMap(); flashMap.put("key", "value"); flashMap.setTargetRequestPath("/path"); @@ -295,11 +289,8 @@ public class FlashMapManagerTests { assertEquals("value", flashMap.get("key")); } - // SPR-12569 - - @Test + @Test // SPR-12569 public void flashAttributesWithQueryParamsWithSpace() throws Exception { - String encodedValue = URLEncoder.encode("1 2", "UTF-8"); FlashMap flashMap = new FlashMap();