[jira] [Updated] (OFBIZ-9720) [FB] Package org.apache.ofbiz.entity.util

["Julian Leichert (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/OFBIZ-9720?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Julian Leichert updated OFBIZ-9720:
-----------------------------------
    Attachment: OFBIZ-9720_org.apache.ofbiz.entity.util_bugfixes.patch

class EntityCrypto
 - Line 54ff : changed protected to private, to represent true use of field
 - Line 377 : added Utf8

class EntityDataLoader
 - Line 162 : added Outfit
 - Line 170 : added null-check to prevent NullPointerException
 - Line 258 : changed to multi catch for better Exception handling

class EntityQuery
 - removed trailing whitespaces

class EntitySaxReader
 - Line 315 : used InputStreamReader to add utf8
 - Line 372 : added utf8
 - Line 379 : used Arrays.toString otherwise String would be useless

class SequenceUtil
 - Line 225 : removed redundant null-check
 - Line 258 : multi-catch for better Exception handling


> [FB] Package org.apache.ofbiz.entity.util
> -----------------------------------------
>
>                 Key: OFBIZ-9720
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-9720
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: ALL APPLICATIONS, ALL COMPONENTS
>    Affects Versions: Trunk
>            Reporter: Julian Leichert
>            Priority: Minor
>         Attachments: OFBIZ-9720_org.apache.ofbiz.entity.util_bugfixes.patch
>
>
> ByteWrapper.java:35, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.entity.util.ByteWrapper is Serializable; consider declaring a serialVersionUID
> This class implements the Serializable interface, but does not define a serialVersionUID field.  A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.
> ByteWrapper.java:38, EI_EXPOSE_REP2
> - EI2: new org.apache.ofbiz.entity.util.ByteWrapper(byte[]) may expose internal representation by storing an externally mutable object into ByteWrapper.bytes
> This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
> ByteWrapper.java:42, EI_EXPOSE_REP
> - EI: org.apache.ofbiz.entity.util.ByteWrapper.getBytes() may expose internal representation by returning ByteWrapper.bytes
> Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.
> EntityCrypto.java:-1, CI_CONFUSED_INHERITANCE
> - CI: Class org.apache.ofbiz.entity.util.EntityCrypto is final but declares protected field org.apache.ofbiz.entity.util.EntityCrypto.delegator
> This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.
> EntityCrypto.java:-1, CI_CONFUSED_INHERITANCE
> - CI: Class org.apache.ofbiz.entity.util.EntityCrypto is final but declares protected field org.apache.ofbiz.entity.util.EntityCrypto.NormalHashStorageHandler
> This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.
> EntityCrypto.java:-1, CI_CONFUSED_INHERITANCE
> - CI: Class org.apache.ofbiz.entity.util.EntityCrypto is final but declares protected field org.apache.ofbiz.entity.util.EntityCrypto.OldFunnyHashStorageHandler
> This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.
> EntityCrypto.java:-1, CI_CONFUSED_INHERITANCE
> - CI: Class org.apache.ofbiz.entity.util.EntityCrypto is final but declares protected field org.apache.ofbiz.entity.util.EntityCrypto.keyMap
> This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.
> EntityCrypto.java:-1, CI_CONFUSED_INHERITANCE
> - CI: Class org.apache.ofbiz.entity.util.EntityCrypto is final but declares protected field org.apache.ofbiz.entity.util.EntityCrypto.handlers
> This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.
> EntityCrypto.java:175, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> - PZLA: Should org.apache.ofbiz.entity.util.EntityCrypto.findKey(String, EntityCrypto$StorageHandler) 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.
> EntityCrypto.java:342, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in org.apache.ofbiz.entity.util.EntityCrypto$3.getHashedKeyName(String): String.getBytes()
> Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
> EntityCrypto.java:377, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in org.apache.ofbiz.entity.util.EntityCrypto$SaltedBase64StorageHandler.getHashedKeyName(String): String.getBytes()
> Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
> EntityDataLoader.java:162, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.entity.util.EntityDataLoader.getUrlList(String, String, List)
> 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.
> EntityDataLoader.java:167, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
> - NP: Possible null pointer dereference in org.apache.ofbiz.entity.util.EntityDataLoader.getUrlList(String, String, List) due to return value of called method
> The return value from a method is dereferenced without a null check, and the return value of that method is one that should generally be checked for null. This may lead to a NullPointerException when the code is executed.
> EntityDataLoader.java:258, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.util.EntityDataLoader.loadData(URL, String, Delegator, List, int, boolean, boolean, boolean)
> 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 ...
>   }
> EntityFindOptions.java:44, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.entity.util.EntityFindOptions is Serializable; consider declaring a serialVersionUID
> This class implements the Serializable interface, but does not define a serialVersionUID field.  A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.
> EntityListIterator.java:151, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of StringBuilder.toString(), which is known to be non-null in org.apache.ofbiz.entity.util.EntityListIterator.close()
> This method contains a redundant check of a known non-null value against the constant null.
> EntityQuery.java:327, EI_EXPOSE_REP2
> - EI2: org.apache.ofbiz.entity.util.EntityQuery.filterByDate(Timestamp) may expose internal representation by storing an externally mutable object into EntityQuery.filterByDateMoment
> This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
> EntityQuery.java:365, EI_EXPOSE_REP2
> - EI2: org.apache.ofbiz.entity.util.EntityQuery.filterByDate(Timestamp, String[]) may expose internal representation by storing an externally mutable object into EntityQuery.filterByDateMoment
> This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
> EntitySaxReader.java:236, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.util.EntitySaxReader.parse(InputStream, String)
> 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 ...
>   }
> EntitySaxReader.java:315, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in org.apache.ofbiz.entity.util.EntitySaxReader.endElement(String, String, String): new java.io.InputStreamReader(InputStream)
> Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
> EntitySaxReader.java:372, DM_DEFAULT_ENCODING
> - Dm: Found reliance on default encoding in org.apache.ofbiz.entity.util.EntitySaxReader.endElement(String, String, String): String.getBytes()
> Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
> EntitySaxReader.java:378, DMI_INVOKING_TOSTRING_ON_ARRAY
> - USELESS_STRING: Invocation of toString on EntitySaxReader.currentFieldValue in org.apache.ofbiz.entity.util.EntitySaxReader.endElement(String, String, String)
> The code invokes toString on an array, which will generate a fairly useless result such as [C@16f0472. Consider using Arrays.toString to convert the array into a readable String that gives the contents of the array. See Programming Puzzlers, chapter 3, puzzle 12.
> EntityStoreOptions.java:28, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.entity.util.EntityStoreOptions is Serializable; consider declaring a serialVersionUID
> This class implements the Serializable interface, but does not define a serialVersionUID field.  A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.
> EntityUtilProperties.java:50, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.entity.util.EntityUtilProperties is Serializable; consider declaring a serialVersionUID
> This class implements the Serializable interface, but does not define a serialVersionUID field.  A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.
> SequenceUtil.java:226, SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE
> - SQL: org.apache.ofbiz.entity.util.SequenceUtil$SequenceBank.fillBank(long) passes a nonconstant String to an execute method on an SQL statement
> The method invokes the execute or addBatch method on an SQL statement with a String that seems to be dynamically generated. Consider using a prepared statement instead. It is more efficient and less vulnerable to SQL injection attacks.
> SequenceUtil.java:253, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of connection, which is known to be non-null in org.apache.ofbiz.entity.util.SequenceUtil$SequenceBank.fillBank(long)
> This method contains a redundant check of a known non-null value against the constant null.
> SequenceUtil.java:258, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.util.SequenceUtil$SequenceBank.fillBank(long)
> 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 ...
>   }



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)