rgodfrey commented on a change in pull request #55:
URL: https://github.com/apache/qpid-broker-j/pull/55#discussion_r465567642
##########
File path: broker-core/src/main/java/org/apache/qpid/server/util/Strings.java
##########
@@ -134,7 +134,8 @@ public static Resolver chain(Resolver... resolvers)
base64String = base64String.replaceAll("\\s","");
if(!base64String.matches("^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$"))
{
- throw new IllegalArgumentException("Cannot convert string '"+
base64String+ "'to a byte[] - it does not appear to be base64 data");
+ // do not add base64String to exception message as it can contain
private data
+ throw new IllegalArgumentException("Cannot convert string to a
byte[] - it does not appear to be base64 data");
Review comment:
My concern here is that while I understand the desire not to leak
sensitive information; for an end user trying to track down the invalid
configuration information, removing the actual value in error here would seem
to make it much more difficult.
Are there particular instances where you have identified that this could be
a problem, and can we isolate them in some way that ensures that enough context
information is provided so the user can resolve the underlying issue?
##########
File path:
broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
##########
@@ -255,7 +255,8 @@ public T convert(final Object value, C object)
Type returnType = getGetter().getGenericReturnType();
String simpleName = returnType instanceof Class ? ((Class)
returnType).getSimpleName() : returnType.toString();
- throw new IllegalArgumentException("Cannot convert '" + value
+ throw new IllegalArgumentException("Cannot convert '" +
+
(User.PASSWORD.equals(getName()) ?
AbstractConfiguredObject.SECURED_STRING_VALUE : value)
Review comment:
Rather than use `User.PASSWORD.equals(getName())` why not use
`isSecure()`? The secure annotation on an attribute is designed specifically
for this use case of identifying attributes whose value should be considered
confidential information
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
