-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
Hotfix rest Chinese encoding #13617
Hotfix rest Chinese encoding #13617
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.2 #13617 +/- ##
==========================================
- Coverage 70.38% 70.37% -0.02%
==========================================
Files 1606 1606
Lines 69999 70036 +37
Branches 10098 10105 +7
==========================================
+ Hits 49272 49290 +18
- Misses 16103 16114 +11
- Partials 4624 4632 +8 ☔ View full report in Codecov by Sentry. |
@suncairong163 PTAL |
I'm wondering if anybody can review my code |
String decodedRequestURI = requestURI; | ||
try { | ||
decodedRequestURI = URLDecoder.decode(decodedRequestURI, "UTF-8"); | ||
} catch (UnsupportedEncodingException e) { | ||
// do nothing | ||
} |
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.
This is hardcoded, we can set utf-8 by default, but it must come from Accept-Charset
first.
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.
ok, I'll make a adjustment
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.
I've made an adjustment according to your opinon, please take a look again, take your time
As json standard format, non-ansi chars are not allowed, so is nessary to do this? |
Take an example of SpringMVC framework, Chinese character encoding problems are very common among Chinese developers, and it offers solution. I think dubbo should also offer such solution to avoid such problems, even though it's not common to pass Chinese character as parameter. But in my situation,I does need to do so. |
And many users including me, are not familiar with json standard format(sorry for that).So this bug can be a trap |
I know your case now.
refer: |
PATL, thanks |
...c/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/request/RequestFacade.java
Outdated
Show resolved
Hide resolved
|
* decode uri in RequestFacade * supplement the unit test * fix code style violations * remove duplicate codes * consider Accept-Charset as enc * run mvn spotless apply * add comment * take weight into consideration * fix code style violations according sonar * add DEFAULT_CHARSET --------- Co-authored-by: chenxinyuan1 <chenxinyuan1@xiaomi.com>
What is the purpose of the change
When utilizing Dubbo REST with the @QueryParam annotation on parameters, issues with Chinese character decoding may arise. Refer to the 13349 issue for a comprehensive and detailed description of the problem, including its cause, solution, and an explanation of why it works.
Brief changelog
RequestFacade.initParameters will decode the request uri with UTF-8 before parsing uri to parameters.
Verifying this change
Checklist