yuqi1129 commented on code in PR #12025:
URL: https://github.com/apache/gravitino/pull/12025#discussion_r3596096848
##########
common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java:
##########
@@ -88,13 +100,20 @@ public static void validateJdbcConfig(String driver,
String url, Map<String, Str
private static void checkUnsafeParameters(
String url, Map<String, String> config, List<String> unsafeParams,
String dbType) {
- String lowerUrl = url.toLowerCase();
+ // Percent-decoding in recursiveDecode can reintroduce upper-case
characters (e.g. "%4a" ->
+ // 'J'), so lower-case again here rather than relying on the pre-decode
lower-casing.
+ String lowerUrl = url.toLowerCase(Locale.ROOT);
+
+ // Parameter names that reach the JDBC driver through the config map: the
config keys
+ // themselves (defense in depth) plus any names embedded in the DBCP2
"connectionProperties"
+ // value, which is forwarded verbatim to the driver.
+ Set<String> configParamNames = collectConfigParameterNames(config);
Review Comment:
[P1] This still leaves an unsafe effective URL path unvalidated.
`JdbcCatalogOperations` strips arbitrary `gravitino.bypass.*` keys, and
`DataSourceUtils` passes the resulting map to
`BasicDataSourceFactory.createDataSource` before applying the canonical
URL/driver/user/password. With DBCP properties such as `url`,
`driverClassName`, and `initialSize > 0`, the factory initializes the pool
before returning; after that, the later setters have no effect. I reproduced
this with the pinned DBCP 2.11.0: after factory initialization and
`setUrl(canonical)`, `getConnection().getMetaData().getURL()` still returned
the bootstrap URL. Because this method only collects config/property names, the
DBCP `url` value can still carry one of the denied parameters (and can bypass
the H2 guard) without being checked. Please prevent factory initialization
until the canonical connection fields are applied—for example, hold/remove
`initialSize` and override sensitive DBCP properties before invoking the
factory—and add a regression test for this ordering.
--
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]
