DATACMNS-408 - Handle invalid pagination and sorting parameters gracefully.

The PageableHandlerMethodArgumentResolver and SortHandlerMethodArgumentResolver now transparently ignore invalid values provided for page number, size and sort. We fall back to ignore the invalid values (for sort) and fall back to the defaults where appropriate (page number and size).

Used and adapted test cases from pull request #48.

Author: odrotbohm

src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java

@@ -234,10 +234,10 @@ public Pageable resolveArgument(MethodParameter methodParameter, ModelAndViewCon
 		String pageString = webRequest.getParameter(getParameterNameToUse(pageParameterName, methodParameter));
 		String pageSizeString = webRequest.getParameter(getParameterNameToUse(sizeParameterName, methodParameter));
 
-		int page = StringUtils.hasText(pageString) ? Integer.parseInt(pageString) - (oneIndexedParameters ? 1 : 0)
-				: defaultOrFallback.getPageNumber();
-		int pageSize = StringUtils.hasText(pageSizeString) ? Integer.parseInt(pageSizeString) : defaultOrFallback
-				.getPageSize();
+		int page = StringUtils.hasText(pageString) ? parseAndApplyBoundaries(pageString, 0, Integer.MAX_VALUE)
+				- (oneIndexedParameters ? 1 : 0) : defaultOrFallback.getPageNumber();
+		int pageSize = StringUtils.hasText(pageSizeString) ? parseAndApplyBoundaries(pageSizeString, 0, maxPageSize)
+				: defaultOrFallback.getPageSize();
 
 		// Limit lower bound
 		pageSize = pageSize < 1 ? defaultOrFallback.getPageSize() : pageSize;
@@ -301,4 +301,23 @@ private static Pageable getDefaultPageRequestFrom(MethodParameter parameter) {
 
 		return new PageRequest(defaultPageNumber, defaultPageSize, defaults.direction(), defaults.sort());
 	}
+
+	/**
+	 * Tries to parse the given {@link String} into an integer and applies the given boundaries. Will return the lower
+	 * boundary if the {@link String} cannot be parsed.
+	 * 
+	 * @param parameter
+	 * @param lower
+	 * @param upper
+	 * @return
+	 */
+	private static int parseAndApplyBoundaries(String parameter, int lower, int upper) {
+
+		try {
+			int parsed = Integer.parseInt(parameter);
+			return parsed < lower ? lower : parsed > upper ? upper : parsed;
+		} catch (NumberFormatException e) {
+			return lower;
+		}
+	}
 }

src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java

@@ -251,7 +251,13 @@ Sort parseParameterIntoSort(String[] source, String delimiter) {
 					continue;
 				}
 
-				allOrders.add(new Order(direction, elements[i]));
+				String property = elements[i];
+
+				if (!StringUtils.hasText(property)) {
+					continue;
+				}
+
+				allOrders.add(new Order(direction, property));
 			}
 		}
 

src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.web;
 
+import static org.springframework.data.web.PageableHandlerMethodArgumentResolver.*;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -97,8 +99,7 @@ public void usesDefaultPageSizeIfRequestPageSizeIsLessThanOne() throws Exception
 		request.addParameter("page", "0");
 		request.addParameter("size", "0");
 
-		assertSupportedAndResult(supportedMethodParameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST,
-				request);
+		assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request);
 	}
 
 	/**
@@ -112,7 +113,43 @@ public void rejectsInvalidCustomDefaultForPageSize() throws Exception {
 		exception.expect(IllegalStateException.class);
 		exception.expectMessage("invalidDefaultPageSize");
 
-		assertSupportedAndResult(parameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST);
+		assertSupportedAndResult(parameter, DEFAULT_PAGE_REQUEST);
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void fallsBackToFirstPageIfNegativePageNumberIsGiven() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("page", "-1");
+
+		assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request);
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void pageParamIsNotNumeric() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("page", "a");
+
+		assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request);
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void sizeParamIsNotNumeric() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("size", "a");
+
+		assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request);
 	}
 
 	@Override

src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java

@@ -17,7 +17,11 @@
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.springframework.data.domain.Sort.Direction.*;
 
+import javax.servlet.http.HttpServletRequest;
+
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.core.MethodParameter;
@@ -40,6 +44,13 @@
  */
 public class SortHandlerMethodArgumentResolverUnitTests extends SortDefaultUnitTests {
 
+	static MethodParameter PARAMETER;
+
+	@BeforeClass
+	public static void setUp() throws Exception {
+		PARAMETER = new MethodParameter(Controller.class.getMethod("supportedMethod", Sort.class), 0);
+	}
+
 	/**
 	 * @see DATACMNS-351
 	 */
@@ -126,6 +137,61 @@ public void requestForMultipleSortPropertiesIsUnmarshalledCorrectly() throws Exc
 		assertThat(result, is(new Sort(Direction.ASC, "firstname", "lastname")));
 	}
 
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void parsesEmptySortToNull() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("sort", "");
+
+		assertThat(resolveSort(request, PARAMETER), is(nullValue()));
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void sortParamIsInvalidProperty() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("sort", ",DESC");
+
+		assertThat(resolveSort(request, PARAMETER), is(nullValue()));
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void sortParamIsInvalidPropertyWhenMultiProperty() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("sort", "property1,,DESC");
+
+		assertThat(resolveSort(request, PARAMETER), is(new Sort(DESC, "property1")));
+	}
+
+	/**
+	 * @see DATACMNS-408
+	 */
+	@Test
+	public void sortParamIsEmptyWhenMultiParams() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("sort", "property,DESC");
+		request.addParameter("sort", "");
+
+		assertThat(resolveSort(request, PARAMETER), is(new Sort(DESC, "property")));
+	}
+
+	private static Sort resolveSort(HttpServletRequest request, MethodParameter parameter) throws Exception {
+
+		SortHandlerMethodArgumentResolver resolver = new SortHandlerMethodArgumentResolver();
+		return resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null);
+	}
+
 	private static void assertSupportedAndResolvedTo(NativeWebRequest request, MethodParameter parameter, Sort sort) {
 
 		SortHandlerMethodArgumentResolver resolver = new SortHandlerMethodArgumentResolver();