CalvinKirs commented on code in PR #68129:
URL: https://github.com/apache/doris/pull/68129#discussion_r4035131277
##########
fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcCatalogPropertiesTest.java:
##########
@@ -268,4 +268,28 @@ void toStringMasksThePassword() {
Assertions.assertFalse(rendered.contains("secret-p"), "got: " +
rendered);
Assertions.assertTrue(rendered.contains("password=***"), "got: " +
rendered);
}
+
+ // ---- driver_url mandatory security rule: statement-side rejects,
of()-side must not ----
+
+ @Test
+ void checkCreateTimeOnlyRulesRejectsTraversalDriverUrl() {
+ // MUTATION: drop the JdbcDriverUrlSecurity.check call from
checkCreateTimeOnlyRules -> red.
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcCatalogProperties.of(
+ with(JdbcCatalogProperties.DRIVER_URL,
"file:///opt/a/../../etc/evil.jar"))
+ .checkCreateTimeOnlyRules());
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcCatalogProperties.of(
+ with(JdbcCatalogProperties.DRIVER_URL,
"sub/dir/evil.jar"))
+ .checkCreateTimeOnlyRules());
+ }
+
+ @Test
+ void ofToleratesPreRuleDriverUrl() {
Review Comment:
This is the compatibility red line of the whole PR, pinned as a test: `of()`
runs on every catalog rebuild including edit-log replay, so the rule must never
move into it - a catalog created before the rule existed with a value the rule
now rejects has to keep coming back after an FE restart. Moving the check from
`checkCreateTimeOnlyRules` into `of()` turns this red.
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java:
##########
@@ -240,6 +241,12 @@ public JdbcCatalogProperties checkCreateTimeOnlyRules() {
.require(driverClass, "Required property '" + DRIVER_CLASS +
"' is missing")
.validate();
+ // Mandatory, non-configurable security rule (no '..' segment; a bare
name must be a plain
+ // *.jar file name), shared with the iceberg-jdbc / paimon-jdbc
catalogs. It lives in this
+ // statement-time hook and NOT in of(): a catalog created before the
rule existed must keep
+ // coming back after an FE restart (see class javadoc).
+ JdbcDriverUrlSecurity.check(driverUrl);
Review Comment:
For the jdbc catalog the statement-time hook is `checkCreateTimeOnlyRules`
itself (no metastore holder). ALTER reaches it through the SPI-default
`validatePropertiesForUpdate`, which merges and falls back to
`validateProperties`. `getDriverUrl()` is read after the `jdbc.` prefix strip,
so both spellings funnel into this single check.
--
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]
