DATACMNS-377 - Prevent page sizes less than one.

PageRequest now actively prevents page sizes less than one. PageableHandlerMethodArgumentResolver falls back to default page size if it detects a page size less than one. It also rejects invalid default configuration using @PageableDefault.

Author: odrotbohm

src/main/java/org/springframework/data/domain/PageRequest.java

@@ -68,8 +68,8 @@ public PageRequest(int page, int size, Sort sort) {
 			throw new IllegalArgumentException("Page index must not be less than zero!");
 		}
 
-		if (size < 0) {
-			throw new IllegalArgumentException("Page size must not be less than zero!");
+		if (size < 1) {
+			throw new IllegalArgumentException("Page size must not be less than one!");
 		}
 
 		this.page = page;

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

@@ -17,6 +17,8 @@
 
 import static org.springframework.data.web.SpringDataAnnotationUtils.*;
 
+import java.lang.reflect.Method;
+
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.core.MethodParameter;
 import org.springframework.data.domain.PageRequest;
@@ -41,6 +43,8 @@
 @SuppressWarnings("deprecation")
 public class PageableHandlerMethodArgumentResolver implements HandlerMethodArgumentResolver {
 
+	private static final String INVALID_DEFAULT_PAGE_SIZE = "Invalid default page size configured for method %s! Must not be less than one!";
+
 	/**
 	 * A {@link PageableHandlerMethodArgumentResolver} preconfigured to the setup of {@link PageableArgumentResolver}. Use
 	 * that if you need to stick to the former request parameters an 1-indexed behavior. This will be removed in the next
@@ -58,12 +62,12 @@ public class PageableHandlerMethodArgumentResolver implements HandlerMethodArgum
 		LEGACY.sortResolver.setSortParameter("page.sort");
 	}
 
-	private static final Pageable DEFAULT_PAGE_REQUEST = new PageRequest(0, 20);
 	private static final String DEFAULT_PAGE_PARAMETER = "page";
 	private static final String DEFAULT_SIZE_PARAMETER = "size";
 	private static final String DEFAULT_PREFIX = "";
 	private static final String DEFAULT_QUALIFIER_DELIMITER = "_";
 	private static final int DEFAULT_MAX_PAGE_SIZE = 2000;
+	static final Pageable DEFAULT_PAGE_REQUEST = new PageRequest(0, 20);
 
 	private Pageable fallbackPageable = DEFAULT_PAGE_REQUEST;
 	private SortHandlerMethodArgumentResolver sortResolver;
@@ -234,6 +238,10 @@ public Pageable resolveArgument(MethodParameter methodParameter, ModelAndViewCon
 				: defaultOrFallback.getPageNumber();
 		int pageSize = StringUtils.hasText(pageSizeString) ? Integer.parseInt(pageSizeString) : defaultOrFallback
 				.getPageSize();
+
+		// Limit lower bound
+		pageSize = pageSize < 1 ? defaultOrFallback.getPageSize() : pageSize;
+		// Limit upper bound
 		pageSize = pageSize > maxPageSize ? maxPageSize : pageSize;
 
 		Sort sort = sortResolver.resolveArgument(methodParameter, mavContainer, webRequest, binderFactory);
@@ -269,17 +277,24 @@ private Pageable getDefaultFromAnnotationOrFallback(MethodParameter methodParame
 		}
 
 		if (methodParameter.hasParameterAnnotation(PageableDefault.class)) {
-			return getDefaultPageRequestFrom(methodParameter.getParameterAnnotation(PageableDefault.class));
+			return getDefaultPageRequestFrom(methodParameter);
 		}
 
 		return fallbackPageable;
 	}
 
-	private static Pageable getDefaultPageRequestFrom(PageableDefault defaults) {
+	private static Pageable getDefaultPageRequestFrom(MethodParameter parameter) {
+
+		PageableDefault defaults = parameter.getParameterAnnotation(PageableDefault.class);
 
 		Integer defaultPageNumber = defaults.page();
 		Integer defaultPageSize = getSpecificPropertyOrDefaultFromValue(defaults, "size");
 
+		if (defaultPageSize < 1) {
+			Method annotatedMethod = parameter.getMethod();
+			throw new IllegalStateException(String.format(INVALID_DEFAULT_PAGE_SIZE, annotatedMethod));
+		}
+
 		if (defaults.sort().length == 0) {
 			return new PageRequest(defaultPageNumber, defaultPageSize);
 		}

src/test/java/org/springframework/data/domain/PageRequestUnitTests.java

@@ -94,4 +94,12 @@ public void equalsHonoursPageAndSize() {
 		// Does not equal on different size
 		assertNotEqualsAndHashcode(request, new PageRequest(0, 11));
 	}
+
+	/**
+	 * @see DATACMNS-377
+	 */
+	@Test(expected = IllegalArgumentException.class)
+	public void preventsPageSizeLessThanOne() {
+		new PageRequest(0, 0);
+	}
 }

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.web;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.core.MethodParameter;
@@ -33,6 +34,13 @@
  */
 public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefaultUnitTests {
 
+	MethodParameter supportedMethodParameter;
+
+	@Before
+	public void setUp() throws Exception {
+		this.supportedMethodParameter = new MethodParameter(Sample.class.getMethod("supportedMethod", Pageable.class), 0);
+	}
+
 	/**
 	 * @see DATACMNS-335
 	 */
@@ -44,9 +52,7 @@ public void preventsPageSizeFromExceedingMayValueIfConfigured() throws Exception
 		request.addParameter("page", "0");
 		request.addParameter("size", "200");
 
-		MethodParameter parameter = new MethodParameter(Sample.class.getMethod("supportedMethod", Pageable.class), 0);
-
-		assertSupportedAndResult(parameter, new PageRequest(0, 100), request);
+		assertSupportedAndResult(supportedMethodParameter, new PageRequest(0, 100), request);
 	}
 
 	@Test(expected = IllegalArgumentException.class)
@@ -81,6 +87,34 @@ public void qualifierIsUsedInParameterLookup() throws Exception {
 		assertSupportedAndResult(parameter, new PageRequest(2, 10), request);
 	}
 
+	/**
+	 * @see DATACMNS-377
+	 */
+	@Test
+	public void usesDefaultPageSizeIfRequestPageSizeIsLessThanOne() throws Exception {
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addParameter("page", "0");
+		request.addParameter("size", "0");
+
+		assertSupportedAndResult(supportedMethodParameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST,
+				request);
+	}
+
+	/**
+	 * @see DATACMNS-377
+	 */
+	@Test
+	public void rejectsInvalidCustomDefaultForPageSize() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(Sample.class.getMethod("invalidDefaultPageSize", Pageable.class), 0);
+
+		exception.expect(IllegalStateException.class);
+		exception.expectMessage("invalidDefaultPageSize");
+
+		assertSupportedAndResult(parameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST);
+	}
+
 	@Override
 	protected PageableHandlerMethodArgumentResolver getResolver() {
 		PageableHandlerMethodArgumentResolver resolver = new PageableHandlerMethodArgumentResolver();
@@ -99,6 +133,8 @@ interface Sample {
 
 		void unsupportedMethod(String string);
 
+		void invalidDefaultPageSize(@PageableDefault(size = 0) Pageable pageable);
+
 		void simpleDefault(@PageableDefault(size = PAGE_SIZE, page = PAGE_NUMBER) Pageable pageable);
 
 		void simpleDefaultWithSort(@PageableDefault(size = PAGE_SIZE, page = PAGE_NUMBER,