github-actions[bot] commented on code in PR #65212:
URL: https://github.com/apache/doris/pull/65212#discussion_r3521349380
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/LoadAction.java:
##########
@@ -854,6 +855,9 @@ private TNetworkAddress
handleStreamLoadRedirect(HttpServletRequest request, boo
if (!Config.isCloudMode() || !groupCommit ||
!Config.enable_group_commit_streamload_be_forward) {
return selectRedirectBackend(request, groupCommit, tableId);
}
Review Comment:
This makes cloud group-commit stream load fail whenever HTTP TLS is active
and BE-forward is enabled. The unsupported part is the `_stream_load_forward`
BE-to-BE hop, which uses a plain libevent HTTP connection, but the code already
has a non-forwarding path (`selectRedirectBackend(request, groupCommit,
tableId)`) used whenever the optimization is disabled. Throwing here converts a
valid load into `RestBaseResult("group commit stream load BE forward is not
supported...")` instead of just skipping the optimization. Please fall back to
the normal redirect path in this branch so TLS does not break group-commit
loads.
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/HttpURLUtil.java:
##########
@@ -48,6 +50,26 @@ public static HttpURLConnection
getConnectionWithNodeIdent(String request) throw
}
}
+ public static HttpURLConnection getInternalConnectionWithNodeIdent(String
request,
+ InternalHttpClientProvider.Target target) throws IOException {
+ HttpURLConnection conn = getInternalConnection(request, target);
+ setNodeIdentHeaders(conn);
+ return conn;
+ }
+
+ public static HttpURLConnection getInternalConnection(String request,
+ InternalHttpClientProvider.Target target) throws IOException {
+ InternalHttpClientProvider provider =
InternalHttpClientProviderFactory.getProvider();
+ String normalizedRequest = provider.normalizeInternalUrl(request,
target);
+ try {
+ SecurityChecker.getInstance().startSSRFChecking(normalizedRequest);
Review Comment:
`getInternalConnection` now leaves the SSRF hook installed after every
successful internal open. It starts checking and immediately returns the opened
connection, but `stopSSRFChecking()` only runs from the catch block; the old
`getConnectionWithNodeIdent` and the other `SecurityChecker` callers all clear
the hook in `finally`. Since this helper is now used by successful paths like
`SHOW CONFIG BACKEND`, `SHOW LOAD WARNINGS`, meta image fetches, and audit/load
submissions, a real `UrlSecurityChecker` can remain active on the FE worker
thread and affect unrelated later work. Please restore the same scoped
lifecycle here and stop the checker on the success path as well.
--
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]
