Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: Upcoming Release
    • Component/s: framework
    • Labels:
      None

      Description

      • JavaMailContainer.java:127, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
        RCN: Redundant nullcheck of store, which is known to be non-null in org.apache.ofbiz.service.mail.JavaMailContainer.start()

      This method contains a redundant check of a known non-null value against the constant null.

      • JavaMailContainer.java:167, DM_CONVERT_CASE
        Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.service.mail.JavaMailContainer.makeSession(ContainerConfig$Configuration$Property)

      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.

      • JavaMailContainer.java:245, DM_BOXED_PRIMITIVE_FOR_PARSING
        Bx: Boxing/unboxing to parse a primitive org.apache.ofbiz.service.mail.JavaMailContainer.updateUrlName(URLName, Properties)

      A boxed primitive is created from a String, just to extract the unboxed primitive value. It is more efficient to just call the static parseXXX method.

      • JavaMailContainer.java:269, SIC_INNER_SHOULD_BE_STATIC
        SIC: Should org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener be a static inner class?

      This class is an inner class, but does not use its embedded reference to the object which created it. This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary. If possible, the class should be made static.

      • JavaMailContainer.java:274, SF_SWITCH_NO_DEFAULT
        SF: Switch statement found in org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener.notification(StoreEvent) where default case is missing

      This method contains a switch statement where default case is missing. Usually you need to provide a default case.

      Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

      • MimeMessageWrapper.java:50, SE_NO_SERIALVERSIONID
        SnVI: org.apache.ofbiz.service.mail.MimeMessageWrapper 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.

      • MimeMessageWrapper.java:68, IS2_INCONSISTENT_SYNC
        IS: Inconsistent synchronization of org.apache.ofbiz.service.mail.MimeMessageWrapper.session; locked 75% of time

      The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that

      The class contains a mix of locked and unlocked accesses,
      The class is not annotated as javax.annotation.concurrent.NotThreadSafe,
      At least one locked access was performed by one of the class's own methods, and
      The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
      A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

      You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

      Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held. Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

      • MimeMessageWrapper.java:69, IS2_INCONSISTENT_SYNC
        IS: Inconsistent synchronization of org.apache.ofbiz.service.mail.MimeMessageWrapper.mailProperties; locked 50% of time

      The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that

      The class contains a mix of locked and unlocked accesses,
      The class is not annotated as javax.annotation.concurrent.NotThreadSafe,
      At least one locked access was performed by one of the class's own methods, and
      The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
      A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

      You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

      Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held. Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

      • MimeMessageWrapper.java:82, IS2_INCONSISTENT_SYNC
        IS: Inconsistent synchronization of org.apache.ofbiz.service.mail.MimeMessageWrapper.message; locked 75% of time

      The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that

      The class contains a mix of locked and unlocked accesses,
      The class is not annotated as javax.annotation.concurrent.NotThreadSafe,
      At least one locked access was performed by one of the class's own methods, and
      The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
      A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

      You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

      Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held. Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

      • MimeMessageWrapper.java:87, IS2_INCONSISTENT_SYNC
        IS: Inconsistent synchronization of org.apache.ofbiz.service.mail.MimeMessageWrapper.serializedBytes; locked 66% of time

      The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that

      The class contains a mix of locked and unlocked accesses,
      The class is not annotated as javax.annotation.concurrent.NotThreadSafe,
      At least one locked access was performed by one of the class's own methods, and
      The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
      A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

      You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

      Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held. Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

      • MimeMessageWrapper.java:142, PZLA_PREFER_ZERO_LENGTH_ARRAYS
        PZLA: Should org.apache.ofbiz.service.mail.MimeMessageWrapper.getHeader(String) 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.

      • MimeMessageWrapper.java:152, PZLA_PREFER_ZERO_LENGTH_ARRAYS
        PZLA: Should org.apache.ofbiz.service.mail.MimeMessageWrapper.getFrom() 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.

      • MimeMessageWrapper.java:162, PZLA_PREFER_ZERO_LENGTH_ARRAYS
        PZLA: Should org.apache.ofbiz.service.mail.MimeMessageWrapper.getTo() 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.

      • MimeMessageWrapper.java:172, PZLA_PREFER_ZERO_LENGTH_ARRAYS
        PZLA: Should org.apache.ofbiz.service.mail.MimeMessageWrapper.getCc() 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.

      • MimeMessageWrapper.java:182, PZLA_PREFER_ZERO_LENGTH_ARRAYS
        PZLA: Should org.apache.ofbiz.service.mail.MimeMessageWrapper.getBcc() 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.

      • MimeMessageWrapper.java:294, DM_CONVERT_CASE
        Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.service.mail.MimeMessageWrapper.getMessageBody()

      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.

      • MimeMessageWrapper.java:315, DM_CONVERT_CASE
        Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.service.mail.MimeMessageWrapper.getMessageBodyContentType()

      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.

      • MimeMessageWrapper.java:526, DM_DEFAULT_ENCODING
        Dm: Found reliance on default encoding in org.apache.ofbiz.service.mail.MimeMessageWrapper.getTextFromStream(InputStream): new String(byte[], int, int)

      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.

      • ServiceMcaAction.java:36, SE_NO_SERIALVERSIONID
        SnVI: org.apache.ofbiz.service.mail.ServiceMcaAction 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.

      • ServiceMcaCondition.java:44, SE_NO_SERIALVERSIONID
        SnVI: org.apache.ofbiz.service.mail.ServiceMcaCondition 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.

      • ServiceMcaCondition.java:56, SF_SWITCH_NO_DEFAULT
        SF: Switch statement found in new org.apache.ofbiz.service.mail.ServiceMcaCondition(Element, int) where default case is missing

      This method contains a switch statement where default case is missing. Usually you need to provide a default case.

      Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

      • ServiceMcaRule.java:35, SE_NO_SERIALVERSIONID
        SnVI: org.apache.ofbiz.service.mail.ServiceMcaRule 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.

        Activity

        Hide
        Dennis Balkir Dennis Balkir added a comment -
        • Diamond Operator fixed

        class JavaMailContainer:

        • Line 127: removed unnecessary null-check
        • Line 165: added a default Locale to toLowerCase()
        • Line 244: changed valueOf() to parseInt() because there is no unnecessary boxing/unboxing this way
        • Line 253: changed valueOf() to parseInt() because there is no unnecessary boxing/unboxing this way
        • Line 256: made the inner class static
        • Line 281: added a default case

        class MimeMessageWrapper:

        • removed unnecessary else
        • Line 50: it’s not necessary for the class to implement serialVerisonUID
        • Line 67: made the method setSession synchronized
        • Line 79: made the method setMessage synchronized
        • Line 98: changed the catches to a multicatch
        • Line 142: at every call the return gets checked for null, so this is okay
        • Line 152: at every call the return gets checked for null, so this is okay
        • Line 162: at every call the return gets checked for null, so this is okay
        • Line 172: at every call the return gets checked for null, so this is okay
        • Line 182: at every call the return gets checked for null, so this is okay
        • Line 226: made the method getContentType synchronized
        • Line 230: made the method getMainPartCount synchronized
        • Line 293: added a default Locale to toLowerCase
        • Line 300: added a default Locale to toLowerCase
        • Line 313: added a default Locale to toLowerCase
        • Line 322: added a default Locale to toLowerCase
        • Line 329: added a default Locale to toLowerCase
        • Line 445: changed the dot from string to char, since it is only one char
        • Line 513: added a default charset to new String

        class ServiceMcaAction:

        • removed unnecessary else
        • Line 36: it’s not necessary for the class to implement serialVerisonUID

        class ServiceMcaCondition:

        • removed unnecessary else
        • Line 44: it’s not necessary for the class to implement serialVerisonUID
        • Line 76: added a default case to the switch

        class ServiceMcaRule:

        • Line 35: it’s not necessary for the class to implement serialVerisonUID
        Show
        Dennis Balkir Dennis Balkir added a comment - Diamond Operator fixed class JavaMailContainer: Line 127: removed unnecessary null-check Line 165: added a default Locale to toLowerCase() Line 244: changed valueOf() to parseInt() because there is no unnecessary boxing/unboxing this way Line 253: changed valueOf() to parseInt() because there is no unnecessary boxing/unboxing this way Line 256: made the inner class static Line 281: added a default case class MimeMessageWrapper: removed unnecessary else Line 50: it’s not necessary for the class to implement serialVerisonUID Line 67: made the method setSession synchronized Line 79: made the method setMessage synchronized Line 98: changed the catches to a multicatch Line 142: at every call the return gets checked for null, so this is okay Line 152: at every call the return gets checked for null, so this is okay Line 162: at every call the return gets checked for null, so this is okay Line 172: at every call the return gets checked for null, so this is okay Line 182: at every call the return gets checked for null, so this is okay Line 226: made the method getContentType synchronized Line 230: made the method getMainPartCount synchronized Line 293: added a default Locale to toLowerCase Line 300: added a default Locale to toLowerCase Line 313: added a default Locale to toLowerCase Line 322: added a default Locale to toLowerCase Line 329: added a default Locale to toLowerCase Line 445: changed the dot from string to char, since it is only one char Line 513: added a default charset to new String class ServiceMcaAction: removed unnecessary else Line 36: it’s not necessary for the class to implement serialVerisonUID class ServiceMcaCondition: removed unnecessary else Line 44: it’s not necessary for the class to implement serialVerisonUID Line 76: added a default case to the switch class ServiceMcaRule: Line 35: it’s not necessary for the class to implement serialVerisonUID
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Dennis,

        your patch is in trunk r1812051.

        Show
        mbrohl Michael Brohl added a comment - Thanks Dennis, your patch is in trunk r1812051.

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            Dennis Balkir Dennis Balkir
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development