github-actions[bot] commented on code in PR #69182:
URL: https://github.com/apache/doris/pull/69182#discussion_r4291950055
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/TableQueryPlanAction.java:
##########
@@ -119,6 +119,10 @@ public Object query_plan(
try {
String postContent = HttpUtils.getBody(request);
+ if ("true".equals(request.getParameter("forward")) &&
checkForwardToMaster(request)) {
+ // Use POST internally so forwarding a GET request preserves
the SQL body.
+ return forwardToMaster(request, postContent, "POST");
Review Comment:
[P2] Preserve UTF-8 bytes when forwarding the SQL body
A valid UTF-8 JSON body with no Content-Type, or with text/plain without a
charset, works on the existing local path because HttpUtils.getBody always
decodes UTF-8 and this endpoint has no consumes restriction. This call passes
that decoded String to a standalone RestTemplate while retaining the original
headers; its default StringHttpMessageConverter uses ISO-8859-1 for those media
types. A VARCHAR predicate such as name = 'δΈ' therefore reaches the master as
name = '?', producing a plan for different SQL. Forward the original/UTF-8
bytes, or explicitly make the outgoing encoding match the receiver, and cover a
Unicode body without the explicit JSON/UTF-8 header used by the new test. The
charset behavior is defined in [Spring 6.2.19
StringHttpMessageConverter](https://github.com/spring-projects/spring-framework/blob/v6.2.19/spring-web/src/main/java/org/springframework/http/converter/StringHttpMessageConverter.java#L134-L148).
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/TableQueryPlanAction.java:
##########
@@ -119,6 +119,10 @@ public Object query_plan(
try {
String postContent = HttpUtils.getBody(request);
+ if ("true".equals(request.getParameter("forward")) &&
checkForwardToMaster(request)) {
+ // Use POST internally so forwarding a GET request preserves
the SQL body.
+ return forwardToMaster(request, postContent, "POST");
Review Comment:
[P2] Preserve the authenticated client identity when forwarding
The master calls executeCheckPassword again, and
BaseController.parseAuthInfo derives the account host from
request.getRemoteAddr(). For a valid account such as 'reader'@'10.10.1.%' with
SELECT, a client at 10.10.1.25 can plan locally or contact the master directly,
but forwarding through FE 10.20.1.11 makes the master reject the same
credentials because its peer is now the FE. Copying Authorization alone
therefore breaks this option for host-qualified accounts and can resolve a
different same-name account. Preserve the resolved user/client context through
trusted FE forwarding, or keep authentication on a direct client-to-master
request; add a second-FE host-qualified-account case rather than only the
root/echo-server check.
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/TableQueryPlanAction.java:
##########
@@ -119,6 +119,10 @@ public Object query_plan(
try {
String postContent = HttpUtils.getBody(request);
+ if ("true".equals(request.getParameter("forward")) &&
checkForwardToMaster(request)) {
+ // Use POST internally so forwarding a GET request preserves
the SQL body.
+ return forwardToMaster(request, postContent, "POST");
Review Comment:
[P2] Preserve already encoded resource paths on the master hop
The reused helper builds a String URL from request.getRequestURI() and
passes it to RestTemplate.exchange(String, ...), which applies URI encoding
again. For example, /api/test%44b/testTbl/_query_plan resolves the ordinary
database testDb locally, but forwarding sends
/api/test%2544b/testTbl/_query_plan, so the master sees the different name
test%44b and fails resource/privilege lookup. Escaped table names have the same
problem. Pass the completed, already encoded target as a URI (and preserve the
query octets) rather than treating it as an unencoded URL template; add an
encoded-path case beyond the ASCII echo test. The relevant default encoding is
set by [Spring 6.2.19
RestTemplate](https://github.com/spring-projects/spring-framework/blob/v6.2.19/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java#L254-L257).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
