Details

      Description

      DumbTransactionFactory.java:50, SIC_INNER_SHOULD_BE_STATIC_ANON

      • SIC: The class org.apache.ofbiz.entity.transaction.DumbTransactionFactory$1 could be refactored into a named 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 into a static inner class. Since anonymous inner classes cannot be marked as static, doing this will require refactoring the inner class so that it is a named inner class.

      DumbTransactionFactory.java:84, SIC_INNER_SHOULD_BE_STATIC_ANON

      • SIC: The class org.apache.ofbiz.entity.transaction.DumbTransactionFactory$2 could be refactored into a named 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 into a static inner class. Since anonymous inner classes cannot be marked as static, doing this will require refactoring the inner class so that it is a named inner class.

      GenericXaResource.java:210, ICAST_INTEGER_MULTIPLY_CAST_TO_LONG

      • ICAST: Result of integer multiplication cast to long in org.apache.ofbiz.entity.transaction.GenericXaResource.run()

      This code performs integer multiply and then converts the result to a long, as in:

      long convertDaysToMilliseconds(int days)

      { return 1000*3600*24*days; }

      If the multiplication is done using long arithmetic, you can avoid the possibility that the result will overflow. For example, you could fix the above code to:

      long convertDaysToMilliseconds(int days)

      { return 1000L*3600*24*days; }

      or
      static final long MILLISECONDS_PER_DAY = 24L*3600*1000;
      long convertDaysToMilliseconds(int days)

      { return days * MILLISECONDS_PER_DAY; }

      JNDITransactionFactory.java:56, MS_SHOULD_BE_FINAL

      • MS: org.apache.ofbiz.entity.transaction.JNDITransactionFactory.dsCache isn't final but should be

      This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

      JNDITransactionFactory.java:59, DC_DOUBLECHECK

      • DC: Possible doublecheck on org.apache.ofbiz.entity.transaction.JNDITransactionFactory.transactionManager in org.apache.ofbiz.entity.transaction.JNDITransactionFactory.getTransactionManager()

      This method may contain an instance of double-checked locking. This idiom is not correct according to the semantics of the Java memory model. For more information, see the web page http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html.

      JNDITransactionFactory.java:74, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD

      • ST: Write to static field org.apache.ofbiz.entity.transaction.JNDITransactionFactory.transactionManager from instance method org.apache.ofbiz.entity.transaction.JNDITransactionFactory.getTransactionManager()

      This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

      JNDITransactionFactory.java:95, DC_DOUBLECHECK

      • DC: Possible doublecheck on org.apache.ofbiz.entity.transaction.JNDITransactionFactory.userTransaction in org.apache.ofbiz.entity.transaction.JNDITransactionFactory.getUserTransaction()

      This method may contain an instance of double-checked locking. This idiom is not correct according to the semantics of the Java memory model. For more information, see the web page http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html.

      JNDITransactionFactory.java:109, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD

      • ST: Write to static field org.apache.ofbiz.entity.transaction.JNDITransactionFactory.userTransaction from instance method org.apache.ofbiz.entity.transaction.JNDITransactionFactory.getUserTransaction()

      This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

      JNDITransactionFactory.java:121, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD

      • ST: Write to static field org.apache.ofbiz.entity.transaction.JNDITransactionFactory.transactionManager from instance method org.apache.ofbiz.entity.transaction.JNDITransactionFactory.getUserTransaction()

      This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

      TransactionUtil.java:77, MS_SHOULD_BE_FINAL

      • MS: org.apache.ofbiz.entity.transaction.TransactionUtil.debugResMap isn't final but should be

      This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

        Activity

        Hide
        jleichert Julian Leichert added a comment -

        class GenericXaResource

        • line 210: changed to long

        class JNDITransactionFactory

        • line 52,53: changed static to volatile. fixes double-locking and writing to static.

        class TransactionUtil

        • line 77: changed to final
        Show
        jleichert Julian Leichert added a comment - class GenericXaResource line 210: changed to long class JNDITransactionFactory line 52,53: changed static to volatile. fixes double-locking and writing to static. class TransactionUtil line 77: changed to final
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Julian,

        your patch is in trunk r1812915.

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

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            jleichert Julian Leichert
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development