LuciferYang commented on code in PR #12025:
URL: https://github.com/apache/gravitino/pull/12025#discussion_r3600175461
##########
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:
Good catch, and thanks for the DBCP 2.11.0 repro — I reproduced the same:
with a raw `url` + `initialSize > 0`, `BasicDataSourceFactory` initializes the
pool on the bootstrap URL before the later `setUrl(canonical)` takes effect, so
`getConnection().getMetaData().getURL()` returns the attacker URL.
Since the root cause is `DataSourceUtils`'s factory-init ordering (not the
`JdbcUrlUtils` name detection this PR is about), I've addressed it in the
companion PR #12036, which rejects unsafe DBCP pool properties before the
config reaches the factory. Following your comment I expanded that blocklist to
also cover the raw `url`, `username`, and `password` keys — the canonical
values are applied from `jdbc-url`/`jdbc-user`/`jdbc-password` via explicit
setters, so the raw DBCP keys are never legitimate — alongside
`driverClassName`/`initialSize`. I added a regression test that a raw-`url` +
`initialSize` config is rejected before factory init, plus a positive control
asserting a legitimate config's live connection uses the canonical URL. Could
we continue this thread on #12036?
--
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]
