turcsanyip commented on code in PR #6853:
URL: https://github.com/apache/nifi/pull/6853#discussion_r1095082514
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteSQLRecord.java:
##########
@@ -99,6 +105,32 @@
@WritesAttribute(attribute = "mime.type", description = "Sets the
mime.type attribute to the MIME Type specified by the Record Writer."),
@WritesAttribute(attribute = "record.count", description = "The number
of records output by the Record Writer.")
})
+@DynamicProperties({
Review Comment:
`@SupportsSensitiveDynamicProperties` is missing.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -711,18 +711,59 @@ public static void setParameters(final PreparedStatement
stmt, final Map<String,
}
}
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes.
+ *
+ * @param stmt the statement to set the parameters on
+ * @param attributes the attributes from which to derive parameter
indices, values, and types
+ * @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
+ */
+ public static void setSensitiveParameters(final PreparedStatement stmt,
final Map<String, SensitiveValueWrapper> attributes) throws SQLException {
Review Comment:
This and the old `setParameters()` methods have mostly the same logic,
except the masked logging.
To avoid code duplication, would it be possible the extract the common parts?
The simplest (but maybe not the best) approach would be to convert
`Map<String, String>` to `Map<String, SensitiveValueWrapper>` (with fixed
`sensitive=false`) in the old method and then to call
`setSensitiveParameters()` with that map.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -711,18 +711,59 @@ public static void setParameters(final PreparedStatement
stmt, final Map<String,
}
}
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes.
+ *
+ * @param stmt the statement to set the parameters on
+ * @param attributes the attributes from which to derive parameter
indices, values, and types
+ * @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
+ */
+ public static void setSensitiveParameters(final PreparedStatement stmt,
final Map<String, SensitiveValueWrapper> attributes) throws SQLException {
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
attributes.entrySet()) {
+ final String key = entry.getKey();
+ final boolean isSensitive = entry.getValue().isSensitive();
+ final String value = entry.getValue().getValue();
+ final String logValue = isSensitive ? "???" : value;
+ final Matcher matcher = SQL_TYPE_ATTRIBUTE_PATTERN.matcher(key);
+ if (matcher.matches()) {
+ final int parameterIndex = Integer.parseInt(matcher.group(1));
+
+ final boolean isNumeric =
NUMBER_PATTERN.matcher(value).matches();
+ if (!isNumeric) {
+ throw new SQLDataException("Value of the " + key + "
attribute is '" + logValue + "', which is not a valid JDBC numeral type");
Review Comment:
The JDBC type of the parameter is logged here (the value of
`sql.args.N.type`). I don't think we need to handle it sensitively even if the
user has configured the property that way for some reason. The point is to mask
`sql.args.N.value`'s value (the SQL parameter value).
@exceptionfactory What is your opinion?
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -711,18 +711,59 @@ public static void setParameters(final PreparedStatement
stmt, final Map<String,
}
}
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes.
+ *
+ * @param stmt the statement to set the parameters on
+ * @param attributes the attributes from which to derive parameter
indices, values, and types
+ * @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
+ */
+ public static void setSensitiveParameters(final PreparedStatement stmt,
final Map<String, SensitiveValueWrapper> attributes) throws SQLException {
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
attributes.entrySet()) {
+ final String key = entry.getKey();
+ final boolean isSensitive = entry.getValue().isSensitive();
+ final String value = entry.getValue().getValue();
+ final String logValue = isSensitive ? "???" : value;
+ final Matcher matcher = SQL_TYPE_ATTRIBUTE_PATTERN.matcher(key);
+ if (matcher.matches()) {
+ final int parameterIndex = Integer.parseInt(matcher.group(1));
+
+ final boolean isNumeric =
NUMBER_PATTERN.matcher(value).matches();
+ if (!isNumeric) {
+ throw new SQLDataException("Value of the " + key + "
attribute is '" + logValue + "', which is not a valid JDBC numeral type");
+ }
+
+ final int jdbcType = Integer.parseInt(value);
+ final String valueAttrName = "sql.args." + parameterIndex +
".value";
+ final String parameterValue =
attributes.get(valueAttrName).getValue();
+ final String logParameterValue = isSensitive ? "???" :
parameterValue;
Review Comment:
`isSensitive` variable contains the sensitivity of `sql.args.N.type`
property (which may not be needed at all as I mentioned in my previous
comment). Furthermore, it is not relevant for the parameter value
(`sql.args.N.value`). The 2 properties can be configured with different
sensitiveness.
`attributes.get(valueAttrName).isSensitive()` needs to be retrieved and used
instead.
--
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]
