bugfix: fix error parsing application/x-www-form-urlencoded requests in Http2HttpHandler

changes/en-us/2.x.md

@@ -47,6 +47,7 @@ Add changes here for all PR submitted to the 2.x branch.
 - [[#7662](https://github.com/apache/incubator-seata/pull/7662)] ensure visibility of rm and The methods in MockTest are executed in order
 - [[#7683](https://github.com/apache/incubator-seata/pull/7683)] Override XABranchXid equals() and hashCode() to fix memory leak in mysql driver
 - [[#7643](https://github.com/apache/incubator-seata/pull/7643)] fix DM transaction rollback not using database auto-increment primary keys
+- [[#7749](https://github.com/apache/incubator-seata/pull/7749)] fix error parsing application/x-www-form-urlencoded requests in Http2HttpHandler
 
 
 ### optimize:

changes/zh-cn/2.x.md

@@ -47,6 +47,7 @@
 - [[#7662](https://github.com/apache/incubator-seata/pull/7662)] 确保 rm 的可见性，并且 MockTest 中的方法按顺序执行
 - [[#7683](https://github.com/apache/incubator-seata/pull/7683)] 重写 XABranchXid的equals和hashCode,解决mysql driver内存泄漏问题
 - [[#7643](https://github.com/apache/incubator-seata/pull/7643)] 修复 DM 事务回滚不使用数据库自动增量主键
+- [[#7749](https://github.com/apache/incubator-seata/pull/7749)] 修复 Http2HttpHandler 解析 application/x-www-form-urlencoded 请求失败的问题
 
 
 ### optimize:

core/src/main/java/org/apache/seata/core/protocol/detector/Http2Detector.java

@@ -33,6 +33,7 @@
 import org.apache.seata.core.rpc.netty.http.Http2HttpHandler;
 
 public class Http2Detector implements ProtocolDetector {
+    // HTTP/2 connection preface for detecting h2c (plaintext HTTP/2, not encrypted HTTPS)
     private static final byte[] HTTP2_PREFIX_BYTES = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(CharsetUtil.UTF_8);
     private final ChannelHandler[] serverHandlers;
 

core/src/main/java/org/apache/seata/core/rpc/netty/http/Http2HttpHandler.java

@@ -39,7 +39,10 @@
 import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.Method;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
 
 /**
  * The http2 http handler.
@@ -111,12 +114,28 @@ private void handleRequest(ChannelHandlerContext ctx) {
             if (request.getMethod() == HttpMethod.POST
                     && request.getBody() != null
                     && !request.getBody().isEmpty()) {
-                // assume body is json
+                CharSequence contentTypeSeq = request.getHeaders().get(HttpHeaderNames.CONTENT_TYPE);
+                String contentType = contentTypeSeq != null ? contentTypeSeq.toString() : "";
                 try {
-                    ObjectNode bodyDataNode = (ObjectNode) OBJECT_MAPPER.readTree(request.getBody());
-                    requestDataNode.set("body", bodyDataNode);
+                    if (contentType.contains("application/json")) {
+                        ObjectNode bodyDataNode = (ObjectNode) OBJECT_MAPPER.readTree(request.getBody());
+                        requestDataNode.set("body", bodyDataNode);
+                    } else if (contentType.contains("application/x-www-form-urlencoded")) {
+                        Map<String, String> formParams = new HashMap<>();
+                        String[] pairs = request.getBody().split("&");
+                        for (String pair : pairs) {
+                            String[] kv = pair.split("=", 2);
+                            if (kv.length == 2) {
+                                String key = URLDecoder.decode(kv[0], StandardCharsets.UTF_8.name());
+                                String value = URLDecoder.decode(kv[1], StandardCharsets.UTF_8.name());
+                                formParams.put(key, value);
+                            }
+                        }
+                        ObjectNode formDataNode = OBJECT_MAPPER.valueToTree(formParams);
+                        requestDataNode.set("body", formDataNode);
+                    }
                 } catch (Exception e) {
-                    LOGGER.warn("Failed to parse http2 body as json: {}", e.getMessage());
+                    LOGGER.warn("Failed to parse http2 body: {}", e.getMessage());
                 }
             }
             Object httpController = httpInvocation.getController();

core/src/main/java/org/apache/seata/core/rpc/netty/http/filter/HttpRequestParamWrapper.java

@@ -66,7 +66,8 @@ public HttpRequestParamWrapper(SimpleHttp2Request request) {
         parseQueryParams(request.getPath());
         parseHeaders(request.getHeaders());
 
-        String contentType = (String) request.getHeaders().get(HttpHeaderNames.CONTENT_TYPE);
+        CharSequence contentTypeSeq = request.getHeaders().get(HttpHeaderNames.CONTENT_TYPE);
+        String contentType = contentTypeSeq != null ? contentTypeSeq.toString() : null;
         if (contentType == null) {
             return;
         }

server/pom.xml

@@ -297,6 +297,12 @@
             <artifactId>bucket4j_jdk8-core</artifactId>
             <version>${bucket4j.version}</version>
         </dependency>
+
+        <dependency>
+            <groupId>com.squareup.okhttp3</groupId>
+            <artifactId>okhttp</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

server/src/test/java/org/apache/seata/server/controller/ClusterControllerTest.java

@@ -16,11 +16,14 @@
  */
 package org.apache.seata.server.controller;
 
+import okhttp3.Protocol;
+import okhttp3.Response;
 import org.apache.http.HttpStatus;
 import org.apache.http.StatusLine;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.entity.ContentType;
 import org.apache.http.protocol.HTTP;
+import org.apache.seata.common.executor.HttpCallback;
 import org.apache.seata.common.holder.ObjectHolder;
 import org.apache.seata.common.util.HttpClientUtil;
 import org.apache.seata.server.BaseSpringBootTest;
@@ -39,9 +42,14 @@
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import static org.apache.seata.common.ConfigurationKeys.SERVER_SERVICE_PORT_CAMEL;
 import static org.apache.seata.common.Constants.OBJECT_KEY_SPRING_APPLICATION_CONTEXT;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 class ClusterControllerTest extends BaseSpringBootTest {
@@ -76,6 +84,44 @@ void watchTimeoutTest() throws Exception {
 
     @Test
     @Order(2)
+    void watchTimeoutTest_withHttp2() throws Exception {
+        CountDownLatch latch = new CountDownLatch(1);
+
+        Map<String, String> headers = new HashMap<>();
+        headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
+
+        Map<String, String> params = new HashMap<>();
+        params.put("default-test", "1");
+
+        HttpCallback<Response> callback = new HttpCallback<Response>() {
+            @Override
+            public void onSuccess(Response response) {
+                Assertions.assertNotNull(response);
+                Assertions.assertEquals(Protocol.H2_PRIOR_KNOWLEDGE, response.protocol());
+                Assertions.assertEquals(HttpStatus.SC_NOT_MODIFIED, response.code());
+                latch.countDown();
+            }
+
+            @Override
+            public void onFailure(Throwable t) {
+                Assertions.fail("Should not fail");
+            }
+
+            @Override
+            public void onCancelled() {
+                Assertions.fail("Should not be cancelled");
+            }
+        };
+
+        HttpClientUtil.doPostWithHttp2(
+                "http://127.0.0.1:" + port + "/metadata/v1/watch?timeout=3000", params, headers, callback);
+        // Currently, the server side does not have the ability to send http2 responses,
+        // so if no response is received here, it will definitely time out
+        Assertions.assertFalse(latch.await(5, TimeUnit.SECONDS));
+    }
+
+    @Test
+    @Order(3)
     void watch() throws Exception {
         Map<String, String> header = new HashMap<>();
         header.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
@@ -106,7 +152,7 @@ public void run() {
     }
 
     @Test
-    @Order(3)
+    @Order(4)
     void testXssFilterBlocked_queryParam() throws Exception {
         String malicious = "<script>alert('xss')</script>";
         Map<String, String> header = new HashMap<>();
@@ -123,7 +169,118 @@ void testXssFilterBlocked_queryParam() throws Exception {
     }
 
     @Test
-    @Order(4)
+    @Order(5)
+    void testXssFilterBlocked_queryParam_withGetHttp2() throws Exception {
+        CountDownLatch latch = new CountDownLatch(1);
+
+        String malicious = "<script>alert('xss')</script>";
+        Map<String, String> header = new HashMap<>();
+        header.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
+
+        HttpCallback<Response> callback = new HttpCallback<Response>() {
+            @Override
+            public void onSuccess(Response response) {
+                assertNotNull(response);
+                Assertions.assertEquals(Protocol.H2_PRIOR_KNOWLEDGE, response.protocol());
+                Assertions.assertEquals(HttpStatus.SC_BAD_REQUEST, response.code());
+                latch.countDown();
+            }
+
+            @Override
+            public void onFailure(Throwable t) {
+                fail("Should not fail");
+            }
+
+            @Override
+            public void onCancelled() {
+                fail("Should not be cancelled");
+            }
+        };
+
+        HttpClientUtil.doGetWithHttp2(
+                "http://127.0.0.1:" + port + "/metadata/v1/watch?timeout=3000&testParam="
+                        + URLEncoder.encode(malicious, String.valueOf(StandardCharsets.UTF_8)),
+                header,
+                callback,
+                5000);
+
+        assertTrue(latch.await(10, TimeUnit.SECONDS));
+    }
+
+    @Test
+    @Order(6)
+    void testXssFilterBlocked_formParam_withPostHttp2() throws Exception {
+        CountDownLatch latch = new CountDownLatch(1);
+
+        String malicious = "<script>alert('xss')</script>";
+        Map<String, String> header = new HashMap<>();
+        header.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
+
+        Map<String, String> params = new HashMap<>();
+        params.put("key", malicious);
+
+        HttpCallback<Response> callback = new HttpCallback<Response>() {
+            @Override
+            public void onSuccess(Response response) {
+                assertNotNull(response);
+                Assertions.assertEquals(Protocol.H2_PRIOR_KNOWLEDGE, response.protocol());
+                Assertions.assertEquals(HttpStatus.SC_BAD_REQUEST, response.code());
+                latch.countDown();
+            }
+
+            @Override
+            public void onFailure(Throwable t) {
+                fail("Should not fail");
+            }
+
+            @Override
+            public void onCancelled() {
+                fail("Should not be cancelled");
+            }
+        };
+
+        HttpClientUtil.doPostWithHttp2("http://127.0.0.1:" + port + "/random", params, header, callback, 5000);
+
+        assertTrue(latch.await(10, TimeUnit.SECONDS));
+    }
+
+    @Test
+    @Order(7)
+    void testXssFilterBlocked_bodyParam_withPostHttp2() throws Exception {
+        CountDownLatch latch = new CountDownLatch(1);
+
+        String malicious = "<script>alert('xss')</script>";
+        Map<String, String> header = new HashMap<>();
+
+        String jsonBody = "{\"key\":\"" + malicious + "\"}";
+
+        HttpCallback<Response> callback = new HttpCallback<Response>() {
+            @Override
+            public void onSuccess(Response response) {
+                assertNotNull(response);
+                Assertions.assertEquals(Protocol.H2_PRIOR_KNOWLEDGE, response.protocol());
+                Assertions.assertEquals(HttpStatus.SC_BAD_REQUEST, response.code());
+                latch.countDown();
+            }
+
+            @Override
+            public void onFailure(Throwable t) {
+                fail("Should not fail");
+            }
+
+            @Override
+            public void onCancelled() {
+                fail("Should not be cancelled");
+            }
+        };
+
+        HttpClientUtil.doPostWithHttp2("http://127.0.0.1:" + port + "/random", jsonBody, header, callback, 5000);
+
+        assertTrue(latch.await(10, TimeUnit.SECONDS));
+    }
+
+    @Test
+    @Order(8)
     void testXssFilterBlocked_formParam() throws Exception {
         Map<String, String> headers = new HashMap<>();
         headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
@@ -139,7 +296,7 @@ void testXssFilterBlocked_formParam() throws Exception {
     }
 
     @Test
-    @Order(5)
+    @Order(9)
     void testXssFilterBlocked_jsonBody() throws Exception {
         Map<String, String> headers = new HashMap<>();
         headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType());
@@ -154,7 +311,7 @@ void testXssFilterBlocked_jsonBody() throws Exception {
     }
 
     @Test
-    @Order(6)
+    @Order(10)
     void testXssFilterBlocked_headerParam() throws Exception {
         Map<String, String> headers = new HashMap<>();
         headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());
@@ -171,7 +328,7 @@ void testXssFilterBlocked_headerParam() throws Exception {
     }
 
     @Test
-    @Order(7)
+    @Order(11)
     void testXssFilterBlocked_multiSource() throws Exception {
         Map<String, String> headers = new HashMap<>();
         headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType());
@@ -191,7 +348,7 @@ void testXssFilterBlocked_multiSource() throws Exception {
     }
 
     @Test
-    @Order(8)
+    @Order(12)
     void testXssFilterBlocked_formParamWithUserCustomKeyWords() throws Exception {
         Map<String, String> headers = new HashMap<>();
         headers.put(HTTP.CONTENT_TYPE, ContentType.APPLICATION_FORM_URLENCODED.getMimeType());