-
Notifications
You must be signed in to change notification settings - Fork 8.9k
【Don't merge . It will be split into multiple PRS】Enhance the support for http2 on both the client and server sides #7736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.x
Are you sure you want to change the base?
Conversation
|
@funky-eyes @YongGoose PTAL |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 2.x #7736 +/- ##
============================================
+ Coverage 63.64% 63.66% +0.02%
Complexity 990 990
============================================
Files 1324 1323 -1
Lines 50125 50155 +30
Branches 5920 5928 +8
============================================
+ Hits 31901 31932 +31
+ Misses 15386 15376 -10
- Partials 2838 2847 +9
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances HTTP/2 protocol support across both client and server components of the Seata system. The changes include unifying the HTTP client utility to support h2c (HTTP/2 cleartext) protocol, adding HTTP/2 response handling in the server-side watch logic, and fixing a parsing issue for form-urlencoded requests in the HTTP/2 handler.
Key Changes:
- Unified HTTP client utilities by consolidating
Http5ClientUtilfunctionality intoHttpClientUtilwith HTTP/2 support - Added HTTP/2 response handling in
ClusterWatcherManagerfor watch operations - Fixed
Http2HttpHandlerto properly parseapplication/x-www-form-urlencodedrequest bodies
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
common/src/main/java/org/apache/seata/common/util/HttpClientUtil.java |
Added HTTP/2 client methods using OkHttp with h2c protocol support |
common/src/main/java/org/apache/seata/common/util/Http5ClientUtil.java |
Removed (functionality merged into HttpClientUtil) |
core/src/main/java/org/apache/seata/core/rpc/netty/http/Http2HttpHandler.java |
Enhanced to parse both JSON and form-urlencoded request bodies |
server/src/main/java/org/apache/seata/server/cluster/manager/ClusterWatcherManager.java |
Added HTTP/2 response sending logic for watch operations |
server/src/test/java/org/apache/seata/server/controller/ClusterControllerTest.java |
Added HTTP/2-specific test cases for watch and XSS filter functionality |
common/pom.xml |
Changed okhttp dependency scope to provided |
| Multiple pom.xml files | Added okhttp dependencies where needed |
Comments suppressed due to low confidence (1)
server/src/main/java/org/apache/seata/server/cluster/manager/ClusterWatcherManager.java:1
- The HTTP/1.1 response handling duplicates
ctx.writeAndFlush(response)in both the if and else branches. Consider simplifying by removing the conditional logic since the only difference is setting the keep-alive header, which doesn't affect the flush operation.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
common/src/main/java/org/apache/seata/common/util/HttpClientUtil.java
Outdated
Show resolved
Hide resolved
| String[] pairs = request.getBody().split("&"); | ||
| for (String pair : pairs) { | ||
| String[] kv = pair.split("=", 2); | ||
| if (kv.length == 2) { | ||
| formParams.put(kv[0], kv[1]); | ||
| } | ||
| } |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Form parameter values are not URL-decoded. When parsing 'application/x-www-form-urlencoded' data, values must be decoded using URLDecoder.decode() to properly handle special characters and encoded spaces.
…il.java Co-authored-by: Copilot <[email protected]>
…il.java Co-authored-by: Copilot <[email protected]>
Ⅰ. Describe what this PR did
1.Enhanced and unified the HTTP client utility to support the
h2cprotocol.2.Added support for sending responses over HTTP/2 within the server-side
watchlogic.3.Fixed an issue in Http2HttpHandler where parsing requests with the
application/x-www-form-urlencodedcontent type could fail.Ⅱ. Does this pull request fix one issue?
fix #7732
fix #7712
fix #7685
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
It can be verified by observing the test cases of the integrated test case
ClusterControllerTestⅤ. Special notes for reviews