From b408ca138e1721eaa26fae92e728eb2ed820bd06 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 30 Oct 2018 14:05:15 -0400 Subject: [PATCH] fix #76: Safe exception messages provide argument data (#77) Only logMessage is redacted. --- .../LoggableExceptionAssertionsTest.java | 2 +- preconditions/build.gradle | 3 + .../logsafe/exceptions/SafeExceptions.java | 46 ++++++++++++ .../SafeIllegalArgumentException.java | 11 ++- .../exceptions/SafeIllegalStateException.java | 11 ++- .../logsafe/exceptions/SafeIoException.java | 9 ++- .../exceptions/SafeNullPointerException.java | 7 +- .../exceptions/SafeRuntimeException.java | 11 ++- .../logsafe/exceptions/SafeExceptionTest.java | 72 +++++++++++++++++++ 9 files changed, 157 insertions(+), 15 deletions(-) create mode 100644 preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeExceptions.java create mode 100644 preconditions/src/test/java/com/palantir/logsafe/exceptions/SafeExceptionTest.java diff --git a/preconditions-assertj/src/test/java/com/palantir/logsafe/testing/LoggableExceptionAssertionsTest.java b/preconditions-assertj/src/test/java/com/palantir/logsafe/testing/LoggableExceptionAssertionsTest.java index f8cc3d7c..124f1582 100755 --- a/preconditions-assertj/src/test/java/com/palantir/logsafe/testing/LoggableExceptionAssertionsTest.java +++ b/preconditions-assertj/src/test/java/com/palantir/logsafe/testing/LoggableExceptionAssertionsTest.java @@ -47,7 +47,7 @@ public final class LoggableExceptionAssertionsTest { public void testIllegalArgumentException(LoggableExceptionAssert assertion) { assertion.isInstanceOf(SafeIllegalArgumentException.class) - .hasMessage("{index} must be less than {size}") + .hasMessage("{index} must be less than {size}: {index=4, size=1}") .hasExactlyArgs(UnsafeArg.of("index", 4), SafeArg.of("size", 1)) .hasArgs(UnsafeArg.of("index", 4)); } diff --git a/preconditions/build.gradle b/preconditions/build.gradle index a770c282..38f4cc52 100755 --- a/preconditions/build.gradle +++ b/preconditions/build.gradle @@ -4,4 +4,7 @@ dependencies { api project(':safe-logging') api 'com.google.errorprone:error_prone_annotations:2.1.3' compileOnly 'com.google.code.findbugs:jsr305' + + testImplementation 'junit:junit' + testImplementation 'org.assertj:assertj-core' } diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeExceptions.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeExceptions.java new file mode 100644 index 00000000..17a63497 --- /dev/null +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeExceptions.java @@ -0,0 +1,46 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.logsafe.exceptions; + +import com.palantir.logsafe.Arg; + +/** + * {@link SafeExceptions} provides utility functionality for SafeLoggable exception implementations. + */ +final class SafeExceptions { + private SafeExceptions() {} + + static String renderMessage(String message, Arg... args) { + if (args.length == 0) { + return message; + } + + StringBuilder builder = new StringBuilder(); + builder.append(message).append(": {"); + for (int i = 0; i < args.length; i++) { + Arg arg = args[i]; + if (i > 0) { + builder.append(", "); + } + + builder.append(arg.getName()).append("=").append(arg.getValue()); + } + builder.append("}"); + + return builder.toString(); + } +} diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalArgumentException.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalArgumentException.java index 803294c5..0b38bb19 100755 --- a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalArgumentException.java +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalArgumentException.java @@ -23,31 +23,36 @@ import java.util.List; public final class SafeIllegalArgumentException extends IllegalArgumentException implements SafeLoggable { + private final String logMessage; private final List> arguments; public SafeIllegalArgumentException() { super(""); + this.logMessage = ""; this.arguments = Collections.emptyList(); } public SafeIllegalArgumentException(String message, Arg... arguments) { - super(message); + super(SafeExceptions.renderMessage(message, arguments)); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeIllegalArgumentException(String message, Throwable cause, Arg... arguments) { - super(message, cause); + super(SafeExceptions.renderMessage(message, arguments), cause); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeIllegalArgumentException(Throwable cause) { super("", cause); + this.logMessage = ""; this.arguments = Collections.emptyList(); } @Override public String getLogMessage() { - return getMessage(); + return logMessage; } @Override diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalStateException.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalStateException.java index 76e6a604..1797c5b4 100755 --- a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalStateException.java +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIllegalStateException.java @@ -23,31 +23,36 @@ import java.util.List; public final class SafeIllegalStateException extends IllegalStateException implements SafeLoggable { + private final String logMessage; private final List> arguments; public SafeIllegalStateException() { super(""); + this.logMessage = ""; this.arguments = Collections.emptyList(); } public SafeIllegalStateException(String message, Arg... arguments) { - super(message); + super(SafeExceptions.renderMessage(message, arguments)); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeIllegalStateException(String message, Throwable cause, Arg... arguments) { - super(message, cause); + super(SafeExceptions.renderMessage(message, arguments), cause); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeIllegalStateException(Throwable cause) { super("", cause); + this.logMessage = ""; this.arguments = Collections.emptyList(); } @Override public String getLogMessage() { - return getMessage(); + return logMessage; } @Override diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIoException.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIoException.java index 0cda03f7..64a1a381 100644 --- a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIoException.java +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeIoException.java @@ -24,21 +24,24 @@ import java.util.List; public final class SafeIoException extends IOException implements SafeLoggable { + private final String logMessage; private final List> arguments; public SafeIoException(String message, Arg... arguments) { - super(message); + super(SafeExceptions.renderMessage(message, arguments)); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeIoException(String message, Throwable cause, Arg... arguments) { - super(message, cause); + super(SafeExceptions.renderMessage(message, arguments), cause); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } @Override public String getLogMessage() { - return getMessage(); + return logMessage; } @Override diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeNullPointerException.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeNullPointerException.java index 4f97bdc2..6b9cd296 100755 --- a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeNullPointerException.java +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeNullPointerException.java @@ -23,21 +23,24 @@ import java.util.List; public final class SafeNullPointerException extends NullPointerException implements SafeLoggable { + private final String logMessage; private final List> arguments; public SafeNullPointerException() { super(""); + this.logMessage = ""; this.arguments = Collections.emptyList(); } public SafeNullPointerException(String message, Arg... arguments) { - super(message); + super(SafeExceptions.renderMessage(message, arguments)); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } @Override public String getLogMessage() { - return getMessage(); + return logMessage; } @Override diff --git a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeRuntimeException.java b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeRuntimeException.java index c0968bec..6d9f04da 100644 --- a/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeRuntimeException.java +++ b/preconditions/src/main/java/com/palantir/logsafe/exceptions/SafeRuntimeException.java @@ -23,31 +23,36 @@ import java.util.List; public final class SafeRuntimeException extends RuntimeException implements SafeLoggable { + private final String logMessage; private final List> arguments; public SafeRuntimeException() { super(""); + this.logMessage = ""; this.arguments = Collections.emptyList(); } public SafeRuntimeException(String message, Arg... arguments) { - super(message); + super(SafeExceptions.renderMessage(message, arguments)); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeRuntimeException(String message, Throwable cause, Arg... arguments) { - super(message, cause); + super(SafeExceptions.renderMessage(message, arguments), cause); + this.logMessage = message; this.arguments = Collections.unmodifiableList(Arrays.asList(arguments)); } public SafeRuntimeException(Throwable cause) { super("", cause); + this.logMessage = ""; this.arguments = Collections.emptyList(); } @Override public String getLogMessage() { - return getMessage(); + return logMessage; } @Override diff --git a/preconditions/src/test/java/com/palantir/logsafe/exceptions/SafeExceptionTest.java b/preconditions/src/test/java/com/palantir/logsafe/exceptions/SafeExceptionTest.java new file mode 100644 index 00000000..9dcc2119 --- /dev/null +++ b/preconditions/src/test/java/com/palantir/logsafe/exceptions/SafeExceptionTest.java @@ -0,0 +1,72 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.logsafe.exceptions; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.UnsafeArg; +import org.junit.Test; + +@SuppressWarnings("ThrowableNotThrown") +public class SafeExceptionTest { + + @Test + public void testSafeIllegalArgumentException() { + SafeIllegalArgumentException exception = new SafeIllegalArgumentException("Failure", + SafeArg.of("value", 3), + UnsafeArg.of("user", "akarp")); + assertThat(exception.getMessage()).isEqualTo("Failure: {value=3, user=akarp}"); + assertThat(exception.getLogMessage()).isEqualTo("Failure"); + } + + @Test + public void testSafeIllegalStateException() { + SafeIllegalStateException exception = new SafeIllegalStateException("Failure", + SafeArg.of("value", 3), + UnsafeArg.of("user", "akarp")); + assertThat(exception.getMessage()).isEqualTo("Failure: {value=3, user=akarp}"); + assertThat(exception.getLogMessage()).isEqualTo("Failure"); + } + + @Test + public void testSafeIoException() { + SafeIoException exception = new SafeIoException("Failure", + SafeArg.of("value", 3), + UnsafeArg.of("user", "akarp")); + assertThat(exception.getMessage()).isEqualTo("Failure: {value=3, user=akarp}"); + assertThat(exception.getLogMessage()).isEqualTo("Failure"); + } + + @Test + public void testSafeNullPointerException() { + SafeNullPointerException exception = new SafeNullPointerException("Failure", + SafeArg.of("value", 3), + UnsafeArg.of("user", "akarp")); + assertThat(exception.getMessage()).isEqualTo("Failure: {value=3, user=akarp}"); + assertThat(exception.getLogMessage()).isEqualTo("Failure"); + } + + @Test + public void testSafeRuntimeException() { + SafeRuntimeException exception = new SafeRuntimeException("Failure", + SafeArg.of("value", 3), + UnsafeArg.of("user", "akarp")); + assertThat(exception.getMessage()).isEqualTo("Failure: {value=3, user=akarp}"); + assertThat(exception.getLogMessage()).isEqualTo("Failure"); + } +}