Refine FormHttpMessageConverter exception handling

FormHttpMessageConverter could throw a more specific
HttpMessageNotReadableException instead of an IllegalArgumentException
when the http form data is invalid.

See gh-34594
Signed-off-by: Russell Bolles <[email protected]>

Author: Russell Bolles

Diff for: spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java

@@ -353,12 +353,22 @@ public MultiValueMap<String, String> read(@Nullable Class<? extends MultiValueMa
 		for (String pair : pairs) {
 			int idx = pair.indexOf('=');
 			if (idx == -1) {
-				result.add(URLDecoder.decode(pair, charset), null);
+				try {
+					result.add(URLDecoder.decode(pair, charset), null);
+				}
+				catch (IllegalArgumentException ex) {
+					throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage);
+				}
 			}
 			else {
-				String name = URLDecoder.decode(pair.substring(0, idx), charset);
-				String value = URLDecoder.decode(pair.substring(idx + 1), charset);
-				result.add(name, value);
+				try {
+					String name = URLDecoder.decode(pair.substring(0, idx), charset);
+					String value = URLDecoder.decode(pair.substring(idx + 1), charset);
+					result.add(name, value);
+				}
+				catch (IllegalArgumentException ex) {
+					throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage);
+				}
 			}
 		}
 		return result;

Diff for: spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java

@@ -49,6 +49,7 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED;
 import static org.springframework.http.MediaType.APPLICATION_JSON;
 import static org.springframework.http.MediaType.MULTIPART_FORM_DATA;
@@ -132,6 +133,27 @@ void readForm() throws Exception {
 		assertThat(result.getFirst("name 3")).as("Invalid result").isNull();
 	}
 
+	@Test
+	void readInvalidFormWithValueThatWontUrlDecode() {
+		//java.net.URLDecoder doesn't like negative integer values after a % character
+		String body = "name+1=value+1&name+2=value+2%" + ((char)-1);
+		assertInvalidFormIsRejectedWithSpecificException(body);
+	}
+
+	@Test
+	void readInvalidFormWithNameThatWontUrlDecode() {
+		//java.net.URLDecoder doesn't like negative integer values after a % character
+		String body = "name+1=value+1&name+2%" + ((char)-1) + "=value+2";
+		assertInvalidFormIsRejectedWithSpecificException(body);
+	}
+
+	@Test
+	void readInvalidFormWithNameWithNoValueThatWontUrlDecode() {
+		//java.net.URLDecoder doesn't like negative integer values after a % character
+		String body = "name+1=value+1&name+2%" + ((char)-1);
+		assertInvalidFormIsRejectedWithSpecificException(body);
+	}
+
 	@Test
 	void writeForm() throws IOException {
 		MultiValueMap<String, String> body = new LinkedMultiValueMap<>();
@@ -410,6 +432,17 @@ private void assertCannotWrite(MediaType mediaType) {
 		assertThat(this.converter.canWrite(clazz, mediaType)).as(clazz.getSimpleName() + " : " + mediaType).isFalse();
 	}
 
+	private void assertInvalidFormIsRejectedWithSpecificException(String body) {
+		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.ISO_8859_1));
+		inputMessage.getHeaders().setContentType(
+				new MediaType("application", "x-www-form-urlencoded", StandardCharsets.ISO_8859_1));
+
+		assertThatThrownBy(() -> this.converter.read(null, inputMessage))
+				.isInstanceOf(HttpMessageNotReadableException.class)
+				.hasCauseInstanceOf(IllegalArgumentException.class)
+				.hasMessage("Could not decode HTTP form payload");
+	}
+
 
 	private static class MockHttpOutputMessageRequestContext implements UploadContext {