Skip to content

Commit

Permalink
Make a number of format string fixes
Browse files Browse the repository at this point in the history
Fixed a number of cases in which the arguments provided to a message
format string did not align with the arguments expected for that
string.  This includes:

* Attempts to format messages with too few arguments
* Attempts to format messages with a non-numeric argument when a
  numeric argument was expected
* Cases in which single quotation marks were silently dropped
  because of a quirk in the way that Java sometimes handles them in
  format strings

The build process and unit test framework has been updated to help
catch these kinds of problems in the future.
  • Loading branch information
dirmgr committed Mar 12, 2018
1 parent 2e65024 commit 8c77d1e
Show file tree
Hide file tree
Showing 45 changed files with 1,954 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,26 @@ else if (message.contains("%d"))

// Add a set of constants to the source file.
w(" /**");
w(" * The name of the properties file containing the default");
w(" * message strings.");
w(" */");
w(" static final String PROPERTIES_FILE_NAME =");
w(" \"" + propertiesFileName + "\";");
w();
w();
w();
w(" /**");
w(" * Indicates whether the unit tests are currently running.");
w(" */");
w(" private static final boolean IS_WITHIN_UNIT_TESTS =");
w(" Boolean.getBoolean(" +
"\"com.unboundid.ldap.sdk.RunningUnitTests\") ||");
w(" Boolean.getBoolean(" +
"\"com.unboundid.directory.server.RunningUnitTests\");");
w();
w();
w();
w(" /**");
w(" * The resource bundle that will be used to load the properties ",
"file.");
w(" */");
Expand Down Expand Up @@ -426,7 +446,7 @@ else if (message.contains("%d"))
w(" {");
w(" if (RESOURCE_BUNDLE == null)");
w(" {");
w(" return defaultText;");
w(" s = defaultText;");
w(" }");
w(" else");
w(" {");
Expand All @@ -441,6 +461,25 @@ else if (message.contains("%d"))
w(" MESSAGE_STRINGS.putIfAbsent(this, s);");
w(" }");
w(" }");
w();
w(" if (IS_WITHIN_UNIT_TESTS &&");
w(" (s.contains(\"{0}\") || s.contains(\"{0,number,0}\") ||");
w(" s.contains(\"{1}\") || s.contains(\"{1,number,0}\") ||");
w(" s.contains(\"{2}\") || s.contains(\"{2,number,0}\") ||");
w(" s.contains(\"{3}\") || s.contains(\"{3,number,0}\") ||");
w(" s.contains(\"{4}\") || s.contains(\"{4,number,0}\") ||");
w(" s.contains(\"{5}\") || s.contains(\"{5,number,0}\") ||");
w(" s.contains(\"{6}\") || s.contains(\"{6,number,0}\") ||");
w(" s.contains(\"{7}\") || s.contains(\"{7,number,0}\") ||");
w(" s.contains(\"{8}\") || s.contains(\"{8,number,0}\") ||");
w(" s.contains(\"{9}\") || s.contains(\"{9,number,0}\") ||");
w(" s.contains(\"{10}\") || s.contains(\"{10,number,0}\")))");
w(" {");
w(" throw new IllegalArgumentException(");
w(" \"Message \" + getClass().getName() + '.' + name() +");
w(" \" contains an un-replaced token: \" + s);");
w(" }");
w();
w(" return s;");
w(" }");

Expand Down Expand Up @@ -480,10 +519,44 @@ else if (message.contains("%d"))
w(" }");
w(" MESSAGES.putIfAbsent(this, f);");
w(" }");
w();
w(" final String formattedMessage;");
w(" synchronized (f)");
w(" {");
w(" return f.format(args);");
w(" formattedMessage = f.format(args);");
w(" }");
w();
w(" if (IS_WITHIN_UNIT_TESTS &&");
w(" (formattedMessage.contains(\"{0}\") ||");
w(" formattedMessage.contains(\"{0,number,01}\") ||");
w(" formattedMessage.contains(\"{1}\") ||");
w(" formattedMessage.contains(\"{1,number,0}\") ||");
w(" formattedMessage.contains(\"{2}\") ||");
w(" formattedMessage.contains(\"{2,number,0}\") ||");
w(" formattedMessage.contains(\"{3}\") ||");
w(" formattedMessage.contains(\"{3,number,0}\") ||");
w(" formattedMessage.contains(\"{4}\") ||");
w(" formattedMessage.contains(\"{4,number,0}\") ||");
w(" formattedMessage.contains(\"{5}\") ||");
w(" formattedMessage.contains(\"{5,number,0}\") ||");
w(" formattedMessage.contains(\"{6}\") ||");
w(" formattedMessage.contains(\"{6,number,0}\") ||");
w(" formattedMessage.contains(\"{7}\") ||");
w(" formattedMessage.contains(\"{7,number,0}\") ||");
w(" formattedMessage.contains(\"{8}\") ||");
w(" formattedMessage.contains(\"{8,number,0}\") ||");
w(" formattedMessage.contains(\"{9}\") ||");
w(" formattedMessage.contains(\"{9,number,0}\") ||");
w(" formattedMessage.contains(\"{10}\") ||");
w(" formattedMessage.contains(\"{10,number,0}\")))");
w(" {");
w(" throw new IllegalArgumentException(");
w(" \"Message \" + getClass().getName() + '.' + name() +");
w(" \" contains an un-replaced token: \" +" +
" formattedMessage);");
w(" }");
w();
w(" return f.format(args);");
w(" }");


Expand All @@ -499,7 +572,28 @@ else if (message.contains("%d"))
w(" @Override()");
w(" public String toString()");
w(" {");
w(" return get();");
w(" String s = MESSAGE_STRINGS.get(this);");
w(" if (s == null)");
w(" {");
w(" if (RESOURCE_BUNDLE == null)");
w(" {");
w(" s = defaultText;");
w(" }");
w(" else");
w(" {");
w(" try");
w(" {");
w(" s = RESOURCE_BUNDLE.getString(name());");
w(" }");
w(" catch (final Exception e)");
w(" {");
w(" s = defaultText;");
w(" }");
w(" MESSAGE_STRINGS.putIfAbsent(this, s);");
w(" }");
w(" }");
w();
w(" return s;");
w(" }");


Expand Down
1 change: 1 addition & 0 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@
<jvmarg value="-XX:+UseMembar" />
<jvmarg value="-Demma.coverage.out.file=${coverage.collected.dir}/unit.emma" />
<jvmarg value="-Demma.coverage.out.merge=false" />
<jvmarg value="-Dcom.unboundid.ldap.sdk.RunningUnitTests=true" />
<jvmarg value="-Dbasedir=${basedir}" />
<jvmarg value="-Dunit.resource.dir=${unit.resource.dir}" />
<jvmarg value="-Djava.io.tmpdir=${unit.temp.dir}" />
Expand Down
7 changes: 7 additions & 0 deletions docs/release-notes.html
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ <h3>Version 4.0.5</h3>
arguments.
<br><br>
</li>

<li>
Fixed a number of cases in which there was a mismatch between the arguments
provided to a message format string and the arguments expected by that format
string. Unit tests have been added to help prevent this from recurring.
<br><br>
</li>
</ul>

<p></p>
Expand Down
2 changes: 1 addition & 1 deletion messages/unboundid-ldapsdk-asn1.properties
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ ERR_PRINTABLE_STRING_DECODE_VALUE_NOT_PRINTABLE=Unable to create an ASN.1 \
printable string with the provided value because the value contains one or \
more characters that are not in the set of printable characters. A \
printable string may contain ASCII characters from the following set: all \
uppercase and lowercase letters, all digits, space, apostophe, open and \
uppercase and lowercase letters, all digits, space, apostrophe, open and \
close parentheses, plus sign, minus sign, comma, period, forward slash, \
colon, equal sign, and question mark.
ERR_NUMERIC_STRING_DECODE_VALUE_NOT_NUMERIC=Unable to create an ASN.1 numeric \
Expand Down
36 changes: 19 additions & 17 deletions messages/unboundid-ldapsdk-cert.properties
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ ERR_CERT_VERIFY_SIGNATURE_CANNOT_GET_SIGNATURE_VERIFIER=Unable verify the \
certificate signature because an error occurred while trying to get a \
signature verifier for the ''{0}'' signature algorithm: {1}
ERR_CERT_VERIFY_SIGNATURE_CANNOT_INIT_SIGNATURE_VERIFIER=Unable to initialize \
the ''{0}'' signature verifier with the issuer certificate's public key: {1}
the ''{0}'' signature verifier with the issuer certificate''s public key: \
{1}
ERR_CERT_VERIFY_SIGNATURE_NOT_VALID=ERROR: Certificate ''{0}'' has an \
invalid signature.
ERR_CERT_VERIFY_SIGNATURE_ERROR=ERROR: An error occurred while attempting to \
Expand Down Expand Up @@ -188,14 +189,14 @@ ERR_CSR_GEN_SIGNATURE_CANNOT_COMPUTE=An error occurred while attempting to \
compute the ''{0}'' signature for the certificate signing request: {1}
ERR_CSR_VERIFY_SIGNATURE_CANNOT_GET_PUBLIC_KEY=Unable to verify the \
certificate signing request signature because an error occurred while \
attempting to parse the request's public key: {0}
attempting to parse the request''s public key: {0}
ERR_CSR_VERIFY_SIGNATURE_CANNOT_GET_SIGNATURE_VERIFIER=Unable to verify the \
certificate signing request signature because an error occurred while \
attempting to get a signature verifier for the ''{0}'' signature \
algorithm: {1}
ERR_CSR_VERIFY_SIGNATURE_CANNOT_INIT_SIGNATURE_VERIFIER=Unable to verify the \
certificate signing request signature because an error occurred while \
attempting to initialize the ''{0}'' signature verifier with the request's \
attempting to initialize the ''{0}'' signature verifier with the request''s \
public key: {1}
ERR_CSR_VERIFY_SIGNATURE_NOT_VALID=ERROR: The certificate signing request \
with subject ''{0}'' has an invalid signature.
Expand Down Expand Up @@ -761,9 +762,9 @@ INFO_MANAGE_CERTS_SC_GEN_CERT_ARG_DAYS_VALID_DESC=The number of days that \
the certificate should be considered valid. If this argument is not \
provided, then a default value of 365 days will be used.
INFO_MANAGE_CERTS_SC_GEN_CERT_ARG_VALIDITY_START_TIME_DESC=The time that the \
certificate's validity window should start (that is, the 'notBefore' \
certificate''s validity window should start (that is, the ''notBefore'' \
value). If this is not provided, then the current time will be used. If a \
value is given, it should be in the form 'YYYYMMDDhhmmss' (for example, \
value is given, it should be in the form ''YYYYMMDDhhmmss'' (for example, \
''{0}''). Timestamp values are assumed to be in the local time zone.
INFO_MANAGE_CERTS_SC_GEN_CERT_ARG_KEY_ALGORITHM_DESC=The name of the key \
algorithm to use to generate the key pair. If present, the value will \
Expand Down Expand Up @@ -1205,10 +1206,11 @@ INFO_MANAGE_CERTS_SC_SIGN_CSR_ARG_DAYS_VALID_DESC=The number of days that \
the signed certificate should be considered valid. If this argument is not \
provided, then a default value of 365 days will be used.
INFO_MANAGE_CERTS_SC_SIGN_CSR_ARG_VALIDITY_START_TIME_DESC=The time that the \
signed certificate's validity window should start (that is, the 'notBefore' \
value). If this is not provided, then the current time will be used. If a \
value is given, it should be in the form 'YYYYMMDDhhmmss' (for example, \
''{0}''). Timestamp values are assumed to be in the local time zone.
signed certificate''s validity window should start (that is, the \
''notBefore'' value). If this is not provided, then the current time will \
be used. If a value is given, it should be in the form ''YYYYMMDDhhmmss'' \
(for example, ''{0}''). Timestamp values are assumed to be in the local \
time zone.
INFO_MANAGE_CERTS_SC_SIGN_CSR_ARG_SIG_ALG_DESC=The name of the algorithm to \
use to sign the certificate. If this is not provided, then the signature \
algorithm from the certificate signing request will be used.
Expand Down Expand Up @@ -1594,17 +1596,17 @@ INFO_MANAGE_CERTS_SC_TRUST_SERVER_ARG_NO_PROMPT_DESC=Trust the server \
INFO_MANAGE_CERTS_SC_TRUST_SERVER_ARG_VERBOSE_DESC=Display verbose \
information about the certificates in the server's certificate chain.
INFO_MANAGE_CERTS_SC_TRUST_SERVER_EXAMPLE_1=Establishes a secure connection \
to the server ds.example.com on port 636 and adds that server's certificate \
chain to the ''{0}'' keystore with a base alias of 'ds.example.com:636'. \
The tool will display verbose information about the certificate chain \
presented by the server, and will interactively prompt about whether to \
trust that chain.
to the server ds.example.com on port 636 and adds that server''s \
certificate chain to the ''{0}'' keystore with a base alias of \
''ds.example.com:636''. The tool will display verbose information about \
the certificate chain presented by the server, and will interactively \
prompt about whether to trust that chain.
INFO_MANAGE_CERTS_SC_TRUST_SERVER_EXAMPLE_2=Establishes a non-secure \
connection to ds.example.com on port 389, and then uses the LDAP StartTLS \
extended operation to transition to a secure connection. It will then add \
the server's issuer certificates to the ''{0}'' keystore with a base alias \
of 'ds-start-tls-cert'. The tool will trust the certificate chain without \
any confirmation from the user.
the server''s issuer certificates to the ''{0}'' keystore with a base alias \
of ''ds-start-tls-cert''. The tool will trust the certificate chain \
without any confirmation from the user.
INFO_MANAGE_CERTS_SC_CHECK_USABILITY_DESC=Examines a keystore to determine \
how suitable a specified certificate is for use as a server certificate.
INFO_MANAGE_CERTS_SC_CHECK_USABILITY_ARG_KS_DESC=The path to the keystore \
Expand Down
2 changes: 1 addition & 1 deletion messages/unboundid-ldapsdk-json.properties
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ ERR_OBJECT_READER_UNEXPECTED_END_OF_STREAM=Unexpected end of the input stream \
reached {0,number,0} bytes into an incomplete JSON object.
ERR_OBJECT_READER_INVALID_UTF_8_BYTE_IN_STREAM=Invalid data read from the \
input stream {0,number,0} bytes into the JSON object: non-ASCII byte \
''{0}'' encountered in the middle of a JSON string is not a valid first \
''{1}'' encountered in the middle of a JSON string is not a valid first \
byte for a multi-byte UTF-8 character.
ERR_OBJECT_READER_INVALID_UNICODE_ESCAPE=Invalid data read from the input \
stream {0,number,0} bytes into the JSON object: a Unicode escape \
Expand Down
6 changes: 3 additions & 3 deletions messages/unboundid-ldapsdk-listener.properties
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ ERR_MEM_HANDLER_NO_BASE_DNS=Unable to create an in-memory request handler \
with no base DNs.
ERR_MEM_HANDLER_NULL_BASE_DN=Unable to use the null DN as a base DN for the \
in-memory request handler.
ERR_MEM_HANDLER_CHANGELOG_BASE_DN=Unable to use the ''{0}'' as a base DN for \
the in-memory request handler because it conflicts with the base DN used \
for changelog entries.
ERR_MEM_HANDLER_CHANGELOG_BASE_DN=Unable to use ''{0}'' as a base DN for the \
in-memory request handler because it conflicts with the base DN used for \
changelog entries.
ERR_MEM_HANDLER_SCHEMA_BASE_DN=Unable to use a DN at or below the subschema \
subentry DN ''{0}'' as a base DN for the in-memory request handler.
ERR_MEM_HANDLER_EXTENDED_REQUEST_HANDLER_CONFLICT=The provided configuration \
Expand Down
4 changes: 2 additions & 2 deletions src/com/unboundid/ldap/listener/InMemoryRequestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ public InMemoryRequestHandler(final InMemoryDirectoryServerConfig config)
if (baseDNSet.contains(changeLogBaseDN))
{
throw new LDAPException(ResultCode.PARAM_ERROR,
ERR_MEM_HANDLER_CHANGELOG_BASE_DN.get());
ERR_MEM_HANDLER_CHANGELOG_BASE_DN.get(changeLogBaseDN));
}

maxChangelogEntries = config.getMaxChangeLogEntries();
Expand Down Expand Up @@ -437,7 +437,7 @@ public InMemoryRequestHandler(final InMemoryDirectoryServerConfig config)
if (baseDNs.contains(subschemaSubentryDN))
{
throw new LDAPException(ResultCode.PARAM_ERROR,
ERR_MEM_HANDLER_SCHEMA_BASE_DN.get());
ERR_MEM_HANDLER_SCHEMA_BASE_DN.get(subschemaSubentryDN));
}

if (maxChangelogEntries > 0)
Expand Down
4 changes: 2 additions & 2 deletions src/com/unboundid/ldap/matchingrules/IntegerMatchingRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ public ASN1OctetString normalize(final ASN1OctetString value)
if ((i != 0) || (valueStr.length() == 1))
{
throw new LDAPException(ResultCode.INVALID_ATTRIBUTE_SYNTAX,
ERR_INTEGER_INVALID_CHARACTER.get());
ERR_INTEGER_INVALID_CHARACTER.get(i));
}
break;

Expand Down Expand Up @@ -426,7 +426,7 @@ public ASN1OctetString normalize(final ASN1OctetString value)
if ((i != 0) || (valueBytes.length == 1))
{
throw new LDAPException(ResultCode.INVALID_ATTRIBUTE_SYNTAX,
ERR_INTEGER_INVALID_CHARACTER.get());
ERR_INTEGER_INVALID_CHARACTER.get(i));
}
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,50 @@ public final Filter toLDAPFilter(final String attributeDescription)



/**
* Creates a string representation of the provided field path. The path will
* be constructed by using the JSON value representations of the field paths
* (with each path element surrounded by quotation marks and including any
* appropriate escaping) and using the period as a delimiter between each
* path element.
*
* @param fieldPath The field path to process.
*
* @return A string representation of the provided field path.
*/
static String fieldPathToName(final List<String> fieldPath)
{
if (fieldPath == null)
{
return "null";
}
else if (fieldPath.isEmpty())
{
return "";
}
else if (fieldPath.size() == 1)
{
return new JSONString(fieldPath.get(0)).toString();
}
else
{
final StringBuilder buffer = new StringBuilder();
for (final String pathElement : fieldPath)
{
if (buffer.length() > 0)
{
buffer.append('.');
}

new JSONString(pathElement).toString(buffer);
}

return buffer.toString();
}
}



/**
* Retrieves a hash code for this JSON object filter.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ protected RegularExpressionJSONObjectFilter decodeFilter(
throw new JSONException(
ERR_REGEX_FILTER_DECODE_INVALID_REGEX.get(
String.valueOf(filterObject), FIELD_REGULAR_EXPRESSION,
StaticUtils.getExceptionMessage(e)),
fieldPathToName(fieldPath), StaticUtils.getExceptionMessage(e)),
e);
}

Expand Down
Loading

0 comments on commit 8c77d1e

Please sign in to comment.