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 678a4c070ca [fix](jdbc) Harden JDBC driver URL validation and remove
file-upload HTTP API (#65987)
678a4c070ca is described below
commit 678a4c070ca2bfd83890af2e2af961395bb7b3ef
Author: Calvin Kirs <[email protected]>
AuthorDate: Mon Jul 27 10:03:50 2026 +0800
[fix](jdbc) Harden JDBC driver URL validation and remove file-upload HTTP
API (#65987)
### What problem does this PR solve?
Hardens JDBC driver loading, locks down security-sensitive configs, and
removes an
unnecessary file-upload attack surface on the FE.
#### Summary of changes
| Area | Change |
|---|---|
| `jdbc_driver_secure_path` matching | Component-based (structural)
instead of a raw string prefix. `file://` URLs are decoded once via
`URI.getPath()` and compared with `Path.startsWith`; `http(s)` URLs
compare scheme/host/port, **user-info and query**, plus a
component-based path prefix. Closes prefix-confusion, path-traversal
(literal **and** percent-encoded `%2e%2e`), and query/user-info
bypasses. `*` and empty still mean allow-all (unchanged). |
| Fail-closed parsing | A `driver_url` that cannot be parsed as a URI is
now rejected instead of being accepted. |
| Mandatory CREATE/ALTER rule | New
`JdbcDorisConnector.checkDriverUrlSecurityRule()`, invoked from the
connector's `validateProperties()` (run by `checkProperties()` on both
CREATE and ALTER, both `!isReplay`). It **cannot be turned off**:
rejects any `..` segment (decoded), and requires a scheme-less
`driver_url` to be a bare `[A-Za-z0-9._-]+.jar` name resolved under
`jdbc_drivers_dir`. Never runs during replay or at query time. |
| Config immutability | `jdbc_driver_url_white_list`,
`s3_load_endpoint_white_list`, `force_sqlserver_jdbc_encrypt_false` are
now settable only in `fe.conf` (non-mutable). |
| Removed endpoint | `UploadAction` (`/api/{ns}/{db}/{tbl}/upload`) and
its `LoadSubmitter` / `TmpFileMgr` helpers, plus the now-orphaned
`http_load_submitter_max_worker_threads` config. |
### Behavior changes
| # | Before | After | Impact / who is affected |
|---|---|---|---|
| 1 | `POST/PUT/GET/DELETE /api/{ns}/{db}/{tbl}/upload` uploaded a file
to FE and stream-loaded it | Endpoint **removed** (404) | Web-UI "Data
Import" page / scripts hitting this route. Stream Load, Broker Load,
Routine Load, INSERT, S3 Load, CopyInto are **unaffected**. |
| 2 | `driver_url` with `..`, encoded `%2e%2e`, or a scheme-less path
with directories was accepted at CREATE (and never re-checked on ALTER)
| Rejected at **CREATE and ALTER CATALOG** | Only new/altered catalogs.
Existing catalogs are **not** re-validated (replay and queries
unaffected). |
| 3 | `jdbc_driver_secure_path` matched by raw string prefix; parse
failures were let through | Structural matching; parse failures rejected
(fail-closed) | Driver URLs that only passed by a coincidental prefix,
via `..`, `%2e%2e`, or a differing query/user-info are now rejected.
Legitimate URLs are unaffected. `*` and empty both still mean allow-all,
so default deployments see no change. |
| 4 | `jdbc_driver_url_white_list`, `s3_load_endpoint_white_list`,
`force_sqlserver_jdbc_encrypt_false` were runtime-mutable | Non-mutable
(fe.conf only, restart to take effect) | Scripts using `ADMIN SET
FRONTEND CONFIG` for these now get a "not mutable" error. |
| 5 | `http_load_submitter_max_worker_threads` config existed | Removed
| Old `fe.conf` entries get the existing unknown-key warning. |
### Config changes
| Config | Before | After |
|---|---|---|
| `jdbc_driver_url_white_list` | `mutable = true` | fe.conf only
(non-mutable) |
| `s3_load_endpoint_white_list` | `mutable = true` | fe.conf only
(non-mutable) |
| `force_sqlserver_jdbc_encrypt_false` | `mutable = true` | fe.conf only
(non-mutable) |
| `jdbc_driver_secure_path` | raw string-prefix match | structural
(component-based) match; `*`/empty still allow-all |
| `http_load_submitter_max_worker_threads` | present | removed |
### Release note
Removed the `/api/{ns}/{db}/{tbl}/upload` file-upload endpoint. Hardened
JDBC
driver URL validation (structural `jdbc_driver_secure_path` matching
incl.
encoded-traversal and query/user-info handling, fail-closed parsing, and
a
mandatory CREATE/ALTER rule that forbids path traversal and requires a
plain jar
file name); `*` and empty `jdbc_driver_secure_path` still mean
allow-all.
`jdbc_driver_url_white_list`, `s3_load_endpoint_white_list` and
`force_sqlserver_jdbc_encrypt_false` are now settable only in `fe.conf`.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- See the "Behavior changes" table above. -->
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- The removed endpoint and the config/validation changes
should be documented. -->
---
.../main/java/org/apache/doris/common/Config.java | 24 +-
.../java/org/apache/doris/common/ConfigTest.java | 26 +-
.../connector/jdbc/JdbcConnectorProvider.java | 5 +
.../doris/connector/jdbc/JdbcDorisConnector.java | 63 ++++
.../jdbc/JdbcConnectorProviderValidateTest.java | 31 +-
.../jdbc/JdbcDriverUrlSecurityRuleTest.java | 92 ++++++
.../org/apache/doris/catalog/JdbcResource.java | 133 ++++++++-
.../org/apache/doris/httpv2/rest/UploadAction.java | 318 ---------------------
.../apache/doris/httpv2/util/LoadSubmitter.java | 178 ------------
.../org/apache/doris/httpv2/util/TmpFileMgr.java | 307 --------------------
.../org/apache/doris/catalog/JdbcResourceTest.java | 166 +++++++++++
11 files changed, 510 insertions(+), 833 deletions(-)
diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
index 655af570cfb..66dc4cc881f 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
@@ -159,7 +159,9 @@ public class Config extends ConfigBase {
@ConfField(description = {"The safe path of the JDBC driver. When creating
a JDBC Catalog, "
+ "you can configure multiple files or network paths that are
allowed to be used, "
+ "separated by semicolons. "
- + "The default is * to allow all; if set to empty, it also means
to allow all"})
+ + "The default is * to allow all; if set to empty, it also means
to allow all. "
+ + "When set to concrete paths, driver URLs are matched
structurally (component-based), "
+ + "so path traversal and prefix confusion are rejected."})
public static String jdbc_driver_secure_path = "*";
@ConfField(description = {"Functions that MySQL JDBC Catalog does not
support pushing down"})
@@ -172,7 +174,9 @@ public class Config extends ConfigBase {
+ "these variables, it just needs to accept them without
error."})
public static String[] mysql_compat_var_whitelist = {};
- @ConfField(mutable = true, masterOnly = true, description = {"Force
SQLServer Jdbc Catalog encrypt to false"})
+ @ConfField(description = {"Force SQLServer Jdbc Catalog encrypt to false. "
+ + "This is a security-sensitive switch (it disables SQLServer JDBC
transport encryption), "
+ + "so it can only be set in fe.conf and is not modifiable at
runtime via ADMIN SET FRONTEND CONFIG."})
public static boolean force_sqlserver_jdbc_encrypt_false = false;
@ConfField(mutable = true, masterOnly = true, description = {
@@ -2846,10 +2850,6 @@ public class Config extends ConfigBase {
"The maximum number of worker threads for the HTTP SQL
submitter."})
public static int http_sql_submitter_max_worker_threads = 2;
- @ConfField(mutable = false, masterOnly = false, description = {
- "The maximum number of worker threads for the HTTP upload
submitter."})
- public static int http_load_submitter_max_worker_threads = 2;
-
@ConfField(mutable = true, masterOnly = true, description = {
"The threshold of load labels' number. After this number is
exceeded, "
+ "the labels of the completed import jobs or tasks will
be deleted, "
@@ -3256,9 +3256,11 @@ public class Config extends ConfigBase {
@ConfField(mutable = true)
public static int mow_calculate_delete_bitmap_retry_times = 10;
- @ConfField(mutable = true, description = {
+ @ConfField(description = {
"The allowlist for S3 load endpoints. If it is empty, no allowlist
will be set. "
- + "For example: s3_load_endpoint_white_list=a,b,c"})
+ + "For example: s3_load_endpoint_white_list=a,b,c. "
+ + "This can only be set in fe.conf and takes effect after
a restart; "
+ + "it is intentionally not modifiable at runtime via ADMIN
SET FRONTEND CONFIG."})
public static String[] s3_load_endpoint_white_list = {};
@ConfField(mutable = true, description = {
@@ -3290,9 +3292,11 @@ public class Config extends ConfigBase {
".dfs.core.cloudapi.de"
};
- @ConfField(mutable = true, description = {
+ @ConfField(description = {
"The allowlist for JDBC driver URLs. If it is empty, no allowlist
will be set. "
- + "For example: jdbc_driver_url_white_list=a,b,c"})
+ + "For example: jdbc_driver_url_white_list=a,b,c. "
+ + "This can only be set in fe.conf and takes effect after
a restart; "
+ + "it is intentionally not modifiable at runtime via ADMIN
SET FRONTEND CONFIG."})
public static String[] jdbc_driver_url_white_list = {};
@ConfField(description = {"The maximum length of label in Stream Load is
limited."})
diff --git a/fe/fe-common/src/test/java/org/apache/doris/common/ConfigTest.java
b/fe/fe-common/src/test/java/org/apache/doris/common/ConfigTest.java
index d48d7ae8ed3..c59cb44c66d 100644
--- a/fe/fe-common/src/test/java/org/apache/doris/common/ConfigTest.java
+++ b/fe/fe-common/src/test/java/org/apache/doris/common/ConfigTest.java
@@ -94,8 +94,28 @@ public class ConfigTest {
@Test
public void testSetEmptyArray() throws ConfigException {
- ConfigBase.setMutableConfig("s3_load_endpoint_white_list", "a,b,c");
- ConfigBase.setMutableConfig("s3_load_endpoint_white_list", "");
- Assert.assertEquals("array length should be 0", 0,
Config.s3_load_endpoint_white_list.length);
+ ConfigBase.setMutableConfig("mysql_compat_var_whitelist", "a,b,c");
+ ConfigBase.setMutableConfig("mysql_compat_var_whitelist", "");
+ Assert.assertEquals("array length should be 0", 0,
Config.mysql_compat_var_whitelist.length);
+ }
+
+ // File-path and jdbc-driver security configs must only be settable in
fe.conf (ops), never at runtime
+ // via ADMIN SET FRONTEND CONFIG. setMutableConfig is exactly that runtime
entrypoint, so it must reject them.
+ @Test
+ public void testSecurityPathConfigsAreNotRuntimeMutable() {
+ String[] opsOnlyConfigs = {
+ "jdbc_driver_url_white_list",
+ "jdbc_drivers_dir",
+ "jdbc_driver_secure_path",
+ "tmp_dir",
+ "plugin_dir",
+ "s3_load_endpoint_white_list",
+ "force_sqlserver_jdbc_encrypt_false",
+ };
+ for (String key : opsOnlyConfigs) {
+ ConfigException e = Assert.assertThrows(key + " should not be
runtime-mutable",
+ ConfigException.class, () ->
ConfigBase.setMutableConfig(key, "x"));
+ Assert.assertTrue(e.getMessage().contains("is not mutable"));
+ }
}
}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
index 0f675c99d24..ac4fd613daa 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
@@ -58,6 +58,11 @@ public class JdbcConnectorProvider implements
ConnectorProvider {
}
}
+ // 1b. Mandatory, non-configurable driver_url security rule.
checkProperties() runs this on
+ // both CREATE and ALTER CATALOG (both !isReplay), so a malicious
driver_url cannot be
+ // introduced by either; metadata replay of existing catalogs is never
affected.
+ JdbcDorisConnector.checkDriverUrlSecurityRule(resolve(properties,
JdbcConnectorProperties.DRIVER_URL));
+
// 2. Reject deprecated lower_case_table_names
if
(properties.containsKey(JdbcConnectorProperties.LOWER_CASE_TABLE_NAMES)
|| properties.containsKey(
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
index 750a3bdd250..d4f8464e6df 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
@@ -38,12 +38,15 @@ import org.apache.thrift.TSerializer;
import java.io.File;
import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.regex.Pattern;
/**
* JDBC connector implementation. Manages the lifecycle of
@@ -125,11 +128,71 @@ public class JdbcDorisConnector implements Connector {
return scanPlanProvider;
}
+ // A scheme-less driver_url must be a plain jar file name: letters,
digits, dot, underscore, hyphen.
+ // This intentionally forbids any path separator, so it can never escape
jdbc_drivers_dir.
+ private static final Pattern SAFE_DRIVER_FILE_NAME =
Pattern.compile("^[A-Za-z0-9._-]+\\.jar$");
+
+ /**
+ * Mandatory, non-configurable driver_url security rule. It is invoked from
+ * {@link JdbcConnectorProvider#validateProperties} (and from {@link
#preCreateValidation}),
+ * i.e. from the engine's {@code checkProperties()} hook, which runs only
on the user-facing
+ * CREATE / ALTER CATALOG paths (both guarded by {@code !isReplay}).
Therefore the rule never
+ * runs during metadata/edit-log replay nor at query time, so existing
catalogs are unaffected
+ * and FE startup / follower replay can never be blocked by it.
+ *
+ * <p>The rule cannot be turned off:
+ * <ul>
+ * <li>any {@code ..} path-traversal segment is rejected, for {@code
file://} and {@code http(s)} alike;</li>
+ * <li>a scheme-less driver_url must be a bare jar file name matching
{@code [A-Za-z0-9._-]+.jar}
+ * (no directories, no special characters), which is then resolved
under {@code jdbc_drivers_dir}.</li>
+ * </ul>
+ * Whether a remote/absolute URL is allowed at all remains governed by the
fe.conf-only
+ * {@code jdbc_driver_secure_path} / {@code jdbc_driver_url_white_list}
configs; this rule only
+ * forbids traversal and enforces the bare-name charset.
+ *
+ * <p>Throws {@link IllegalArgumentException} so the engine wraps it into
a {@code DdlException}
+ * (and, on ALTER, triggers the property rollback).
+ */
+ public static void checkDriverUrlSecurityRule(String driverUrl) {
+ if (driverUrl == null || driverUrl.isEmpty()) {
+ return;
+ }
+ // Check traversal on the decoded path so percent-encoded segments
(e.g. %2e%2e) — which the
+ // driver-loading consumers decode — cannot slip a ".." past this rule.
+ String pathToCheck = driverUrl;
+ if (driverUrl.contains("://")) {
+ try {
+ String decoded = new URI(driverUrl).getPath();
+ if (decoded != null) {
+ pathToCheck = decoded;
+ }
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException("Invalid driver_url: " +
driverUrl);
+ }
+ }
+ String probe = pathToCheck.replace('\\', '/');
+ for (String segment : probe.split("/")) {
+ if ("..".equals(segment)) {
+ throw new IllegalArgumentException(
+ "Invalid driver_url: path traversal ('..') is not
allowed: " + driverUrl);
+ }
+ }
+ if (!driverUrl.contains("://")) {
+ if (!SAFE_DRIVER_FILE_NAME.matcher(driverUrl).matches()) {
+ throw new IllegalArgumentException(
+ "Invalid driver_url: a driver file name must match
[A-Za-z0-9._-]+.jar (got: "
+ + driverUrl + ")");
+ }
+ }
+ }
+
@Override
public void preCreateValidation(ConnectorValidationContext context) throws
Exception {
// 1. Validate/resolve JDBC driver — format, whitelist, secure_path,
file existence.
String driverUrl = properties.get(JdbcConnectorProperties.DRIVER_URL);
if (driverUrl != null && !driverUrl.isEmpty()) {
+ // Mandatory, non-configurable security rule, enforced on catalog
creation only.
+ checkDriverUrlSecurityRule(driverUrl);
context.validateAndResolveDriverPath(driverUrl);
// 2. Compute and verify checksum.
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcConnectorProviderValidateTest.java
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcConnectorProviderValidateTest.java
index 3f137720a7f..9e0261d4b80 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcConnectorProviderValidateTest.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcConnectorProviderValidateTest.java
@@ -33,7 +33,7 @@ public class JdbcConnectorProviderValidateTest {
private Map<String, String> validProps() {
Map<String, String> props = new HashMap<>();
props.put("jdbc_url", "jdbc:mysql://localhost:3306/db");
- props.put("driver_url", "/path/to/driver.jar");
+ props.put("driver_url", "mysql-connector-java-8.0.25.jar");
props.put("driver_class", "com.mysql.cj.jdbc.Driver");
return props;
}
@@ -77,11 +77,38 @@ public class JdbcConnectorProviderValidateTest {
public void testPrefixedPropertiesAccepted() {
Map<String, String> props = new HashMap<>();
props.put("jdbc.jdbc_url", "jdbc:mysql://localhost:3306/db");
- props.put("jdbc.driver_url", "/path/to/driver.jar");
+ props.put("jdbc.driver_url", "mysql-connector-java-8.0.25.jar");
props.put("jdbc.driver_class", "com.mysql.cj.jdbc.Driver");
Assertions.assertDoesNotThrow(() ->
provider.validateProperties(props));
}
+ @Test
+ public void testDriverUrlTraversalRejected() {
+ // The driver_url security rule is enforced via validateProperties
(the CREATE/ALTER hook).
+ Map<String, String> props = validProps();
+ props.put("driver_url",
"file:///opt/doris/plugins/jdbc_drivers/../../etc/evil.jar");
+ IllegalArgumentException ex = Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> provider.validateProperties(props));
+ Assertions.assertTrue(ex.getMessage().contains("traversal"));
+ }
+
+ @Test
+ public void testDriverUrlBareNameWithDirectoryRejected() {
+ Map<String, String> props = validProps();
+ props.put("driver_url", "sub/dir/driver.jar");
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> provider.validateProperties(props));
+ }
+
+ @Test
+ public void testDriverUrlNormalHttpsAccepted() {
+ Map<String, String> props = validProps();
+ props.put("driver_url",
"https://repo.example.com/jdbc/mysql-connector-j-8.4.0.jar";);
+ Assertions.assertDoesNotThrow(() ->
provider.validateProperties(props));
+ }
+
@Test
public void testInvalidBooleanProperty() {
Map<String, String> props = validProps();
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
new file mode 100644
index 00000000000..e5c75891a95
--- /dev/null
+++
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
@@ -0,0 +1,92 @@
+// 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.connector.jdbc;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for the mandatory, non-configurable create-time driver_url security
rule
+ * in {@link JdbcDorisConnector#checkDriverUrlSecurityRule(String)}.
+ */
+public class JdbcDriverUrlSecurityRuleTest {
+
+ // ---- rejected ----
+
+ @Test
+ public void testBareNameTraversalRejected() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("../evil.jar"));
+ }
+
+ @Test
+ public void testBareNameWithDirectoryRejected() {
+ // A scheme-less driver_url must be a plain file name; any '/' fails
the charset check.
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("sub/dir/driver.jar"));
+ }
+
+ @Test
+ public void testBareNameSpecialCharsRejected() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("driver.jar; rm -rf /"));
+ }
+
+ @Test
+ public void testFileUrlTraversalRejected() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcDorisConnector.checkDriverUrlSecurityRule(
+
"file:///opt/doris/plugins/jdbc_drivers/../../etc/evil.jar"));
+ }
+
+ @Test
+ public void testHttpUrlTraversalRejected() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("http://host/a/../b.jar";));
+ }
+
+ @Test
+ public void testEncodedTraversalRejected() {
+ // %2e%2e decodes to "..", which must be caught on the decoded path.
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcDorisConnector.checkDriverUrlSecurityRule(
+
"file:///opt/doris/plugins/jdbc_drivers/%2e%2e/%2e%2e/etc/evil.jar"));
+ }
+
+ // ---- allowed ----
+
+ @Test
+ public void testPlainJarNameAllowed() {
+ Assertions.assertDoesNotThrow(
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("mysql-connector-j-8.4.0.jar"));
+ Assertions.assertDoesNotThrow(
+ () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("postgresql-42.5.0.jar"));
+ }
+
+ @Test
+ public void testNormalHttpsUrlAllowed() {
+ Assertions.assertDoesNotThrow(() ->
JdbcDorisConnector.checkDriverUrlSecurityRule(
+
"https://bucket.s3.amazonaws.com/regression/jdbc_driver/mysql-connector-j-8.4.0.jar";));
+ }
+
+ @Test
+ public void testNormalFileUrlAllowed() {
+ Assertions.assertDoesNotThrow(() ->
JdbcDorisConnector.checkDriverUrlSecurityRule(
+
"file:///opt/doris/plugins/jdbc_drivers/mysql-connector-j-8.4.0.jar"));
+ }
+}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
index f1e312faebe..0141d33d276 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
@@ -46,12 +46,15 @@ import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
/**
* External JDBC Catalog resource for external table query.
@@ -304,28 +307,128 @@ public class JdbcResource extends Resource {
+ "file://xxx.jar, http://xxx.jar, https://xxx.jar, or
xxx.jar (without prefix).");
}
+ URI uri;
try {
- URI uri = new URI(driverUrl);
- String schema = uri.getScheme();
- checkCloudWhiteList(driverUrl);
- if (schema == null && !driverUrl.startsWith("/")) {
- return checkAndReturnDefaultDriverUrl(driverUrl);
- }
+ uri = new URI(driverUrl);
+ } catch (URISyntaxException e) {
+ // Fail closed: an unparsable URL must never be silently accepted,
otherwise the
+ // allowed-path check below could be bypassed by a malformed URL.
+ LOG.warn("invalid jdbc driver url: {}", driverUrl, e);
+ throw new IllegalArgumentException("Invalid driver URL: " +
driverUrl);
+ }
+
+ String schema = uri.getScheme();
+ checkCloudWhiteList(driverUrl);
+ if (schema == null && !driverUrl.startsWith("/")) {
+ // A scheme-less driver_url is a plain jar file name resolved
under jdbc_drivers_dir. This
+ // shared resolver is also on the lazy load path of pre-existing
catalogs (Iceberg/Paimon/
+ // legacy JDBC consumers call it directly, with no create/alter or
replay context), so it
+ // deliberately applies no new restriction here: an unmodified
historical catalog must keep
+ // resolving exactly as before. The mandatory bare-name grammar is
enforced only when a
+ // catalog is created or altered, in
JdbcDorisConnector.checkDriverUrlSecurityRule.
+ return checkAndReturnDefaultDriverUrl(driverUrl);
+ }
+
+ // "*" or an empty/blank value means allow all (the documented,
backward-compatible contract).
+ String securePath = Config.jdbc_driver_secure_path;
+ if (securePath == null || securePath.trim().isEmpty() ||
"*".equals(securePath.trim())) {
+ return driverUrl;
+ }
- if ("*".equals(Config.jdbc_driver_secure_path)) {
- return driverUrl;
+ if (!isDriverUrlAllowed(driverUrl, uri)) {
+ throw new IllegalArgumentException("Driver URL does not match any
allowed paths: " + driverUrl);
+ }
+ return driverUrl;
+ }
+
+ /**
+ * Check whether {@code driverUrl} falls under one of the
semicolon-separated prefixes configured in
+ * {@link Config#jdbc_driver_secure_path}. Matching is structural
(component-based) rather than a raw string
+ * prefix, so that neither prefix confusion ({@code /opt/drivers} vs
{@code /opt/drivers-evil}) nor path
+ * traversal ({@code /opt/drivers/../etc}) can slip a driver outside the
allowed location.
+ */
+ private static boolean isDriverUrlAllowed(String driverUrl, URI uri) {
+ String scheme = uri.getScheme();
+ List<String> allowedPaths = new ArrayList<>();
+ for (String p : Config.jdbc_driver_secure_path.split(";")) {
+ String trimmed = p.trim();
+ if (!trimmed.isEmpty()) {
+ allowedPaths.add(trimmed);
}
+ }
+ if ("http".equalsIgnoreCase(scheme) ||
"https".equalsIgnoreCase(scheme)) {
+ URI candidate = uri.normalize();
+ return allowedPaths.stream().anyMatch(allowed ->
remoteUrlMatches(candidate, allowed));
+ }
+ // Only file:// reaches here; bare absolute paths and bare "*.jar" are
handled earlier.
+ // A local file URL must carry no authority, query or fragment.
Otherwise validation (which
+ // looks only at URI.getPath()) and the consumers (URLClassLoader /
checksum, which act on the
+ // whole original URL) would address different objects — e.g.
"file://attacker/dir/x.jar" is
+ // fetched from a remote authority, and "file:///dir/x.jar?evil" maps
to a sibling file.
+ String authority = uri.getRawAuthority();
+ if ((authority != null && !authority.isEmpty())
+ || uri.getRawQuery() != null || uri.getRawFragment() != null) {
+ return false;
+ }
+ Path candidate = toLocalPath(driverUrl).normalize();
+ return allowedPaths.stream()
+ .map(allowed -> toLocalPath(allowed).normalize())
+ .anyMatch(candidate::startsWith);
+ }
- boolean isAllowed =
Arrays.stream(Config.jdbc_driver_secure_path.split(";"))
- .anyMatch(allowedPath ->
driverUrl.startsWith(allowedPath.trim()));
- if (!isAllowed) {
- throw new IllegalArgumentException("Driver URL does not match
any allowed paths: " + driverUrl);
+ /**
+ * Turn a {@code file://} URL or a plain filesystem path into a {@link
Path} for structural comparison.
+ * A {@code file://} URL is decoded exactly once via {@link URI#getPath()}
so that percent-encoded
+ * segments (e.g. {@code %2e%2e}) are resolved into the same
representation the driver-loading
+ * consumers ({@code URL.openStream} / {@code URLClassLoader}) will use;
otherwise an encoded parent
+ * segment would survive normalization and escape the allowed directory.
+ */
+ private static Path toLocalPath(String pathOrUrl) {
+ if (pathOrUrl.startsWith("file:")) {
+ try {
+ String decoded = new URI(pathOrUrl).getPath();
+ if (decoded != null && !decoded.isEmpty()) {
+ return Paths.get(decoded);
+ }
+ } catch (URISyntaxException ignored) {
+ // fall through to literal stripping below
}
- return driverUrl;
+ int sep = pathOrUrl.indexOf("//");
+ return Paths.get(sep >= 0 ? pathOrUrl.substring(sep + 2) :
pathOrUrl.substring("file:".length()));
+ }
+ return Paths.get(pathOrUrl);
+ }
+
+ /**
+ * Structural match for remote (http/https) driver URLs: scheme, host and
port must be equal, and the
+ * candidate path must sit under the allowed path (component-based). A
bare path prefix (no scheme) can
+ * never authorize a remote URL.
+ */
+ private static boolean remoteUrlMatches(URI candidate, String allowedPath)
{
+ URI base;
+ try {
+ base = new URI(allowedPath).normalize();
} catch (URISyntaxException e) {
- LOG.warn("invalid jdbc driver url: " + driverUrl);
- return driverUrl;
+ return false;
}
+ if (base.getScheme() == null) {
+ return false;
+ }
+ // Scheme/host/port and the path prefix must match, and the
resource-selecting components
+ // (user-info and query) that the checksum/classloader consumers act
on must match exactly too,
+ // otherwise e.g. ".../download?id=approved" would authorize
".../download?id=evil".
+ return base.getScheme().equalsIgnoreCase(candidate.getScheme())
+ && base.getHost() != null &&
base.getHost().equalsIgnoreCase(candidate.getHost())
+ && base.getPort() == candidate.getPort()
+ && Objects.equals(base.getUserInfo(), candidate.getUserInfo())
+ && Objects.equals(base.getRawQuery(), candidate.getRawQuery())
+ && pathIsUnder(candidate.getPath(), base.getPath());
+ }
+
+ private static boolean pathIsUnder(String candidatePath, String basePath) {
+ Path candidate = Paths.get(candidatePath == null ||
candidatePath.isEmpty() ? "/" : candidatePath).normalize();
+ Path base = Paths.get(basePath == null || basePath.isEmpty() ? "/" :
basePath).normalize();
+ return candidate.startsWith(base);
}
private static String checkAndReturnDefaultDriverUrl(String driverUrl) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/UploadAction.java
b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/UploadAction.java
deleted file mode 100644
index fa00e4a5ab3..00000000000
--- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/UploadAction.java
+++ /dev/null
@@ -1,318 +0,0 @@
-// 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.httpv2.rest;
-
-import org.apache.doris.common.Config;
-import org.apache.doris.httpv2.entity.ResponseEntityBuilder;
-import org.apache.doris.httpv2.util.LoadSubmitter;
-import org.apache.doris.httpv2.util.TmpFileMgr;
-import org.apache.doris.mysql.privilege.PrivPredicate;
-import org.apache.doris.qe.ConnectContext;
-import org.apache.doris.system.SystemInfoService;
-
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.springframework.web.bind.annotation.PathVariable;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestMethod;
-import org.springframework.web.bind.annotation.RequestParam;
-import org.springframework.web.bind.annotation.RestController;
-import org.springframework.web.multipart.MultipartFile;
-
-import java.io.IOException;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-
-/**
- * Upload file
- */
-@RestController
-public class UploadAction extends RestBaseController {
- private static final Logger LOG = LogManager.getLogger(UploadAction.class);
- private static TmpFileMgr fileMgr = new TmpFileMgr(Config.tmp_dir);
- private static LoadSubmitter loadSubmitter = new LoadSubmitter();
-
- private static final String PARAM_COLUMN_SEPARATOR = "column_separator";
- private static final String PARAM_PREVIEW = "preview";
- private static final String PARAM_FILE_ID = "file_id";
- private static final String PARAM_FILE_UUID = "file_uuid";
-
- /**
- * Upload the file
- * @param ns
- * @param dbName
- * @param tblName
- * @param file
- * @param request
- * @param response
- * @return
- */
- @RequestMapping(path = "/api/{" + NS_KEY + "}/{" + DB_KEY + "}/{" +
TABLE_KEY + "}/upload",
- method = {RequestMethod.POST})
- public Object upload(
- @PathVariable(value = NS_KEY) String ns,
- @PathVariable(value = DB_KEY) String dbName,
- @PathVariable(value = TABLE_KEY) String tblName,
- @RequestParam("file") MultipartFile file,
- HttpServletRequest request, HttpServletResponse response) {
- if (needRedirect(request.getScheme())) {
- return redirectToHttps(request);
- }
-
- checkWithCookie(request, response, false);
-
- if (!ns.equalsIgnoreCase(SystemInfoService.DEFAULT_CLUSTER)) {
- return ResponseEntityBuilder.badRequest("Only support
'default_cluster' now");
- }
-
- checkTblAuth(ConnectContext.get().getCurrentUserIdentity(), dbName,
tblName, PrivPredicate.LOAD);
-
- String columnSeparator = request.getParameter(PARAM_COLUMN_SEPARATOR);
- if (Strings.isNullOrEmpty(columnSeparator)) {
- columnSeparator = "\t";
- }
-
- String preview = request.getParameter(PARAM_PREVIEW);
- if (Strings.isNullOrEmpty(preview)) {
- preview = "false"; // default is false
- }
-
- if (file.isEmpty()) {
- return ResponseEntityBuilder.badRequest("Empty file");
- }
-
- try {
- TmpFileMgr.TmpFile tmpFile = fileMgr.upload(new
TmpFileMgr.UploadFile(file, columnSeparator));
- TmpFileMgr.TmpFile copiedFile = tmpFile.copy();
- if (preview.equalsIgnoreCase("true")) {
- copiedFile.setPreview();
- }
- return ResponseEntityBuilder.ok(copiedFile);
- } catch (TmpFileMgr.TmpFileException | IOException e) {
- return ResponseEntityBuilder.okWithCommonError(e.getMessage());
- }
- }
-
- /**
- * Load the uploaded file
- * @param ns
- * @param dbName
- * @param tblName
- * @param request
- * @param response
- * @return
- */
- @RequestMapping(path = "/api/{" + NS_KEY + "}/{" + DB_KEY + "}/{" +
TABLE_KEY + "}/upload",
- method = {RequestMethod.PUT})
- public Object submit(
- @PathVariable(value = NS_KEY) String ns,
- @PathVariable(value = DB_KEY) String dbName,
- @PathVariable(value = TABLE_KEY) String tblName,
- HttpServletRequest request, HttpServletResponse response) {
-
- // This is a strict restriction
- if (!Strings.isNullOrEmpty(Config.security_checker_class_name)) {
- return ResponseEntityBuilder.badRequest("Not support upload data
api in security env");
- }
-
- ActionAuthorizationInfo authInfo = checkWithCookie(request, response,
false);
-
- if (!ns.equalsIgnoreCase(SystemInfoService.DEFAULT_CLUSTER)) {
- return ResponseEntityBuilder.badRequest("Only support
'default_cluster' now");
- }
-
- checkTblAuth(ConnectContext.get().getCurrentUserIdentity(), dbName,
tblName, PrivPredicate.LOAD);
-
- String fileIdStr = request.getParameter(PARAM_FILE_ID);
- if (Strings.isNullOrEmpty(fileIdStr)) {
- return ResponseEntityBuilder.badRequest("Missing file id
parameter");
- }
- String fileUUIDStr = request.getParameter(PARAM_FILE_UUID);
- if (Strings.isNullOrEmpty(fileUUIDStr)) {
- return ResponseEntityBuilder.badRequest("Missing file id
parameter");
- }
-
- TmpFileMgr.TmpFile tmpFile = null;
- try {
- tmpFile = fileMgr.getFile(Long.valueOf(fileIdStr), fileUUIDStr);
- } catch (TmpFileMgr.TmpFileException e) {
- return ResponseEntityBuilder.okWithCommonError("file not found");
- }
- Preconditions.checkNotNull(tmpFile, fileIdStr);
-
- LoadContext loadContext = new LoadContext(request, dbName, tblName,
- authInfo.fullUserName, authInfo.password, tmpFile);
- Future<LoadSubmitter.SubmitResult> future =
loadSubmitter.submit(loadContext);
-
- try {
- LoadSubmitter.SubmitResult res = future.get();
- return ResponseEntityBuilder.ok(res);
- } catch (InterruptedException | ExecutionException e) {
- return ResponseEntityBuilder.okWithCommonError(e.getMessage());
- }
- }
-
- /**
- * Get all uploaded file or specified file
- * If preview is true, also return the preview of the file
- * @param ns
- * @param dbName
- * @param tblName
- * @param request
- * @param response
- * @return
- */
- @RequestMapping(path = "/api/{" + NS_KEY + "}/{" + DB_KEY + "}/{" +
TABLE_KEY + "}/upload",
- method = {RequestMethod.GET})
- public Object list(
- @PathVariable(value = NS_KEY) String ns,
- @PathVariable(value = DB_KEY) String dbName,
- @PathVariable(value = TABLE_KEY) String tblName,
- HttpServletRequest request, HttpServletResponse response) {
-
- checkWithCookie(request, response, false);
-
- if (!ns.equalsIgnoreCase(SystemInfoService.DEFAULT_CLUSTER)) {
- return ResponseEntityBuilder.badRequest("Only support
'default_cluster' now");
- }
-
- checkTblAuth(ConnectContext.get().getCurrentUserIdentity(), dbName,
tblName, PrivPredicate.LOAD);
-
- String fileIdStr = request.getParameter(PARAM_FILE_ID);
- String fileUUIDStr = request.getParameter(PARAM_FILE_UUID);
-
- if (Strings.isNullOrEmpty(fileIdStr) ||
Strings.isNullOrEmpty(fileUUIDStr)) {
- // not specified file id, return all files list
- List<TmpFileMgr.TmpFileBrief> files = fileMgr.listFiles();
- return ResponseEntityBuilder.ok(files);
- }
-
- // return specified file
- String preview = request.getParameter(PARAM_PREVIEW);
- if (Strings.isNullOrEmpty(preview)) {
- preview = "true"; // default is true
- }
-
- try {
- TmpFileMgr.TmpFile tmpFile =
fileMgr.getFile(Long.valueOf(fileIdStr), fileUUIDStr);
- TmpFileMgr.TmpFile copiedFile = tmpFile.copy();
- if (preview.equalsIgnoreCase("true")) {
- copiedFile.setPreview();
- }
- return ResponseEntityBuilder.ok(copiedFile);
- } catch (TmpFileMgr.TmpFileException | IOException e) {
- return ResponseEntityBuilder.okWithCommonError(e.getMessage());
- }
- }
-
- @RequestMapping(path = "/api/{" + NS_KEY + "}/{" + DB_KEY + "}/{" +
TABLE_KEY + "}/upload",
- method = {RequestMethod.DELETE})
- public Object delete(
- @PathVariable(value = NS_KEY) String ns,
- @PathVariable(value = DB_KEY) String dbName,
- @PathVariable(value = TABLE_KEY) String tblName,
- HttpServletRequest request, HttpServletResponse response) {
-
- checkWithCookie(request, response, false);
-
- if (!ns.equalsIgnoreCase(SystemInfoService.DEFAULT_CLUSTER)) {
- return ResponseEntityBuilder.badRequest("Only support
'default_cluster' now");
- }
-
- checkTblAuth(ConnectContext.get().getCurrentUserIdentity(), dbName,
tblName, PrivPredicate.LOAD);
-
- String fileIdStr = request.getParameter(PARAM_FILE_ID);
- if (Strings.isNullOrEmpty(fileIdStr)) {
- return ResponseEntityBuilder.badRequest("Missing file id
parameter");
- }
- String fileUUIDStr = request.getParameter(PARAM_FILE_UUID);
- if (Strings.isNullOrEmpty(fileUUIDStr)) {
- return ResponseEntityBuilder.badRequest("Missing file id
parameter");
- }
-
- fileMgr.deleteFile(Long.valueOf(fileIdStr), fileUUIDStr);
- return ResponseEntityBuilder.ok();
- }
-
- /**
- * A context to save infos of stream load
- */
- public static class LoadContext {
- public String user;
- public String passwd;
- public String db;
- public String tbl;
- public TmpFileMgr.TmpFile file;
-
- public String label;
- public String columnSeparator;
- public String columns;
- public String where;
- public String maxFilterRatio;
- public String partitions;
- public String timeout;
- public String strictMode;
- public String timezone;
- public String execMemLimit;
- public String format;
- public String jsonPaths;
- public String stripOuterArray;
- public String jsonRoot;
- public String numAsString;
- public String fuzzyParse;
-
-
- public LoadContext(HttpServletRequest request, String db,
- String tbl, String user, String passwd, TmpFileMgr.TmpFile
file) {
- this.db = db;
- this.tbl = tbl;
- this.user = user;
- this.passwd = passwd;
- this.file = file;
-
- parseHeader(request);
- }
-
- private void parseHeader(HttpServletRequest request) {
- this.label = request.getHeader("label");
- this.columnSeparator = file.columnSeparator;
- if (!Strings.isNullOrEmpty(request.getHeader("column_separator")))
{
- this.columnSeparator = request.getHeader("column_separator");
- }
- this.columns = request.getHeader("columns");
- this.where = request.getHeader("where");
- this.maxFilterRatio = request.getHeader("max_filter_ratio");
- this.partitions = request.getHeader("partitions");
- this.timeout = request.getHeader("timeout");
- this.strictMode = request.getHeader("strict_mode");
- this.timezone = request.getHeader("timezone");
- this.execMemLimit = request.getHeader("exec_mem_limit");
- this.format = request.getHeader("format");
- this.jsonPaths = request.getHeader("jsonpaths");
- this.stripOuterArray = request.getHeader("strip_outer_array");
- this.numAsString = request.getHeader("num_as_string");
- this.jsonRoot = request.getHeader("json_root");
- this.fuzzyParse = request.getHeader("fuzzy_parse");
- }
- }
-}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/LoadSubmitter.java
b/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/LoadSubmitter.java
deleted file mode 100644
index 0305199d9fe..00000000000
--- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/LoadSubmitter.java
+++ /dev/null
@@ -1,178 +0,0 @@
-// 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.httpv2.util;
-
-import org.apache.doris.catalog.Env;
-import org.apache.doris.common.Config;
-import org.apache.doris.common.LoadException;
-import org.apache.doris.common.ThreadPoolManager;
-import org.apache.doris.common.util.NetUtils;
-import org.apache.doris.httpv2.rest.UploadAction;
-import org.apache.doris.system.Backend;
-import org.apache.doris.system.BeSelectionPolicy;
-import org.apache.doris.system.SystemInfoService;
-
-import com.google.common.base.Strings;
-import com.google.gson.Gson;
-import com.google.gson.reflect.TypeToken;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
-import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.lang.reflect.Type;
-import java.net.HttpURLConnection;
-import java.net.URL;
-import java.nio.charset.StandardCharsets;
-import java.util.Base64;
-import java.util.List;
-import java.util.concurrent.Callable;
-import java.util.concurrent.Future;
-import java.util.concurrent.ThreadPoolExecutor;
-
-public class LoadSubmitter {
- private static final Logger LOG =
LogManager.getLogger(LoadSubmitter.class);
-
- private ThreadPoolExecutor executor =
ThreadPoolManager.newDaemonCacheThreadPoolThrowException(
- Config.http_load_submitter_max_worker_threads,
"load-submitter", true);
-
- private static final String STREAM_LOAD_URL_PATTERN =
"http://%s/api/%s/%s/_stream_load";;
-
- public Future<SubmitResult> submit(UploadAction.LoadContext loadContext) {
- LoadSubmitter.Worker worker = new LoadSubmitter.Worker(loadContext);
- return executor.submit(worker);
- }
-
- private static class Worker implements Callable<SubmitResult> {
-
- private UploadAction.LoadContext loadContext;
-
- public Worker(UploadAction.LoadContext loadContext) {
- this.loadContext = loadContext;
- }
-
- @Override
- public SubmitResult call() throws Exception {
- try {
- return load();
- } catch (Throwable e) {
- LOG.warn("failed to submit load. label: {}",
loadContext.label, e);
- throw e;
- }
- }
-
- private SubmitResult load() throws Exception {
- // choose a backend to submit the stream load
- Backend be = selectOneBackend();
-
- String hostPort =
NetUtils.getHostPortInAccessibleFormat(be.getHost(), be.getHttpPort());
- String loadUrlStr = String.format(STREAM_LOAD_URL_PATTERN,
hostPort, loadContext.db, loadContext.tbl);
- URL loadUrl = new URL(loadUrlStr);
- HttpURLConnection conn = (HttpURLConnection)
loadUrl.openConnection();
- conn.setRequestMethod("PUT");
- String auth = String.format("%s:%s", loadContext.user,
- loadContext.passwd);
- String authEncoding =
Base64.getEncoder().encodeToString(auth.getBytes(StandardCharsets.UTF_8));
- conn.setRequestProperty("Authorization", "Basic " + authEncoding);
- conn.addRequestProperty("Expect", "100-continue");
- conn.addRequestProperty("Content-Type", "text/plain;
charset=UTF-8");
- if (!Strings.isNullOrEmpty(loadContext.columns)) {
- conn.addRequestProperty("columns", loadContext.columns);
- }
- if (!Strings.isNullOrEmpty(loadContext.columnSeparator)) {
- conn.addRequestProperty("column_separator",
loadContext.columnSeparator);
- }
- if (!Strings.isNullOrEmpty(loadContext.label)) {
- conn.addRequestProperty("label", loadContext.label);
- }
- conn.setDoOutput(true);
- conn.setDoInput(true);
-
- File loadFile = checkAndGetFile(loadContext.file);
- try (BufferedOutputStream bos = new
BufferedOutputStream(conn.getOutputStream());
- BufferedInputStream bis = new BufferedInputStream(new
FileInputStream(loadFile));) {
- int i;
- while ((i = bis.read()) > 0) {
- bos.write(i);
- }
- }
-
- int status = conn.getResponseCode();
- String respMsg = conn.getResponseMessage();
-
- LOG.info("get status: {}, response msg: {}", status, respMsg);
-
- InputStream stream = (InputStream) conn.getContent();
- BufferedReader br = new BufferedReader(new
InputStreamReader(stream));
- StringBuilder sb = new StringBuilder();
- String line;
- while ((line = br.readLine()) != null) {
- sb.append(line);
- }
- Type type = new TypeToken<SubmitResult>() {
- }.getType();
- SubmitResult result = new Gson().fromJson(sb.toString(), type);
- return result;
- }
-
- private File checkAndGetFile(TmpFileMgr.TmpFile tmpFile) {
- File file = new File(tmpFile.absPath);
- return file;
- }
-
- private Backend selectOneBackend() throws LoadException {
- BeSelectionPolicy policy = new
BeSelectionPolicy.Builder().needLoadAvailable().build();
- List<Long> backendIds =
Env.getCurrentSystemInfo().selectBackendIdsByPolicy(policy, 1);
- if (backendIds.isEmpty()) {
- throw new
LoadException(SystemInfoService.NO_BACKEND_LOAD_AVAILABLE_MSG + ", policy: " +
policy);
- }
- Backend backend =
Env.getCurrentSystemInfo().getBackend(backendIds.get(0));
- if (backend == null) {
- throw new
LoadException(SystemInfoService.NO_BACKEND_LOAD_AVAILABLE_MSG + ", policy: " +
policy);
- }
- return backend;
- }
- }
-
- // CHECKSTYLE OFF: These name must match the name in json, case-sensitive.
- public static class SubmitResult {
- public String TxnId;
- public String Label;
- public String Status;
- public String ExistingJobStatus;
- public String Message;
- public String NumberTotalRows;
- public String NumberLoadedRows;
- public String NumberFilteredRows;
- public String NumberUnselectedRows;
- public String LoadBytes;
- public String LoadTimeMs;
- public String BeginTxnTimeMs;
- public String StreamLoadPutTimeMs;
- public String ReadDataTimeMs;
- public String WriteDataTimeMs;
- public String CommitAndPublishTimeMs;
- public String ErrorURL;
- }
- // CHECKSTYLE ON
-}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/TmpFileMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/TmpFileMgr.java
deleted file mode 100644
index dcdbc4bac63..00000000000
--- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/util/TmpFileMgr.java
+++ /dev/null
@@ -1,307 +0,0 @@
-// 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.httpv2.util;
-
-import org.apache.doris.common.util.Util;
-
-import com.google.common.base.Joiner;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.springframework.web.multipart.MultipartFile;
-
-import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileReader;
-import java.io.IOException;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.stream.Collectors;
-
-/**
- * Manager the file uploaded.
- * This file manager is currently only used to manage files
- * uploaded through the Upload RESTFul API.
- * And limit the number and size of the maximum upload file.
- * It can also browse or delete files through the RESTFul API.
- */
-public class TmpFileMgr {
- public static final Logger LOG = LogManager.getLogger(TmpFileMgr.class);
-
- private static final long MAX_TOTAL_FILE_SIZE_BYTES = 1 * 1024 * 1024 *
1024L; // 1GB
- private static final long MAX_TOTAL_FILE_NUM = 100;
- public static final long MAX_SINGLE_FILE_SIZE = 100 * 1024 * 1024L; //
100MB
- private static final String UPLOAD_DIR = "_doris_upload";
-
- private AtomicLong fileIdGenerator = new AtomicLong(0);
- private String rootDir;
- private Map<Long, TmpFile> fileMap = Maps.newConcurrentMap();
-
- private long totalFileSize = 0;
-
- public TmpFileMgr(String dir) {
- this.rootDir = dir + "/" + UPLOAD_DIR;
- init();
- }
-
- private void init() {
- File root = new File(rootDir);
- if (!root.exists()) {
- root.mkdirs();
- } else if (!root.isDirectory()) {
- throw new IllegalStateException("Path " + rootDir + " is not
directory");
- }
-
- // delete all files under this dir at startup.
- // This means that all uploaded files will be lost after FE restarts.
- // This is just for simplicity.
- Util.deleteDirectory(root);
- root.mkdirs();
- }
-
- /**
- * Simply used `synchronized` to allow only one user upload file at one
time.
- * So that we can easily control the number of files and total size of
files.
- *
- * @param uploadFile
- * @return
- * @throws TmpFileException
- */
- public synchronized TmpFile upload(UploadFile uploadFile) throws
TmpFileException {
- if (uploadFile.file.getSize() > MAX_SINGLE_FILE_SIZE) {
- throw new TmpFileException("File size " + uploadFile.file.getSize()
- + " exceed limit " + MAX_SINGLE_FILE_SIZE);
- }
-
- if (totalFileSize + uploadFile.file.getSize() >
MAX_TOTAL_FILE_SIZE_BYTES) {
- throw new TmpFileException("Total file size will exceed limit " +
MAX_TOTAL_FILE_SIZE_BYTES);
- }
-
- if (fileMap.size() > MAX_TOTAL_FILE_NUM) {
- throw new TmpFileException("Number of temp file " + fileMap.size()
+ " exceed limit " + MAX_TOTAL_FILE_NUM);
- }
-
- long fileId = fileIdGenerator.incrementAndGet();
- String fileUUID = UUID.randomUUID().toString();
-
- TmpFile tmpFile = new TmpFile(fileId, fileUUID,
uploadFile.file.getOriginalFilename(),
- uploadFile.file.getSize(), uploadFile.columnSeparator);
- try {
- tmpFile.save(uploadFile.file);
- } catch (IOException e) {
- throw new TmpFileException("Failed to upload file. Reason: " +
e.getMessage());
- }
- fileMap.put(tmpFile.id, tmpFile);
- totalFileSize += uploadFile.file.getSize();
- return tmpFile;
- }
-
- public TmpFile getFile(long id, String uuid) throws TmpFileException {
- TmpFile tmpFile = fileMap.get(id);
- if (tmpFile == null || !tmpFile.uuid.equals(uuid)) {
- throw new TmpFileException("File with [" + id + "-" + uuid + "]
does not exist");
- }
- return tmpFile;
- }
-
- public List<TmpFileBrief> listFiles() {
- return fileMap.values().stream().map(t -> new
TmpFileBrief(t)).collect(Collectors.toList());
- }
-
- /**
- * Delete the specified file and remove it from fileMap
- * @param fileId
- * @param fileUUID
- */
- public void deleteFile(Long fileId, String fileUUID) {
- Iterator<Map.Entry<Long, TmpFile>> iterator =
fileMap.entrySet().iterator();
- while (iterator.hasNext()) {
- Map.Entry<Long, TmpFile> entry = iterator.next();
- if (entry.getValue().id == fileId &&
entry.getValue().uuid.equals(fileUUID)) {
- entry.getValue().delete();
- iterator.remove();
- }
- }
- return;
- }
-
- public class TmpFile {
- public final long id;
- public final String uuid;
- public final String originFileName;
- public final long fileSize;
- public String columnSeparator;
- public String absPath;
-
- public List<List<String>> lines = null;
- public int maxColNum = 0;
-
- private static final int MAX_PREVIEW_LINES = 10;
-
- public TmpFile(long id, String uuid, String originFileName, long
fileSize, String columnSeparator) {
- this.id = id;
- this.uuid = uuid;
- this.originFileName = originFileName;
- this.fileSize = fileSize;
- this.columnSeparator = columnSeparator;
- }
-
- public void save(MultipartFile file) throws IOException {
- File dest = new File(Joiner.on("/").join(rootDir, uuid));
- boolean uploadSucceed = false;
- try {
- file.transferTo(dest);
- this.absPath = dest.getAbsolutePath();
- uploadSucceed = true;
- LOG.info("upload file {} succeed at {}", this,
dest.getAbsolutePath());
- } catch (IOException e) {
- LOG.warn("failed to upload file {}, dest: {}", this,
dest.getAbsolutePath(), e);
- throw e;
- } finally {
- if (!uploadSucceed) {
- dest.delete();
- }
- }
- }
-
- public void setPreview() throws IOException {
- lines = Lists.newArrayList();
- String escapedColSep = Util.escapeSingleRegex(columnSeparator);
- try (FileReader fr = new FileReader(absPath);
- BufferedReader bf = new BufferedReader(fr)) {
- String str;
- while ((str = bf.readLine()) != null) {
- String[] cols = str.split(escapedColSep, -1); // -1 to
keep the last empty column
- lines.add(Lists.newArrayList(cols));
- if (cols.length > maxColNum) {
- maxColNum = cols.length;
- }
- if (lines.size() >= MAX_PREVIEW_LINES) {
- break;
- }
- }
- }
- }
-
- // make a copy without lines and maxColNum.
- // so that can call `setPreview` and will not affect other instance
- public TmpFile copy() {
- TmpFile copiedFile = new TmpFile(this.id, this.uuid,
this.originFileName,
- this.fileSize, this.columnSeparator);
- copiedFile.absPath = this.absPath;
- return copiedFile;
- }
-
- public void delete() {
- File file = new File(absPath);
- file.delete();
- LOG.info("delete tmp file: {}", this);
- }
-
- @Override
- public String toString() {
- StringBuilder sb = new StringBuilder();
- sb.append("[id=").append(id).append(",
uuid=").append(uuid).append("， origin name=").append(originFileName)
- .append(", size=").append(fileSize).append("]");
- return sb.toString();
- }
- }
-
- // a brief of TmpFile.
- // TODO(cmy): it can be removed by using Lombok's annotation in TmpFile
class
- public static class TmpFileBrief {
- public long id;
- public String uuid;
- public String originFileName;
- public long fileSize;
- public String columnSeparator;
-
- public TmpFileBrief(TmpFile tmpFile) {
- this.id = tmpFile.id;
- this.uuid = tmpFile.uuid;
- this.originFileName = tmpFile.originFileName;
- this.fileSize = tmpFile.fileSize;
- this.columnSeparator = tmpFile.columnSeparator;
- }
-
- public long getId() {
- return id;
- }
-
- public void setId(long id) {
- this.id = id;
- }
-
- public String getUuid() {
- return uuid;
- }
-
- public void setUuid(String uuid) {
- this.uuid = uuid;
- }
-
- public String getOriginFileName() {
- return originFileName;
- }
-
- public void setOriginFileName(String originFileName) {
- this.originFileName = originFileName;
- }
-
- public long getFileSize() {
- return fileSize;
- }
-
- public void setFileSize(long fileSize) {
- this.fileSize = fileSize;
- }
-
- public String getColumnSeparator() {
- return columnSeparator;
- }
-
- public void setColumnSeparator(String columnSeparator) {
- this.columnSeparator = columnSeparator;
- }
- }
-
-
- public static class UploadFile {
- public MultipartFile file;
- public String columnSeparator;
-
- public UploadFile(MultipartFile file, String columnSeparator) {
- this.file = file;
- this.columnSeparator = columnSeparator;
- }
- }
-
- public static class TmpFileException extends Exception {
- public TmpFileException(String msg) {
- super(msg);
- }
-
- public TmpFileException(String msg, Throwable t) {
- super(msg, t);
- }
- }
-}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
index 334ee06db28..520a5026ad8 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
@@ -17,6 +17,7 @@
package org.apache.doris.catalog;
+import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.UserException;
@@ -247,4 +248,169 @@ public class JdbcResourceTest {
JdbcResource.getFullDriverUrl(invalidUrl4);
});
}
+
+ @Test
+ public void testSecurePathRejectsPrefixConfusion() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ // A directory that merely shares a string prefix must NOT be
allowed.
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("file:///opt/doris/jdbc_drivers-evil/x.jar"));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsPathTraversal() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("file:///opt/doris/jdbc_drivers/../../etc/x.jar"));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathAllowsPathUnderAllowedDir() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ String url = "file:///opt/doris/jdbc_drivers/sub/x.jar";
+ Assertions.assertDoesNotThrow(() -> Assert.assertEquals(url,
JdbcResource.getFullDriverUrl(url)));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsHostConfusion() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "http://good.com/";;
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("http://good.com.evil.com/x.jar";));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathAllowsRemoteUnderAllowedHost() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "http://good.com/drivers";;
+ String url = "http://good.com/drivers/x.jar";;
+ Assertions.assertDoesNotThrow(() -> Assert.assertEquals(url,
JdbcResource.getFullDriverUrl(url)));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathWildcardAllowsAll() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "*";
+ String url = "file:///any/where/x.jar";
+ Assertions.assertDoesNotThrow(() -> Assert.assertEquals(url,
JdbcResource.getFullDriverUrl(url)));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsEncodedTraversal() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ // %2e%2e decodes to "..", which must be resolved the same way the
classloader resolves it.
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("file:///opt/doris/jdbc_drivers/%2e%2e/%2e%2e/etc/x.jar"));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsRemoteQueryMismatch() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "http://good.com/drivers";;
+ // A query-bearing URL must not be authorized by a query-less
allowed prefix.
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("http://good.com/drivers/x.jar?id=evil";));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsRemoteUserInfoMismatch() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "http://good.com/drivers";;
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("http://[email protected]/drivers/x.jar";));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSchemelessLegacyCharsAccepted() {
+ // The shared resolver is on the lazy load path of pre-existing
catalogs, so it applies no new
+ // restriction: a bare name using historically valid characters (e.g.
'+') must keep resolving,
+ // so an unmodified historical catalog is not broken after upgrade.
The stricter bare-name
+ // grammar applies only when a catalog is created or altered (enforced
in the JDBC connector).
+ String savedDir = Config.jdbc_drivers_dir;
+ try {
+ Config.jdbc_drivers_dir = "/opt/doris/jdbc_drivers";
+
Assert.assertEquals("file:///opt/doris/jdbc_drivers/legacy+patched.jar",
+ JdbcResource.getFullDriverUrl("legacy+patched.jar"));
+ } finally {
+ Config.jdbc_drivers_dir = savedDir;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsFileAuthority() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ // A non-local authority makes consumers fetch a remote object
though the path matches.
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("file://attacker.example/opt/doris/jdbc_drivers/evil.jar"));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testSecurePathRejectsFileQuery() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ Config.jdbc_driver_secure_path = "file:///opt/doris/jdbc_drivers";
+ Assert.assertThrows(IllegalArgumentException.class, () ->
+
JdbcResource.getFullDriverUrl("file:///opt/doris/jdbc_drivers/x.jar?evil"));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
+
+ @Test
+ public void testEmptySecurePathAllowsAll() {
+ String saved = Config.jdbc_driver_secure_path;
+ try {
+ // Empty means allow-all, same as "*" (backward-compatible
contract).
+ Config.jdbc_driver_secure_path = "";
+ String url = "file:///opt/doris/jdbc_drivers/x.jar";
+ Assertions.assertDoesNotThrow(() -> Assert.assertEquals(url,
JdbcResource.getFullDriverUrl(url)));
+ } finally {
+ Config.jdbc_driver_secure_path = saved;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
