Copilot commented on code in PR #13239:
URL: https://github.com/apache/gravitino/pull/13239#discussion_r4026182570
##########
common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java:
##########
@@ -86,23 +87,57 @@ public static void validateJdbcConfig(String driver, String
url, Map<String, Str
Preconditions.checkArgument(StringUtils.isNotBlank(url), "JDBC URL can't
be blank");
String lowerUrl = url.toLowerCase(Locale.ROOT);
- String decodedUrl = recursiveDecode(lowerUrl);
-
- if (decodedUrl.startsWith("jdbc:mysql")) {
- checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS, "MySQL");
- } else if (decodedUrl.startsWith("jdbc:mariadb")) {
- checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS,
"MariaDB");
- } else if (decodedUrl.startsWith("jdbc:postgresql")) {
- checkUnsafeParameters(decodedUrl, all, UNSAFE_POSTGRES_PARAMETERS,
"PostgreSQL");
+ List<String> decodedForms = decodedFormsForScan(lowerUrl);
+
+ if (anyFormStartsWith(decodedForms, "jdbc:mysql")) {
+ checkUnsafeParameters(decodedForms, all, UNSAFE_MYSQL_PARAMETERS,
"MySQL");
+ } else if (anyFormStartsWith(decodedForms, "jdbc:mariadb")) {
+ checkUnsafeParameters(decodedForms, all, UNSAFE_MYSQL_PARAMETERS,
"MariaDB");
+ } else if (anyFormStartsWith(decodedForms, "jdbc:postgresql")) {
+ checkUnsafeParameters(decodedForms, all, UNSAFE_POSTGRES_PARAMETERS,
"PostgreSQL");
}
}
- private static void checkUnsafeParameters(
- String url, Map<String, String> config, List<String> unsafeParams,
String dbType) {
+ /**
+ * Returns the decoded forms of a JDBC URL that unsafe-parameter scans must
cover. Drivers such as
+ * MySQL Connector/J decode query tokens independently and ignore the URL
fragment, so a malformed
+ * percent escape in one part of the URL must not stop the scan from
revealing parameter names
+ * hidden behind valid encodings in another part. The returned forms are the
URL decoded until the
+ * first undecodable escape, plus the fully decoded form of the URL with
malformed escapes treated
+ * as literal '{@code %}' characters.
+ *
+ * @param url the JDBC URL, already lower-cased by the caller.
+ * @return the candidate decoded forms, never empty.
+ */
+ public static List<String> decodedFormsForScan(String url) {
+ // Percent-decoding can reintroduce upper-case characters (e.g. "%4a" ->
'J'), so the
+ // returned forms are lower-cased for substring and prefix matching.
+ String stoppedAtMalformed = recursiveDecode(url).toLowerCase(Locale.ROOT);
+ String fullyDecoded =
+
recursiveDecode(sanitizeMalformedPercentEscapes(url)).toLowerCase(Locale.ROOT);
Review Comment:
This only sanitizes malformed escapes once before calling the recursive
decoder, so a malformed fragment can still block later decode passes. For
example, `jdbc:mysql://localhost:3306/test?%2561utoDeserialize=true#%zz`
produces a raw form and a once-decoded `%61utoDeserialize` form; neither
contains `autodeserialize`, so the double-encoded unsafe parameter is accepted
even though MySQL decodes the query independently. Sanitize malformed percent
signs on every decode pass (or otherwise continue decoding valid escapes after
malformed ones) and add this fragment-poisoned double-encoding regression; the
existing double-encoding test at `TestJdbcUrlUtils.java:527` shows this case
must be rejected.
--
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]
