turcsanyip commented on code in PR #6853:
URL: https://github.com/apache/nifi/pull/6853#discussion_r1111786203
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -681,32 +684,58 @@ public static String normalizeNameForAvro(String
inputName) {
* @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
*/
public static void setParameters(final PreparedStatement stmt, final
Map<String, String> attributes) throws SQLException {
- for (final Map.Entry<String, String> entry : attributes.entrySet()) {
- final String key = entry.getKey();
- 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(entry.getValue()).matches();
- if (!isNumeric) {
- throw new SQLDataException("Value of the " + key + "
attribute is '" + entry.getValue() + "', which is not a valid JDBC numeral
type");
- }
+ final Map<String, SensitiveValueWrapper> sensitiveValueWrapperMap =
attributes.entrySet()
+ .stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, e -> new
SensitiveValueWrapper(e.getValue(), false)));
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
sensitiveValueWrapperMap.entrySet()) {
+ final String flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, sensitiveValueWrapperMap,
flowFileAttributeKey, flowFileAttributeValue);
+ }
Review Comment:
This is the same as the body of `setSensitiveParameters()`.
`setSensitiveParameters(stmt, sensitiveValueWrapperMap)` could be called
instead.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -681,32 +684,58 @@ public static String normalizeNameForAvro(String
inputName) {
* @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
*/
public static void setParameters(final PreparedStatement stmt, final
Map<String, String> attributes) throws SQLException {
- for (final Map.Entry<String, String> entry : attributes.entrySet()) {
- final String key = entry.getKey();
- 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(entry.getValue()).matches();
- if (!isNumeric) {
- throw new SQLDataException("Value of the " + key + "
attribute is '" + entry.getValue() + "', which is not a valid JDBC numeral
type");
- }
+ final Map<String, SensitiveValueWrapper> sensitiveValueWrapperMap =
attributes.entrySet()
+ .stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, e -> new
SensitiveValueWrapper(e.getValue(), false)));
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
sensitiveValueWrapperMap.entrySet()) {
+ final String flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, sensitiveValueWrapperMap,
flowFileAttributeKey, flowFileAttributeValue);
+ }
+ }
- final int jdbcType = Integer.parseInt(entry.getValue());
- final String valueAttrName = "sql.args." + parameterIndex +
".value";
- final String parameterValue = attributes.get(valueAttrName);
- final String formatAttrName = "sql.args." + parameterIndex +
".format";
- final String parameterFormat =
attributes.containsKey(formatAttrName)? attributes.get(formatAttrName):"";
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes
+ * and masks sensitive values.
+ *
+ * @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 flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, attributes, flowFileAttributeKey,
flowFileAttributeValue);
+ }
+ }
- try {
- JdbcCommon.setParameter(stmt, valueAttrName,
parameterIndex, parameterValue, jdbcType, parameterFormat);
- } catch (final NumberFormatException nfe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted into
the necessary data type", nfe);
- } catch (ParseException pe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to a
timestamp", pe);
- } catch (UnsupportedEncodingException uee) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to
UTF-8", uee);
- }
+ private static void setParameterAtIndex(PreparedStatement stmt,
Map<String, SensitiveValueWrapper> attributes, String flowFileAttributeKey,
String flowFileAttributeValue) throws SQLException {
+ final Matcher sqlArgumentTypeMatcher =
SQL_TYPE_ATTRIBUTE_PATTERN.matcher(flowFileAttributeKey);
+ if (sqlArgumentTypeMatcher.matches()) {
+ final int sqlArgumentTypeIndex =
Integer.parseInt(sqlArgumentTypeMatcher.group(1));
Review Comment:
This index is not only "type" related but belongs to the type-value-format
triplet.
```suggestion
final int sqlArgumentIndex =
Integer.parseInt(sqlArgumentTypeMatcher.group(1));
```
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -681,32 +684,58 @@ public static String normalizeNameForAvro(String
inputName) {
* @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
*/
public static void setParameters(final PreparedStatement stmt, final
Map<String, String> attributes) throws SQLException {
- for (final Map.Entry<String, String> entry : attributes.entrySet()) {
- final String key = entry.getKey();
- 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(entry.getValue()).matches();
- if (!isNumeric) {
- throw new SQLDataException("Value of the " + key + "
attribute is '" + entry.getValue() + "', which is not a valid JDBC numeral
type");
- }
+ final Map<String, SensitiveValueWrapper> sensitiveValueWrapperMap =
attributes.entrySet()
+ .stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, e -> new
SensitiveValueWrapper(e.getValue(), false)));
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
sensitiveValueWrapperMap.entrySet()) {
+ final String flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, sensitiveValueWrapperMap,
flowFileAttributeKey, flowFileAttributeValue);
+ }
+ }
- final int jdbcType = Integer.parseInt(entry.getValue());
- final String valueAttrName = "sql.args." + parameterIndex +
".value";
- final String parameterValue = attributes.get(valueAttrName);
- final String formatAttrName = "sql.args." + parameterIndex +
".format";
- final String parameterFormat =
attributes.containsKey(formatAttrName)? attributes.get(formatAttrName):"";
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes
+ * and masks sensitive values.
+ *
+ * @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 flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, attributes, flowFileAttributeKey,
flowFileAttributeValue);
+ }
+ }
- try {
- JdbcCommon.setParameter(stmt, valueAttrName,
parameterIndex, parameterValue, jdbcType, parameterFormat);
- } catch (final NumberFormatException nfe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted into
the necessary data type", nfe);
- } catch (ParseException pe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to a
timestamp", pe);
- } catch (UnsupportedEncodingException uee) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to
UTF-8", uee);
- }
+ private static void setParameterAtIndex(PreparedStatement stmt,
Map<String, SensitiveValueWrapper> attributes, String flowFileAttributeKey,
String flowFileAttributeValue) throws SQLException {
Review Comment:
```suggestion
private static void setParameterAtIndex(final PreparedStatement stmt,
Map<String, final SensitiveValueWrapper> attributes, final String
flowFileAttributeKey, final String flowFileAttributeValue) throws SQLException {
```
##########
nifi-nar-bundles/nifi-extension-utils/nifi-database-utils/src/main/java/org/apache/nifi/util/db/JdbcCommon.java:
##########
@@ -681,32 +684,58 @@ public static String normalizeNameForAvro(String
inputName) {
* @throws SQLException if the PreparedStatement throws a SQLException
when the appropriate setter is called
*/
public static void setParameters(final PreparedStatement stmt, final
Map<String, String> attributes) throws SQLException {
- for (final Map.Entry<String, String> entry : attributes.entrySet()) {
- final String key = entry.getKey();
- 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(entry.getValue()).matches();
- if (!isNumeric) {
- throw new SQLDataException("Value of the " + key + "
attribute is '" + entry.getValue() + "', which is not a valid JDBC numeral
type");
- }
+ final Map<String, SensitiveValueWrapper> sensitiveValueWrapperMap =
attributes.entrySet()
+ .stream()
+ .collect(Collectors.toMap(Map.Entry::getKey, e -> new
SensitiveValueWrapper(e.getValue(), false)));
+ for (final Map.Entry<String, SensitiveValueWrapper> entry :
sensitiveValueWrapperMap.entrySet()) {
+ final String flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, sensitiveValueWrapperMap,
flowFileAttributeKey, flowFileAttributeValue);
+ }
+ }
- final int jdbcType = Integer.parseInt(entry.getValue());
- final String valueAttrName = "sql.args." + parameterIndex +
".value";
- final String parameterValue = attributes.get(valueAttrName);
- final String formatAttrName = "sql.args." + parameterIndex +
".format";
- final String parameterFormat =
attributes.containsKey(formatAttrName)? attributes.get(formatAttrName):"";
+ /**
+ * Sets all of the appropriate parameters on the given PreparedStatement,
based on the given FlowFile attributes
+ * and masks sensitive values.
+ *
+ * @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 flowFileAttributeKey = entry.getKey();
+ final String flowFileAttributeValue = entry.getValue().getValue();
+ setParameterAtIndex(stmt, attributes, flowFileAttributeKey,
flowFileAttributeValue);
+ }
+ }
- try {
- JdbcCommon.setParameter(stmt, valueAttrName,
parameterIndex, parameterValue, jdbcType, parameterFormat);
- } catch (final NumberFormatException nfe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted into
the necessary data type", nfe);
- } catch (ParseException pe) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to a
timestamp", pe);
- } catch (UnsupportedEncodingException uee) {
- throw new SQLDataException("The value of the " +
valueAttrName + " is '" + parameterValue + "', which cannot be converted to
UTF-8", uee);
- }
+ private static void setParameterAtIndex(PreparedStatement stmt,
Map<String, SensitiveValueWrapper> attributes, String flowFileAttributeKey,
String flowFileAttributeValue) throws SQLException {
+ final Matcher sqlArgumentTypeMatcher =
SQL_TYPE_ATTRIBUTE_PATTERN.matcher(flowFileAttributeKey);
+ if (sqlArgumentTypeMatcher.matches()) {
+ final int sqlArgumentTypeIndex =
Integer.parseInt(sqlArgumentTypeMatcher.group(1));
+
+ final boolean isNumeric =
NUMBER_PATTERN.matcher(flowFileAttributeValue).matches();
+ if (!isNumeric) {
+ throw new SQLDataException("Value of the " +
flowFileAttributeKey + " attribute is '" + flowFileAttributeValue + "', which
is not a valid numeral SQL data type");
+ }
Review Comment:
The are a couple of variable renaming in the PR (I believe with the intent
of making the algorithm more readable) but `flowFileAttributeKey` /
`flowFileAttributeValue` are still meaningless.
I would suggest redeclaring `flowFileAttributeKey` with the proper name when
the key matches the "type" pattern. Also, `flowFileAttributeValue` is not
necessary to be passed in as a parameter. It can simply be retrieved from the
attributes map (and then use a proper variable name):
```suggestion
final String sqlArgumentTypeAttributeName = flowFileAttributeKey;
final String sqlArgumentType =
attributes.get(sqlArgumentTypeAttributeName).getValue();
final boolean isNumeric =
NUMBER_PATTERN.matcher(sqlArgumentType).matches();
if (!isNumeric) {
throw new SQLDataException("Value of the " +
sqlArgumentTypeAttributeName + " attribute is '" + sqlArgumentType + "', which
is not a valid numeral SQL data type");
}
```
--
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]
