[ https://issues.apache.org/jira/browse/OFBIZ-9783?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Brohl closed OFBIZ-9783. -------------------------------- Resolution: Implemented Fix Version/s: Upcoming Release Thanks Dennis, your patch is in trunk r1817641. > [FB] Package org.apache.ofbiz.order.shoppingcart > ------------------------------------------------ > > Key: OFBIZ-9783 > URL: https://issues.apache.org/jira/browse/OFBIZ-9783 > Project: OFBiz > Issue Type: Sub-task > Components: framework > Affects Versions: Trunk > Reporter: Dennis Balkir > Assignee: Michael Brohl > Priority: Minor > Fix For: Upcoming Release > > Attachments: > OFBIZ-9783_org.apache.ofbiz.order.shoppingcart_bugfixes.patch > > > --- CartEventListener.java:81, DM_BOXED_PRIMITIVE_TOSTRING > Bx: Primitive boxed just to call toString in > org.apache.ofbiz.order.shoppingcart.CartEventListener.sessionDestroyed(HttpSessionEvent) > A boxed primitive is allocated just to call toString(). It is more effective > to just use the static form of toString which takes the primitive value. So, > Replace... With this... > new Integer(1).toString() Integer.toString(1) > new Long(1).toString() Long.toString(1) > new Float(1.0).toString() Float.toString(1.0) > new Double(1.0).toString() Double.toString(1.0) > new Byte(1).toString() Byte.toString(1) > new Short(1).toString() Short.toString(1) > new Boolean(true).toString() Boolean.toString(true) > --- CheckOutEvents.java:70, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of cart, which is known to be non-null in > org.apache.ofbiz.order.shoppingcart.CheckOutEvents.cartNotEmpty(HttpServletRequest, > HttpServletResponse) > This method contains a redundant check of a known non-null value against the > constant null. > --- CheckOutEvents.java:471, DM_CONVERT_CASE > Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in > org.apache.ofbiz.order.shoppingcart.CheckOutEvents.createOrder(HttpServletRequest, > HttpServletResponse) > 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. > --- CheckOutHelper.java:101, DM_STRING_TOSTRING > Dm: > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutShippingAddress(String) > invokes toString() method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- CheckOutHelper.java:118, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 120 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutShippingAddressInternal(String) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:142, DM_STRING_TOSTRING > Dm: > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutShippingOptions(String, > String, String, String, String, String, String, String, String) invokes > toString() method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- CheckOutHelper.java:170, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 172 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutShippingOptionsInternal(String, > String, String, String, String, String, String, String, String) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:236, DM_STRING_TOSTRING > Dm: > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutPayment(Map, > List, String) invokes toString() method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- CheckOutHelper.java:257, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 290 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutPaymentInternal(Map, > List, String) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:374, DM_STRING_TOSTRING > Dm: > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutDates(Timestamp, > Timestamp) invokes toString() method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- CheckOutHelper.java:424, DM_STRING_TOSTRING > Dm: > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.setCheckOutOptions(String, > String, Map, List, String, String, String, String, String, String, String, > String, String) invokes toString() method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- CheckOutHelper.java:452, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 455 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.checkGiftCard(Map, Map) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:612, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.cart, which is known to be > non-null in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.createOrder(GenericValue, > String, String, List, boolean, String, String) > This method contains a redundant check of a known non-null value against the > constant null. > --- CheckOutHelper.java:1207, DM_CONVERT_CASE > Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.checkOrderBlackList() > 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. > --- CheckOutHelper.java:1258, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.cart, which is known to be > non-null in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.checkOrderBlackList() > This method contains a redundant check of a known non-null value against the > constant null. > --- CheckOutHelper.java:1273, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 1285 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.failedBlacklistCheck(GenericValue, > GenericValue) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:1335, DM_CONVERT_CASE > Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.checkExternalPayment(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. > --- CheckOutHelper.java:1426, NP_NULL_ON_SOME_PATH > NP: Possible null pointer dereference of CheckOutHelper.cart in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.finalizeOrderEntryOptions(int, > String, String, String, String, String, String, String, String, String, > String) > There is a branch of statement that, if executed, guarantees that a null > value will be dereferenced, which would generate a NullPointerException when > the code is executed. Of course, the problem might be that the branch or > statement is infeasible and that the null pointer exception can't ever be > executed; deciding that is beyond the ability of FindBugs. > --- CheckOutHelper.java:1525, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of CheckOutHelper.cart at line 1540 of value previously > dereferenced in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.validatePaymentMethods() > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- CheckOutHelper.java:1549, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of paymentMethods, which is known to be non-null in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.validatePaymentMethods() > This method contains a redundant check of a known non-null value against the > constant null. > --- CheckOutHelper.java:1575, DLS_DEAD_LOCAL_STORE > DLS: Dead store to setOverflow in > org.apache.ofbiz.order.shoppingcart.CheckOutHelper.validatePaymentMethods() > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCart.java:-1, UWF_NULL_FIELD > UwF: Field only ever set to null: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo.carrierRoleTypeId > All writes to this field are of the constant value null, and thus all reads > of the field will return null. Check for errors, or remove it if it is > useless. > --- ShoppingCart.java:-1, UWF_NULL_FIELD > UwF: Field only ever set to null: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartPaymentInfo.track2 > All writes to this field are of the constant value null, and thus all reads > of the field will return null. Check for errors, or remove it if it is > useless. > --- ShoppingCart.java:-1, UWF_NULL_FIELD > UwF: Field only ever set to null: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo.telecomContactMechId > All writes to this field are of the constant value null, and thus all reads > of the field will return null. Check for errors, or remove it if it is > useless. > --- ShoppingCart.java:79, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCart 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. > --- ShoppingCart.java:434, EI_EXPOSE_REP2 > EI2: org.apache.ofbiz.order.shoppingcart.ShoppingCart.setOrderDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart.orderDate > 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. > --- ShoppingCart.java:438, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCart.getOrderDate() may > expose internal representation by returning ShoppingCart.orderDate > 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. > --- ShoppingCart.java:466, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCart.getCartCreatedTime() may > expose internal representation by returning ShoppingCart.cartCreatedTs > 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. > --- ShoppingCart.java:1202, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.setDefaultShipBeforeDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart.defaultShipBeforeDate > 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. > --- ShoppingCart.java:1206, EI_EXPOSE_REP > EI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.getDefaultShipBeforeDate() > may expose internal representation by returning > ShoppingCart.defaultShipBeforeDate > 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. > --- ShoppingCart.java:1210, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.setDefaultShipAfterDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart.defaultShipAfterDate > 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. > --- ShoppingCart.java:1214, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.setCancelBackOrderDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart.cancelBackOrderDate > 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. > --- ShoppingCart.java:1218, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCart.getCancelBackOrderDate() > may expose internal representation by returning > ShoppingCart.cancelBackOrderDate > 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. > --- ShoppingCart.java:1222, EI_EXPOSE_REP > EI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.getDefaultShipAfterDate() > may expose internal representation by returning > ShoppingCart.defaultShipAfterDate > 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. > --- ShoppingCart.java:1322, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.setLastListRestore(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart.lastListRestore > 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. > --- ShoppingCart.java:1326, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCart.getLastListRestore() may > expose internal representation by returning ShoppingCart.lastListRestore > 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. > --- ShoppingCart.java:1394, BC_UNCONFIRMED_CAST_OF_RETURN_VALUE > BC: Unchecked/unconfirmed cast from java.util.List to java.util.LinkedList of > return value in org.apache.ofbiz.order.shoppingcart.ShoppingCart.clear() > This code performs an unchecked cast of the return value of a method. The > code might be calling the method in such a way that the cast is guaranteed to > be safe, but FindBugs is unable to verify that the cast is safe. Check that > your program logic ensures that this cast will not fail. > --- ShoppingCart.java:1834, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of paymentMethods, which is known to be non-null in > org.apache.ofbiz.order.shoppingcart.ShoppingCart.getCreditCards() > This method contains a redundant check of a known non-null value against the > constant null. > --- ShoppingCart.java:1853, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of paymentMethods, which is known to be non-null in > org.apache.ofbiz.order.shoppingcart.ShoppingCart.getGiftCards() > This method contains a redundant check of a known non-null value against the > constant null. > --- ShoppingCart.java:2070, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE > RCN: Redundant nullcheck of shipGroups, which is known to be non-null in > org.apache.ofbiz.order.shoppingcart.ShoppingCart.setShipGroupShipDatesFromItem(ShoppingCartItem) > This method contains a redundant check of a known non-null value against the > constant null. > --- ShoppingCart.java:3328, NP_NULL_ON_SOME_PATH > NP: Possible null pointer dereference of checkItem in > org.apache.ofbiz.order.shoppingcart.ShoppingCart.clearAllPromotionAdjustments() > There is a branch of statement that, if executed, guarantees that a null > value will be dereferenced, which would generate a NullPointerException when > the code is executed. Of course, the problem might be that the branch or > statement is infeasible and that the null pointer exception can't ever be > executed; deciding that is beyond the ability of FindBugs. > --- ShoppingCart.java:3960, WMI_WRONG_MAP_ITERATOR > WMI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.makeAllOrderItemAttributes(String, > int) makes inefficient use of keySet iterator instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCart.java:3962, UC_USELESS_CONDITION > Condition has no effect > This condition always produces the same result as the value of the involved > variable was narrowed before. Probably something else was meant or condition > can be removed. > --- ShoppingCart.java:4003, DB_DUPLICATE_SWITCH_CLAUSES > DB: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.makeAllOrderAttributes(String, > int) uses the same code for two switch clauses > This method uses the same code to implement two clauses of a switch > statement. This could be a case of duplicate code, but it might also indicate > a coding mistake. > --- ShoppingCart.java:4277, WMI_WRONG_MAP_ITERATOR > WMI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart.createDropShipGroups(LocalDispatcher) > makes inefficient use of keySet iterator instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCart.java:4289, SE_NO_SERIALVERSIONID > SnVI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$BasePriceOrderComparator 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. > --- ShoppingCart.java:4304, RV_NEGATING_RESULT_OF_COMPARETO > RV: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$BasePriceOrderComparator.compare(Object, > Object) negates the return value of > java.math.BigDecimal.compareTo(BigDecimal) > This code negatives the return value of a compareTo or compare method. This > is a questionable or bad programming practice, since if the return value is > Integer.MIN_VALUE, negating the return value won't negate the sign of the > result. You can achieve the same intended result by reversing the order of > the operands rather than by negating the results. > --- ShoppingCart.java:4310, HE_EQUALS_USE_HASHCODE > HE: org.apache.ofbiz.order.shoppingcart.ShoppingCart$BasePriceOrderComparator > defines equals and uses Object.hashCode() > This class overrides equals(Object), but does not override hashCode(), and > inherits the implementation of hashCode() from java.lang.Object (which > returns the identity hash code, an arbitrary value assigned to the object by > the VM). Therefore, the class is very likely to violate the invariant that > equal objects must have equal hashcodes. > If you don't think instances of this class will ever be inserted into a > HashMap/HashTable, the recommended hashCode implementation to use is: > public int hashCode() { > assert false : "hashCode not designed"; > return 42; // any arbitrary constant will do > } > --- ShoppingCart.java:4325, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCart$ShoppingCartItemGroup > 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. > --- ShoppingCart.java:4382, HE_EQUALS_USE_HASHCODE > HE: org.apache.ofbiz.order.shoppingcart.ShoppingCart$ShoppingCartItemGroup > defines equals and uses Object.hashCode() > This class overrides equals(Object), but does not override hashCode(), and > inherits the implementation of hashCode() from java.lang.Object (which > returns the identity hash code, an arbitrary value assigned to the object by > the VM). Therefore, the class is very likely to violate the invariant that > equal objects must have equal hashcodes. > If you don't think instances of this class will ever be inserted into a > HashMap/HashTable, the recommended hashCode implementation to use is: > public int hashCode() { > assert false : "hashCode not designed"; > return 42; // any arbitrary constant will do > } > --- ShoppingCart.java:4383, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS > BC: Equals method for > org.apache.ofbiz.order.shoppingcart.ShoppingCart$ShoppingCartItemGroup > assumes the argument is of type ShoppingCart$ShoppingCartItemGroup > The equals(Object o) method shouldn't make any assumptions about the type of > o. It should simply return false if o is not the same type as this. > --- ShoppingCart.java:4391, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo 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. > --- ShoppingCart.java:4416, WMI_WRONG_MAP_ITERATOR > WMI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.getUsageWeight() > makes inefficient use of keySet iterator instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCart.java:4427, EQ_COMPARETO_USE_OBJECT_EQUALS > Eq: org.apache.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo > defines compareTo(ShoppingCart$ProductPromoUseInfo) and uses Object.equals() > This class defines a compareTo(...) method but inherits its equals() method > from java.lang.Object. Generally, the value of compareTo should return zero > if and only if equals returns true. If this is violated, weird and > unpredictable failures will occur in classes such as PriorityQueue. In Java 5 > the PriorityQueue.remove method uses the compareTo method, while in Java 6 it > uses the equals method. > From the JavaDoc for the compareTo method in the Comparable interface: > It is strongly recommended, but not strictly required that > (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that > implements the Comparable interface and violates this condition should > clearly indicate this fact. The recommended language is "Note: this class has > a natural ordering that is inconsistent with equals." > --- ShoppingCart.java:4431, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo 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. > --- ShoppingCart.java:4604, WMI_WRONG_MAP_ITERATOR > WMI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo.makeItemShipGroupAndAssoc(Delegator, > ShoppingCart, String, boolean) makes inefficient use of keySet iterator > instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCart.java:4686, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo.resetShipBeforeDateIfAfter(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart$CartShipInfo.shipBeforeDate > 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. > --- ShoppingCart.java:4698, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo.resetShipAfterDateIfBefore(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCart$CartShipInfo.shipAfterDate > 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. > --- ShoppingCart.java:4723, SE_NO_SERIALVERSIONID > SnVI: > org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartShipInfo$CartShipItemInfo > 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. > --- ShoppingCart.java:4753, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartPaymentInfo 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. > --- ShoppingCart.java:4948, EQ_COMPARETO_USE_OBJECT_EQUALS > Eq: org.apache.ofbiz.order.shoppingcart.ShoppingCart$CartPaymentInfo defines > compareTo(Object) and uses Object.equals() > This class defines a compareTo(...) method but inherits its equals() method > from java.lang.Object. Generally, the value of compareTo should return zero > if and only if equals returns true. If this is violated, weird and > unpredictable failures will occur in classes such as PriorityQueue. In Java 5 > the PriorityQueue.remove method uses the compareTo method, while in Java 6 it > uses the equals method. > From the JavaDoc for the compareTo method in the Comparable interface: > It is strongly recommended, but not strictly required that > (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that > implements the Comparable interface and violates this condition should > clearly indicate this fact. The recommended language is "Note: this class has > a natural ordering that is inconsistent with equals." > --- ShoppingCart.java:5017, FI_USELESS > FI: org.apache.ofbiz.order.shoppingcart.ShoppingCart.finalize() does nothing > except call super.finalize(); delete it > The only thing this finalize() method does is call the superclass's > finalize() method, making it redundant. Delete it. > --- ShoppingCartEvents.java:74, MS_SHOULD_BE_FINAL > MS: org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.module 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. > --- ShoppingCartEvents.java:361, DM_BOXED_PRIMITIVE_FOR_PARSING > Bx: Boxing/unboxing to parse a primitive > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(HttpServletRequest, > HttpServletResponse) > 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. > --- ShoppingCartEvents.java:412, DLS_DEAD_LOCAL_STORE > DLS: Dead store to reservLength in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(HttpServletRequest, > HttpServletResponse) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartEvents.java:425, DLS_DEAD_LOCAL_STORE > DLS: Dead store to reservPersons in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(HttpServletRequest, > HttpServletResponse) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartEvents.java:479, REC_CATCH_EXCEPTION > REC: Exception is caught when Exception is not thrown in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(HttpServletRequest, > HttpServletResponse) > 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 ... > } > --- ShoppingCartEvents.java:608, GC_UNRELATED_TYPES > GC: String is incompatible with expected argument type > org.apache.ofbiz.entity.GenericValue in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(HttpServletRequest, > HttpServletResponse) > This call to a generic collection method contains an argument with an > incompatible class from that of the collection's parameter (i.e., the type of > the argument is neither a supertype nor a subtype of the corresponding > generic type argument). Therefore, it is unlikely that the collection > contains any objects that are equal to the method argument used here. Most > likely, the wrong value is being passed to the method. > In general, instances of two unrelated classes are not equal. For example, if > the Foo and Bar classes are not related by subtyping, then an instance of Foo > should not be equal to an instance of Bar. Among other issues, doing so will > likely result in an equals method that is not symmetrical. For example, if > you define the Foo class so that a Foo can be equal to a String, your equals > method isn't symmetrical since a String can only be equal to a String. > In rare cases, people do define nonsymmetrical equals methods and still > manage to make their code work. Although none of the APIs document or > guarantee it, it is typically the case that if you check if a > Collection<String> contains a Foo, the equals method of argument (e.g., the > equals method of the Foo class) used to perform the equality checks. > --- ShoppingCartEvents.java:1468, DLS_DEAD_LOCAL_STORE > DLS: Dead store to orderAdjustments in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.loadCartFromOrder(HttpServletRequest, > HttpServletResponse) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartEvents.java:1871, NP_LOAD_OF_KNOWN_NULL_VALUE > NP: Load of known null value in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.bulkAddProducts(HttpServletRequest, > HttpServletResponse) > The variable referenced at this point is known to be null due to an earlier > check against null. Although this is valid, it might be a mistake (perhaps > you intended to refer to a different variable, or perhaps the earlier check > to see if the variable is null should have been a check to see if it was > non-null). > --- ShoppingCartEvents.java:1995, DLS_DEAD_LOCAL_STORE > DLS: Dead store to quantity in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.bulkAddProductsInApprovedOrder(HttpServletRequest, > HttpServletResponse) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartEvents.java:2064, NP_LOAD_OF_KNOWN_NULL_VALUE > NP: Load of known null value in > org.apache.ofbiz.order.shoppingcart.ShoppingCartEvents.bulkAddProductsInApprovedOrder(HttpServletRequest, > HttpServletResponse) > The variable referenced at this point is known to be null due to an earlier > check against null. Although this is valid, it might be a mistake (perhaps > you intended to refer to a different variable, or perhaps the earlier check > to see if the variable is null should have been a check to see if it was > non-null). > --- ShoppingCartHelper.java:68, MS_SHOULD_BE_FINAL > MS: org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.module 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. > --- ShoppingCartHelper.java:210, WMI_WRONG_MAP_ITERATOR > WMI: org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(String, > String, String, String, String, String, String, BigDecimal, BigDecimal, > BigDecimal, Timestamp, BigDecimal, BigDecimal, String, String, Timestamp, > Timestamp, ProductConfigWrapper, String, Map, String) makes inefficient use > of keySet iterator instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCartHelper.java:232, DM_STRING_TOSTRING > Dm: org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(String, > String, String, String, String, String, String, BigDecimal, BigDecimal, > BigDecimal, Timestamp, BigDecimal, BigDecimal, String, String, Timestamp, > Timestamp, ProductConfigWrapper, String, Map, String) invokes toString() > method on a String > Calling String.toString() is just a redundant operation. Just use the String. > --- ShoppingCartHelper.java:272, DM_NUMBER_CTOR > Bx: org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(String, > String, String, String, String, String, String, BigDecimal, BigDecimal, > BigDecimal, Timestamp, BigDecimal, BigDecimal, String, String, Timestamp, > Timestamp, ProductConfigWrapper, String, Map, String) invokes inefficient new > Integer(int) constructor; use Integer.valueOf(int) instead > Using new Integer(int) is guaranteed to always result in a new object whereas > Integer.valueOf(int) allows caching of values to be done by the compiler, > class library, or JVM. Using of cached values avoids object allocation and > the code will be faster. > Values between -128 and 127 are guaranteed to have corresponding cached > instances and using valueOf is approximately 3.5 times faster than using > constructor. For values outside the constant range the performance of both > styles is the same. > Unless the class must be compatible with JVMs predating Java 1.5, use either > autoboxing or the valueOf() method when creating instances of Long, Integer, > Short, Character, and Byte. > --- ShoppingCartHelper.java:435, DLS_DEAD_LOCAL_STORE > DLS: Dead store to piecesIncluded in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCartBulk(String, > String, Map) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartHelper.java:528, DLS_DEAD_LOCAL_STORE > DLS: Dead store to quantity in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCartBulkRequirements(String, > Map) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartHelper.java:636, DM_CONVERT_CASE > Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.deleteFromCart(Map) > 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. > --- ShoppingCartHelper.java:670, DLS_DEAD_LOCAL_STORE > DLS: Dead store to oldQuantity in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.modifyCart(Security, > GenericValue, Map, boolean, String[], Locale) > This instruction assigns a value to a local variable, but the value is not > read or used in any subsequent instruction. Often, this indicates an error, > because the value computed is never used. > Note that Sun's javac compiler often generates dead stores for final local > variables. Because FindBugs is a bytecode-based tool, there is no easy way to > eliminate these false positives. > --- ShoppingCartHelper.java:691, WMI_WRONG_MAP_ITERATOR > WMI: > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.modifyCart(Security, > GenericValue, Map, boolean, String[], Locale) makes inefficient use of keySet > iterator instead of entrySet iterator > This method accesses the value of a Map entry, using a key that was retrieved > from a keySet iterator. It is more efficient to use an iterator on the > entrySet of the map, to avoid the Map.get(key) lookup. > --- ShoppingCartHelper.java:699, DM_CONVERT_CASE > Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.modifyCart(Security, > GenericValue, Map, boolean, String[], Locale) > 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. > --- ShoppingCartHelper.java:892, REC_CATCH_EXCEPTION > REC: Exception is caught when Exception is not thrown in > org.apache.ofbiz.order.shoppingcart.ShoppingCartHelper.modifyCart(Security, > GenericValue, Map, boolean, String[], Locale) > 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 ... > } > --- ShoppingCartItem.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED > Se: The field > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem._parentProduct is > transient but isn't set by deserialization > This class contains a field that is updated at multiple places in the class, > thus it seems to be part of the state of the class. However, since the field > is marked as transient and not set in readObject or readResolve, it will > contain the default value in any deserialized instance of the class. > --- ShoppingCartItem.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED > Se: The field org.apache.ofbiz.order.shoppingcart.ShoppingCartItem._product > is transient but isn't set by deserialization > This class contains a field that is updated at multiple places in the class, > thus it seems to be part of the state of the class. However, since the field > is marked as transient and not set in readObject or readResolve, it will > contain the default value in any deserialized instance of the class. > --- ShoppingCartItem.java:78, MS_SHOULD_BE_FINAL > MS: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.module 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. > --- ShoppingCartItem.java:78, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem 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. > --- ShoppingCartItem.java:81, MS_FINAL_PKGPROTECT > MS: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.attributeNames > should be both final and package protected > A mutable static field could be changed by malicious code or by accident from > another package. The field could be made package protected and/or made final > to avoid this vulnerability. > --- ShoppingCartItem.java:705, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE > RCN: Nullcheck of product at line 706 of value previously dereferenced in new > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem(GenericValue, Map, Map, > String, Locale, String, ShoppingCart$ShoppingCartItemGroup, LocalDispatcher) > A value is checked here to see whether it is null, but this value can't be > null because it was previously dereferenced and if it were null a null > pointer exception would have occurred at the earlier dereference. > Essentially, this code and the previous dereference disagree as to whether > this value is allowed to be null. Either the check is redundant or the > previous dereference is erroneous. > --- ShoppingCartItem.java:835, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.setReservStart(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCartItem.reservStart > 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. > --- ShoppingCartItem.java:1236, EI_EXPOSE_REP > EI: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.getReservStart(BigDecimal) > may expose internal representation by returning ShoppingCartItem.reservStart > 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. > --- ShoppingCartItem.java:1274, IS2_INCONSISTENT_SYNC > IS: Inconsistent synchronization of > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.promoQuantityUsed; > locked 71% 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. > --- ShoppingCartItem.java:1433, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.setShipBeforeDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCartItem.shipBeforeDate > 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. > --- ShoppingCartItem.java:1439, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.getShipBeforeDate() > may expose internal representation by returning > ShoppingCartItem.shipBeforeDate > 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. > --- ShoppingCartItem.java:1444, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.setShipAfterDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCartItem.shipAfterDate > 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. > --- ShoppingCartItem.java:1449, EI_EXPOSE_REP > EI: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.getShipAfterDate() > may expose internal representation by returning ShoppingCartItem.shipAfterDate > 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. > --- ShoppingCartItem.java:1454, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.setCancelBackOrderDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCartItem.cancelBackOrderDate > 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. > --- ShoppingCartItem.java:1459, EI_EXPOSE_REP > EI: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.getCancelBackOrderDate() > may expose internal representation by returning > ShoppingCartItem.cancelBackOrderDate > 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. > --- ShoppingCartItem.java:1464, EI_EXPOSE_REP2 > EI2: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.setEstimatedShipDate(Timestamp) > may expose internal representation by storing an externally mutable object > into ShoppingCartItem.estimatedShipDate > 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. > --- ShoppingCartItem.java:1469, EI_EXPOSE_REP > EI: > org.apache.ofbiz.order.shoppingcart.ShoppingCartItem.getEstimatedShipDate() > may expose internal representation by returning > ShoppingCartItem.estimatedShipDate > 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. > --- ShoppingCartItem.java:2266, EQ_SELF_USE_OBJECT > Eq: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem defines > equals(ShoppingCartItem) method and uses Object.equals(Object) > This class defines a covariant version of the equals() method, but inherits > the normal equals(Object) method defined in the base java.lang.Object class. > The class should probably define a boolean equals(Object) method. > --- ShoppingCartItem.java:2266, HE_EQUALS_USE_HASHCODE > HE: org.apache.ofbiz.order.shoppingcart.ShoppingCartItem defines equals and > uses Object.hashCode() > This class overrides equals(Object), but does not override hashCode(), and > inherits the implementation of hashCode() from java.lang.Object (which > returns the identity hash code, an arbitrary value assigned to the object by > the VM). Therefore, the class is very likely to violate the invariant that > equal objects must have equal hashcodes. > If you don't think instances of this class will ever be inserted into a > HashMap/HashTable, the recommended hashCode implementation to use is: > public int hashCode() { > assert false : "hashCode not designed"; > return 42; // any arbitrary constant will do > } > > --- ShoppingCartServices.java:867, DM_BOOLEAN_CTOR > Dm: > org.apache.ofbiz.order.shoppingcart.ShoppingCartServices.loadCartFromQuote(DispatchContext, > Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) > instead > Creating new instances of java.lang.Boolean wastes memory, since Boolean > objects are immutable and there are only two useful values of this type. Use > the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean > objects instead. > --- WebShoppingCart.java:45, SE_NO_SERIALVERSIONID > SnVI: org.apache.ofbiz.order.shoppingcart.WebShoppingCart 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. -- This message was sent by Atlassian JIRA (v6.4.14#64029)