From 573e74b8bd735bf0f120e4f5778d471f3d057a50 Mon Sep 17 00:00:00 2001
From: Russell Bolles <rbolles@netflix.com>
Date: Thu, 13 Mar 2025 12:22:25 -0700
Subject: [PATCH 1/2] 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 <rbolles@netflix.com>
---
 .../converter/FormHttpMessageConverter.java   | 18 +++++++---
 .../FormHttpMessageConverterTests.java        | 33 +++++++++++++++++++
 2 files changed, 47 insertions(+), 4 deletions(-)

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 34062542faa..7e72fad2309 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
@@ -353,12 +353,22 @@ public class FormHttpMessageConverter implements HttpMessageConverter<MultiValue
 		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 --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<String, String> 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 {
 

From 46859d63919f72d192b8c6ca65432c5f3d966a1f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?=
 <sdeleuze@users.noreply.github.com>
Date: Mon, 17 Mar 2025 11:39:15 +0100
Subject: [PATCH 2/2] Polishing

Closes gh-34594
---
 .../converter/FormHttpMessageConverter.java   | 23 ++++++++-----------
 1 file changed, 9 insertions(+), 14 deletions(-)

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 7e72fad2309..38295f2895f 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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -350,27 +350,22 @@ public class FormHttpMessageConverter implements HttpMessageConverter<MultiValue
 
 		String[] pairs = StringUtils.tokenizeToStringArray(body, "&");
 		MultiValueMap<String, String> result = new LinkedMultiValueMap<>(pairs.length);
-		for (String pair : pairs) {
-			int idx = pair.indexOf('=');
-			if (idx == -1) {
-				try {
+		try {
+			for (String pair : pairs) {
+				int idx = pair.indexOf('=');
+				if (idx == -1) {
 					result.add(URLDecoder.decode(pair, charset), null);
 				}
-				catch (IllegalArgumentException ex) {
-					throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage);
-				}
-			}
-			else {
-				try {
+				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);
-				}
 			}
 		}
+		catch (IllegalArgumentException ex) {
+			throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage);
+		}
 		return result;
 	}