Minor Description
- BooleanConverters.java:72, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.base.conversion.BooleanConverters$StringToBoolean.convert(String)
A String is being converted to upper or lowercase, using the platform's default encoding. This may result in improper conversions when used with international characters. Use the
String.toUpperCase( Locale l )
String.toLowerCase( Locale l )
versions instead.
- CollectionConverters.java:46, BC_VACUOUS_INSTANCEOF
BC: instanceof will always return true for all nonnull values in org.apache.ofbiz.base.conversion.CollectionConverters$ArrayCreator.createConverter(Class, Class), since all Class are instances of Object
This instanceof test will always return true (unless the value being tested is null). Although this is safe, make sure it isn't an indication of some misunderstanding or some other logic error. If you really want to test the value for being null, perhaps it would be clearer to do better to do a null test rather than an instanceof test.
- Converters.java:39, MS_MUTABLE_COLLECTION_PKGPROTECT
Field is a mutable collection which should be package protected
A mutable collection instance is assigned to a final static field, thus can be changed by malicious code or by accident from another package. The field could be made package protected to avoid this vulnerability. Alternatively you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to avoid this vulnerability.
- Converters.java:40, MS_MUTABLE_COLLECTION_PKGPROTECT
Field is a mutable collection which should be package protected
A mutable collection instance is assigned to a final static field, thus can be changed by malicious code or by accident from another package. The field could be made package protected to avoid this vulnerability. Alternatively you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to avoid this vulnerability.
- Converters.java:154, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.base.conversion.Converters.loadContainedConverters(Class)
This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try
{ ... }catch (Exception e)
{ something }as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.
A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:
try
{ ... }catch (RuntimeException e)
{ throw e; }catch (Exception e)
{ ... deal with all non-runtime exceptions ... }- MiscConverters.java:90, PZLA_PREFER_ZERO_LENGTH_ARRAYS
PZLA: Should org.apache.ofbiz.base.conversion.MiscConverters$ByteBufferToByteArray.convert(ByteBuffer) return a zero length array rather than null?
It is often a better design to return a length zero array rather than a null reference to indicate that there are no results (i.e., an empty list of results). This way, no explicit check for null is needed by clients of the method.
On the other hand, using null to indicate "there is no answer to this question" is probably appropriate. For example, File.listFiles() returns an empty list if given a directory containing no files, and returns null if the file is not a directory.