[ 
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)

Reply via email to