This is an automated email from the ASF dual-hosted git repository.
CalvinKirs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 961fe301f7b [fix](fe) Add SSRF checks to HTTP TVF metadata requests
(#69455)
961fe301f7b is described below
commit 961fe301f7b85d9259d48337a3154c02aec605b1
Author: Calvin Kirs <[email protected]>
AuthorDate: Mon Oct 26 23:23:16 2026 +0800
[fix](fe) Add SSRF checks to HTTP TVF metadata requests (#69455)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
HTTP TVF fetches file sizes through FE HEAD requests without invoking
the configured `SecurityChecker`. Add a shared helper that starts the
existing SSRF check before HEAD and calls `stopSSRFChecking()` in
`finally`, including when starting the check or requesting metadata
fails. Apply it to both direct HTTP/HTTPS URLs and every HTTP URL
expanded from `hf://`, preserving headers and file statuses.
This change covers FE file-size HEAD requests. BE reads, Hugging Face
tree/list API requests, checker configuration, and the default no-op
checker remain unchanged. The existing `UrlSecurityChecker` stop-method
reflection arity mismatch is outside this patch and requires a separate
follow-up; the new tests verify the caller's hook sequencing, not the
external SDK's cleanup behavior.
### Release note
HTTP TVF FE file-size HEAD requests now invoke the configured SSRF
checker. A rejected check prevents the HEAD request from being sent.
---
.../tablefunction/HttpTableValuedFunction.java | 17 ++-
.../tablefunction/HttpTableValuedFunctionTest.java | 132 +++++++++++++++++++++
2 files changed, 145 insertions(+), 4 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/HttpTableValuedFunction.java
b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/HttpTableValuedFunction.java
index e20e0c9beb0..49f0d9aeba8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/HttpTableValuedFunction.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/HttpTableValuedFunction.java
@@ -19,6 +19,7 @@ package org.apache.doris.tablefunction;
import org.apache.doris.analysis.BrokerDesc;
import org.apache.doris.analysis.StorageBackend.StorageType;
+import org.apache.doris.cloud.security.SecurityChecker;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.datasource.storage.StorageAdapter;
import org.apache.doris.datasource.storage.StorageTypeId;
@@ -64,8 +65,7 @@ public class HttpTableValuedFunction extends
ExternalFileTableValuedFunction {
private void generateFileStatus() throws Exception {
this.fileStatuses.clear();
if (this.uri.startsWith("http://";) || this.uri.startsWith("https://";))
{
- this.fileStatuses.add(new TBrokerFileStatus(this.uri, false,
- HttpUtils.getHttpFileSize(this.uri, getHeaders()), true));
+ this.fileStatuses.add(getHttpFileStatus(this.uri));
} else if (this.uri.startsWith("hf://")) {
List<String> fileUrls = HFUtils.expandGlob(this.uri);
if (LOG.isDebugEnabled()) {
@@ -74,14 +74,23 @@ public class HttpTableValuedFunction extends
ExternalFileTableValuedFunction {
}
}
for (String fileUrl : fileUrls) {
- this.fileStatuses.add(new TBrokerFileStatus(fileUrl, false,
- HttpUtils.getHttpFileSize(fileUrl, getHeaders()),
true));
+ this.fileStatuses.add(getHttpFileStatus(fileUrl));
}
} else {
throw new AnalysisException("HttpTableValuedFunction uri is
invalid: " + this.uri);
}
}
+ private TBrokerFileStatus getHttpFileStatus(String fileUrl) throws
Exception {
+ try {
+ SecurityChecker.getInstance().startSSRFChecking(fileUrl);
+ return new TBrokerFileStatus(fileUrl, false,
+ HttpUtils.getHttpFileSize(fileUrl, getHeaders()), true);
+ } finally {
+ SecurityChecker.getInstance().stopSSRFChecking();
+ }
+ }
+
/** Legacy HttpProperties.getHeaders(): {@code http.header.}-prefixed
keys, prefix stripped. */
private Map<String, String> getHeaders() {
Map<String, String> headers = Maps.newHashMap();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/tablefunction/HttpTableValuedFunctionTest.java
b/fe/fe-core/src/test/java/org/apache/doris/tablefunction/HttpTableValuedFunctionTest.java
new file mode 100644
index 00000000000..18764d960db
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/tablefunction/HttpTableValuedFunctionTest.java
@@ -0,0 +1,132 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.tablefunction;
+
+import org.apache.doris.cloud.security.SecurityChecker;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.httpv2.rest.manager.HttpUtils;
+import org.apache.doris.thrift.TBrokerFileStatus;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+public class HttpTableValuedFunctionTest {
+ private SecurityChecker checker;
+ private MockedStatic<SecurityChecker> security;
+ private MockedStatic<HttpUtils> http;
+ private List<String> events;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+ checker = Mockito.mock(SecurityChecker.class);
+ security = Mockito.mockStatic(SecurityChecker.class);
+ security.when(SecurityChecker::getInstance).thenReturn(checker);
+ http = Mockito.mockStatic(HttpUtils.class);
+ events = new ArrayList<>();
+ Mockito.doAnswer(invocation -> {
+ events.add("start:" + invocation.getArgument(0));
+ return null;
+ }).when(checker).startSSRFChecking(Mockito.anyString());
+ Mockito.doAnswer(invocation -> {
+ events.add("stop");
+ return null;
+ }).when(checker).stopSSRFChecking();
+ }
+
+ @AfterEach
+ public void tearDown() {
+ http.close();
+ security.close();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"http://example.com/data.csv";,
"https://example.com/data.csv"})
+ public void testHeadRequestRunsInsideSecurityCheck(String uri) throws
Exception {
+ mockHeadRequests();
+ HttpTableValuedFunction tvf = new HttpTableValuedFunction(Map.of(
+ "uri", uri, "format", "csv", "http.header.Authorization",
"Bearer token"));
+
+ Assertions.assertEquals(List.of("start:" + uri, "head:" + uri,
"stop"), events);
+ Assertions.assertEquals(List.of(new TBrokerFileStatus(uri, false,
123L, true)), tvf.getFileStatuses());
+ http.verify(() -> HttpUtils.getHttpFileSize(uri,
Map.of("Authorization", "Bearer token")));
+ }
+
+ @Test
+ public void testRejectedUrlDoesNotSendHeadRequest() throws Exception {
+ String uri = "http://127.0.0.1/data.csv";;
+ IOException blocked = new IOException("URL blocked");
+ Mockito.doThrow(blocked).when(checker).startSSRFChecking(uri);
+
+ AnalysisException exception =
Assertions.assertThrows(AnalysisException.class,
+ () -> new HttpTableValuedFunction(Map.of("uri", uri, "format",
"csv")));
+
+ Assertions.assertSame(blocked, exception.getCause());
+ http.verifyNoInteractions();
+ Mockito.verify(checker).stopSSRFChecking();
+ }
+
+ @Test
+ public void testHeadFailureStopsSecurityCheck() {
+ String uri = "https://example.com/data.csv";;
+ IOException failure = new IOException("HEAD failed");
+ http.when(() -> HttpUtils.getHttpFileSize(uri,
Map.of())).thenThrow(failure);
+
+ AnalysisException exception =
Assertions.assertThrows(AnalysisException.class,
+ () -> new HttpTableValuedFunction(Map.of("uri", uri, "format",
"csv")));
+
+ Assertions.assertSame(failure, exception.getCause());
+ Assertions.assertEquals(List.of("start:" + uri, "stop"), events);
+ }
+
+ @Test
+ public void testHuggingFaceChecksEachExpandedHttpUrl() throws Exception {
+ mockHeadRequests();
+ String uri = "hf://datasets/user/repo/*.csv";
+ String first =
"https://huggingface.co/datasets/user/repo/resolve/main/a.csv";;
+ String second =
"https://huggingface.co/datasets/user/repo/resolve/main/b.csv";;
+ try (MockedStatic<HFUtils> hf = Mockito.mockStatic(HFUtils.class)) {
+ hf.when(() -> HFUtils.expandGlob(uri)).thenReturn(List.of(first,
second));
+
+ HttpTableValuedFunction tvf = new
HttpTableValuedFunction(Map.of("uri", uri, "format", "csv"));
+
+ Assertions.assertEquals(List.of("start:" + first, "head:" + first,
"stop",
+ "start:" + second, "head:" + second, "stop"), events);
+ Assertions.assertEquals(List.of(new TBrokerFileStatus(first,
false, 123L, true),
+ new TBrokerFileStatus(second, false, 123L, true)),
tvf.getFileStatuses());
+ }
+ }
+
+ private void mockHeadRequests() {
+ http.when(() -> HttpUtils.getHttpFileSize(Mockito.anyString(),
Mockito.anyMap()))
+ .thenAnswer(invocation -> {
+ events.add("head:" + invocation.getArgument(0));
+ return 123L;
+ });
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
