diff --git a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java index c3cad8ac562..1594dcb4208 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java @@ -350,17 +350,22 @@ public class FormHttpMessageConverter implements HttpMessageConverter result = new LinkedMultiValueMap<>(pairs.length); - for (String pair : pairs) { - int idx = pair.indexOf('='); - if (idx == -1) { - result.add(URLDecoder.decode(pair, charset), null); - } - else { - String name = URLDecoder.decode(pair.substring(0, idx), charset); - String value = URLDecoder.decode(pair.substring(idx + 1), charset); - result.add(name, value); + try { + for (String pair : pairs) { + int idx = pair.indexOf('='); + if (idx == -1) { + result.add(URLDecoder.decode(pair, charset), null); + } + else { + 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 --git a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java index 98c937bc733..e0ac599e3f5 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java @@ -49,6 +49,7 @@ import org.springframework.web.testfixture.http.MockHttpOutputMessage; 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 @@ class FormHttpMessageConverterTests { 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 body = new LinkedMultiValueMap<>(); @@ -410,6 +432,17 @@ class FormHttpMessageConverterTests { 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 {