SPR-5537: ReSTful URLs with content type extension do not work properly

Author: poutsma

org.springframework.core/src/main/java/org/springframework/util/AntPatchStringMatcher.java

@@ -16,11 +16,15 @@
 
 package org.springframework.util;
 
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Package-protected helper class for {@link AntPathMatcher}.
- * Tests whether or not a string matches against a pattern.
+ * Tests whether or not a string matches against a pattern using a regular expression.
  *
  * <p>The pattern may contain special characters: '*' means zero or more characters;
  * '?' means one and only one character; '{' and '}' indicate a URI template pattern.
@@ -30,189 +34,66 @@
  */
 class AntPatchStringMatcher {
 
-	private final char[] patArr;
+	private static final Pattern GLOB_PATTERN = Pattern.compile("\\?|\\*|\\{([^/]+?)\\}");
 
-	private final char[] strArr;
+	private final Pattern pattern;
 
-	private int patIdxStart = 0;
+	private String str;
 
-	private int patIdxEnd;
-
-	private int strIdxStart = 0;
-
-	private int strIdxEnd;
-
-	private char ch;
+	private final List<String> variableNames = new LinkedList<String>();
 
 	private final Map<String, String> uriTemplateVariables;
 
-
-	/**
-	 * Construct a new instance of the <code>AntPatchStringMatcher</code>.
-	 */
-	public AntPatchStringMatcher(String pattern, String str, Map<String, String> uriTemplateVariables) {
-		this.patArr = pattern.toCharArray();
-		this.strArr = str.toCharArray();
-		this.patIdxEnd = this.patArr.length - 1;
-		this.strIdxEnd = this.strArr.length - 1;
+	/** Construct a new instance of the <code>AntPatchStringMatcher</code>. */
+	AntPatchStringMatcher(String pattern, String str, Map<String, String> uriTemplateVariables) {
+		this.str = str;
 		this.uriTemplateVariables = uriTemplateVariables;
+		this.pattern = createPattern(pattern);
 	}
 
-
-	/**
-	 * Main entry point.
-	 * @return <code>true</code> if the string matches against the pattern, or <code>false</code> otherwise.
-	 */
-	public boolean matchStrings() {
-		if (shortcutPossible()) {
-			return doShortcut();
-		}
-		if (patternContainsOnlyStar()) {
-			return true;
-		}
-		if (patternContainsOneTemplateVariable()) {
-			addTemplateVariable(0, patIdxEnd, 0, strIdxEnd);
-			return true;
-		}
-		if (!matchBeforeFirstStarOrCurly()) {
-			return false;
-		}
-		if (allCharsUsed()) {
-			return onlyStarsLeft();
-		}
-		if (!matchAfterLastStarOrCurly()) {
-			return false;
-		}
-		if (allCharsUsed()) {
-			return onlyStarsLeft();
-		}
-		// process pattern between stars. padIdxStart and patIdxEnd point
-		// always to a '*'.
-		while (patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd) {
-			int patIdxTmp;
-			if (patArr[patIdxStart] == '{') {
-				patIdxTmp = findClosingCurly();
-				addTemplateVariable(patIdxStart, patIdxTmp, strIdxStart, strIdxEnd);
-				patIdxStart = patIdxTmp + 1;
-				strIdxStart = strIdxEnd + 1;
-				continue;
-			}
-			patIdxTmp = findNextStarOrCurly();
-			if (consecutiveStars(patIdxTmp)) {
-				continue;
-			}
-			// Find the pattern between padIdxStart & padIdxTmp in str between
-			// strIdxStart & strIdxEnd
-			int patLength = (patIdxTmp - patIdxStart - 1);
-			int strLength = (strIdxEnd - strIdxStart + 1);
-			int foundIdx = -1;
-			strLoop:
-			for (int i = 0; i <= strLength - patLength; i++) {
-				for (int j = 0; j < patLength; j++) {
-					ch = patArr[patIdxStart + j + 1];
-					if (ch != '?') {
-						if (ch != strArr[strIdxStart + i + j]) {
-							continue strLoop;
-						}
-					}
-				}
-
-				foundIdx = strIdxStart + i;
-				break;
-			}
-
-			if (foundIdx == -1) {
-				return false;
-			}
-
-			patIdxStart = patIdxTmp;
-			strIdxStart = foundIdx + patLength;
-		}
-
-		return onlyStarsLeft();
-	}
-
-	private void addTemplateVariable(int curlyIdxStart, int curlyIdxEnd, int valIdxStart, int valIdxEnd) {
-		if (uriTemplateVariables != null) {
-			String varName = new String(patArr, curlyIdxStart + 1, curlyIdxEnd - curlyIdxStart - 1);
-			String varValue = new String(strArr, valIdxStart, valIdxEnd - valIdxStart + 1);
-			uriTemplateVariables.put(varName, varValue);
-		}
-	}
-
-	private boolean consecutiveStars(int patIdxTmp) {
-		if (patIdxTmp == patIdxStart + 1 && patArr[patIdxStart] == '*' && patArr[patIdxTmp] == '*') {
-			// Two stars next to each other, skip the first one.
-			patIdxStart++;
-			return true;
-		}
-		return false;
-	}
-
-	private int findNextStarOrCurly() {
-		for (int i = patIdxStart + 1; i <= patIdxEnd; i++) {
-			if (patArr[i] == '*' || patArr[i] == '{') {
-				return i;
+	private Pattern createPattern(String pattern) {
+		StringBuilder patternBuilder = new StringBuilder();
+		Matcher m = GLOB_PATTERN.matcher(pattern);
+		int end = 0;
+		while (m.find()) {
+			patternBuilder.append(quote(pattern, end, m.start()));
+			String match = m.group();
+			if ("?".equals(match)) {
+				patternBuilder.append('.');
 			}
-		}
-		return -1;
-	}
-
-	private int findClosingCurly() {
-		for (int i = patIdxStart + 1; i <= patIdxEnd; i++) {
-			if (patArr[i] == '}') {
-				return i;
+			else if ("*".equals(match)) {
+				patternBuilder.append(".*");
 			}
-		}
-		return -1;
-	}
-
-	private boolean onlyStarsLeft() {
-		for (int i = patIdxStart; i <= patIdxEnd; i++) {
-			if (patArr[i] != '*') {
-				return false;
+			else if (match.startsWith("{") && match.endsWith("}")) {
+				patternBuilder.append("(.*)");
+				variableNames.add(m.group(1));
 			}
+			end = m.end();
 		}
-		return true;
-	}
-
-	private boolean allCharsUsed() {
-		return strIdxStart > strIdxEnd;
+		patternBuilder.append(quote(pattern, end, pattern.length()));
+		return  Pattern.compile(patternBuilder.toString());
 	}
 
-	private boolean shortcutPossible() {
-		for (char ch : patArr) {
-			if (ch == '*' || ch == '{' || ch == '}') {
-				return false;
-			}
+	private String quote(String s, int start, int end) {
+		if (start == end) {
+			return "";
 		}
-		return true;
+		return Pattern.quote(s.substring(start, end));
 	}
 
-	private boolean doShortcut() {
-		if (patIdxEnd != strIdxEnd) {
-			return false; // Pattern and string do not have the same size
-		}
-		for (int i = 0; i <= patIdxEnd; i++) {
-			ch = patArr[i];
-			if (ch != '?') {
-				if (ch != strArr[i]) {
-					return false;// Character mismatch
-				}
-			}
-		}
-		return true; // String matches against pattern
-	}
-
-	private boolean patternContainsOnlyStar() {
-		return (patIdxEnd == 0 && patArr[0] == '*');
-	}
-
-	private boolean patternContainsOneTemplateVariable() {
-		if ((patIdxEnd >= 2 && patArr[0] == '{' && patArr[patIdxEnd] == '}')) {
-			for (int i = 1; i < patIdxEnd; i++) {
-				if (patArr[i] == '}') {
-					return false;
+	/**
+	 * Main entry point.
+	 *
+	 * @return <code>true</code> if the string matches against the pattern, or <code>false</code> otherwise.
+	 */
+	public boolean matchStrings() {
+		Matcher matcher = pattern.matcher(str);
+		if (matcher.matches()) {
+			if (uriTemplateVariables != null) {
+				for (int i = 1; i <= matcher.groupCount(); i++) {
+					String name = this.variableNames.get(i - 1);
+					String value = matcher.group(i);
+					uriTemplateVariables.put(name, value);
 				}
 			}
 			return true;
@@ -222,30 +103,4 @@ private boolean patternContainsOneTemplateVariable() {
 		}
 	}
 
-	private boolean matchBeforeFirstStarOrCurly() {
-		while ((ch = patArr[patIdxStart]) != '*' && ch != '{' && strIdxStart <= strIdxEnd) {
-			if (ch != '?') {
-				if (ch != strArr[strIdxStart]) {
-					return false;
-				}
-			}
-			patIdxStart++;
-			strIdxStart++;
-		}
-		return true;
-	}
-
-	private boolean matchAfterLastStarOrCurly() {
-		while ((ch = patArr[patIdxEnd]) != '*' && ch != '}' && strIdxStart <= strIdxEnd) {
-			if (ch != '?') {
-				if (ch != strArr[strIdxEnd]) {
-					return false;
-				}
-			}
-			patIdxEnd--;
-			strIdxEnd--;
-		}
-		return true;
-	}
-
 }

org.springframework.core/src/test/java/org/springframework/util/AntPathMatcherTests.java

@@ -43,7 +43,7 @@ public void createMatcher() {
 	}
 
 	@Test
-	public void standard() {
+	public void match() {
 		// test exact matching
 		assertTrue(pathMatcher.match("test", "test"));
 		assertTrue(pathMatcher.match("/test", "/test"));
@@ -123,6 +123,8 @@ public void standard() {
 		assertFalse(pathMatcher.match("/x/x/**/bla", "/x/x/x/"));
 
 		assertTrue(pathMatcher.match("", ""));
+
+		assertTrue(pathMatcher.match("/{bla}.*", "/testing.html"));
 	}
 
 	@Test
@@ -319,8 +321,17 @@ public void extractUriTemplateVariables() throws Exception {
 		result = pathMatcher.extractUriTemplateVariables("/{page}.html", "/42.html");
 		assertEquals(Collections.singletonMap("page", "42"), result);
 
+		result = pathMatcher.extractUriTemplateVariables("/{page}.*", "/42.html");
+		assertEquals(Collections.singletonMap("page", "42"), result);
+
 		result = pathMatcher.extractUriTemplateVariables("/A-{B}-C", "/A-b-C");
 		assertEquals(Collections.singletonMap("B", "b"), result);
+
+		result = pathMatcher.extractUriTemplateVariables("/{name}.{extension}", "/test.html");
+		expected = new LinkedHashMap<String, String>();
+		expected.put("name", "test");
+		expected.put("extension", "html");
+		assertEquals(expected, result);
 	}
 
 	@Test

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java

@@ -6,7 +6,7 @@
 import java.util.Date;
 import javax.servlet.ServletException;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import org.junit.Test;
 
 import org.springframework.beans.BeansException;
@@ -81,6 +81,17 @@ public void relative() throws Exception {
 		assertEquals("test-42-21", response.getContentAsString());
 	}
 
+	@Test
+	public void extension() throws Exception {
+		initServlet(SimpleUriTemplateController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/42.xml");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("test-42", response.getContentAsString());
+
+	}
+
 	private void initServlet(final Class<?> controllerclass) throws ServletException {
 		servlet = new DispatcherServlet() {
 			@Override
@@ -103,8 +114,8 @@ protected WebApplicationContext createWebApplicationContext(WebApplicationContex
 	public static class SimpleUriTemplateController {
 
 		@RequestMapping("/{root}")
-		public void handle(@PathVariable("root") String root, Writer writer) throws IOException {
-			assertEquals("Invalid path variable value", "42", root);
+		public void handle(@PathVariable("root") int root, Writer writer) throws IOException {
+			assertEquals("Invalid path variable value", 42, root);
 			writer.write("test-" + root);
 		}