This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 6f142b09878 branch-4.1: [fix](fe) Add SSRF checks to HTTP TVF metadata
requests (#69742)
6f142b09878 is described below
commit 6f142b0987811a7f41abb6cfa8d3c494a18a48e9
Author: Calvin Kirs <[email protected]>
AuthorDate: Thu Oct 29 09:42:26 2026 +0800
branch-4.1: [fix](fe) Add SSRF checks to HTTP TVF metadata requests (#69742)
https://github.com/apache/doris/pull/69455
---
.../tablefunction/HttpTableValuedFunction.java | 17 ++-
.../tablefunction/HttpTableValuedFunctionTest.java | 133 +++++++++++++++++++++
2 files changed, 146 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 d8311c822a6..38640b3d96b 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.property.storage.HttpProperties;
import org.apache.doris.datasource.property.storage.StorageProperties;
@@ -61,8 +62,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,
this.httpProperties.getHeaders()), true));
+ this.fileStatuses.add(getHttpFileStatus(this.uri));
} else if (this.uri.startsWith("hf://")) {
List<String> fileUrls = HFUtils.expandGlob(this.uri);
if (LOG.isDebugEnabled()) {
@@ -71,14 +71,23 @@ public class HttpTableValuedFunction extends
ExternalFileTableValuedFunction {
}
}
for (String fileUrl : fileUrls) {
- this.fileStatuses.add(new TBrokerFileStatus(fileUrl, false,
- HttpUtils.getHttpFileSize(fileUrl,
this.httpProperties.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,
this.httpProperties.getHeaders()), true);
+ } finally {
+ SecurityChecker.getInstance().stopSSRFChecking();
+ }
+ }
+
@Override
public TFileType getTFileType() {
return TFileType.FILE_HTTP;
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..d146a10478d
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/tablefunction/HttpTableValuedFunctionTest.java
@@ -0,0 +1,133 @@
+// 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 com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+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;
+
+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(ImmutableMap.of(
+ "uri", uri, "format", "csv", "http.header.Authorization",
"Bearer token"));
+
+ Assertions.assertEquals(ImmutableList.of("start:" + uri, "head:" +
uri, "stop"), events);
+ Assertions.assertEquals(ImmutableList.of(new TBrokerFileStatus(uri,
false, 123L, true)), tvf.getFileStatuses());
+ http.verify(() -> HttpUtils.getHttpFileSize(uri,
ImmutableMap.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(ImmutableMap.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,
ImmutableMap.of())).thenThrow(failure);
+
+ AnalysisException exception =
Assertions.assertThrows(AnalysisException.class,
+ () -> new HttpTableValuedFunction(ImmutableMap.of("uri", uri,
"format", "csv")));
+
+ Assertions.assertSame(failure, exception.getCause());
+ Assertions.assertEquals(ImmutableList.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(ImmutableList.of(first, second));
+
+ HttpTableValuedFunction tvf = new
HttpTableValuedFunction(ImmutableMap.of("uri", uri, "format", "csv"));
+
+ Assertions.assertEquals(ImmutableList.of("start:" + first, "head:"
+ first, "stop",
+ "start:" + second, "head:" + second, "stop"), events);
+ Assertions.assertEquals(ImmutableList.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]
