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