pzampino commented on code in PR #914:
URL: https://github.com/apache/knox/pull/914#discussion_r1626161972
##########
gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java:
##########
@@ -176,6 +176,8 @@ public class GatewayConfigImpl extends Configuration
implements GatewayConfig {
public static final String CLUSTER_CONFIG_MONITOR_INTERVAL_SUFFIX =
".interval";
public static final String CLUSTER_CONFIG_MONITOR_ENABLED_SUFFIX =
".enabled";
+ private static final String ERROR_MESSAGE_SANITIZATION_ENABLED =
GATEWAY_CONFIG_FILE_PREFIX + ".error.sanitization.enabled";
+ private static final boolean DEFAULT_ERROR_MESSAGE_SANITIZATION_ENABLED =
true;
Review Comment:
The convention would dictate ERROR_MESSAGE_SANITIZATION_ENABLED_DEFAULT
##########
gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java:
##########
@@ -277,4 +278,34 @@ public Enumeration<String> getInitParameterNames() {
return config.getInitParameterNames();
}
}
+
+ private Exception sanitizeException(Exception e) {
+ if (e == null || e.getMessage() == null) {
+ return e;
+ }
+ if (!isErrorMessageSanitizationEnabled || e.getMessage() == null) {
+ return e;
+ }
+ String sanitizedMessage =
e.getMessage().replaceAll("\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b", "[hidden]");
+ return createNewException(e, sanitizedMessage);
+ }
+
+ private <T extends Exception> T createNewException(T e, String
sanitizedMessage) {
Review Comment:
I would prefer this be named something like createSanitizedException, and
encapsulate the sanitization as part of the creation of the new object. Is
there any scenario where the message would be sanitized and not result in a new
corresponding Exception object?
##########
gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java:
##########
@@ -277,4 +278,34 @@ public Enumeration<String> getInitParameterNames() {
return config.getInitParameterNames();
}
}
+
+ private Exception sanitizeException(Exception e) {
+ if (e == null || e.getMessage() == null) {
+ return e;
+ }
+ if (!isErrorMessageSanitizationEnabled || e.getMessage() == null) {
+ return e;
+ }
+ String sanitizedMessage =
e.getMessage().replaceAll("\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b", "[hidden]");
+ return createNewException(e, sanitizedMessage);
Review Comment:
I agree that a SanitizedException class to wrap other exceptions would be
appropriate for this. The only reason for not doing this is if upstream
catchers of these exceptions would ignore the SanitizedException because
they're expecting what would be wrapped thereby.
##########
gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java:
##########
@@ -277,4 +278,34 @@ public Enumeration<String> getInitParameterNames() {
return config.getInitParameterNames();
}
}
+
+ private Exception sanitizeException(Exception e) {
+ if (e == null || e.getMessage() == null) {
+ return e;
+ }
+ if (!isErrorMessageSanitizationEnabled || e.getMessage() == null) {
+ return e;
+ }
+ String sanitizedMessage =
e.getMessage().replaceAll("\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b", "[hidden]");
+ return createNewException(e, sanitizedMessage);
+ }
+
+ private <T extends Exception> T createNewException(T e, String
sanitizedMessage) {
+ try {
+ Constructor<? extends Exception> constructor =
e.getClass().getConstructor(String.class, Throwable.class);
+ T sanitizedException = (T) constructor.newInstance(sanitizedMessage,
e.getCause());
+ sanitizedException.setStackTrace(e.getStackTrace());
+ return sanitizedException;
+ } catch (Exception ex) {
+ Exception genericException = new Exception(sanitizedMessage,
e.getCause());
+ genericException.setStackTrace(e.getStackTrace());
+ return (T) genericException;
+ }
+ }
+
+ private <T extends Exception> T sanitizeAndRethrow(Exception e) throws T {
+ Exception sanitizedException = sanitizeException(e);
+ LOG.failedToExecuteFilter(sanitizedException);
+ throw (T) sanitizedException;
Review Comment:
Why does this have to throw rather than simply return the sanitized
exception?
--
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]
