[ 
https://issues.apache.org/jira/browse/OFBIZ-9638?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dennis Balkir updated OFBIZ-9638:
---------------------------------
    Attachment: OFBIZ-9638_org.apache.ofbiz.service_bugfixes.patch

- Diamond Operators fixed
- removed unnecessary else

class DispatchContext:
- Line 56: it’s not necessary for the class to implement serialVerisonUID
- Line 62, 63: didn’t change anything, this is intentionally programmed like 
this
- Line: 208: removed if phrase, because serviceMap cannot be null at this 
point, since getGlobalServiceMap cannot return null
- Line 270: removed if phrase, since serviceMap cannot be null at this point; 
it is at least declared as a new HashMap, which is not null

class GeneralServiceException:
- Line 34: couldn’t find the circular dependency, maybe there is none?
- Line 63: didn’t remove the if clause. it seems to be unnecessary, but the 
method getNested claims in its javadoc that is returns null, when there is no 
nested exception, although the code itself seems not to return null at all. 
maybe an old version of the javadoc? maybe a mistake?

class GenericAbstractDispatcher:
- added a catch for RuntimeException

class GenericDispatcherFacory:
- Line 32: made the parameter private
- Line 51: made the inner class static
- other two errors were resolved automatically through the previously made 
changes

class GenericResultWaiter:
- Line 29: serialVersionUID doesn’t have to be implemented
- Line 52, 64: seems like this was intended

class ModelParam:
- Line 41: it’s not necessary for the class to implement serialVerisonUID
- implemented a generic equals method, to prevent potential errors
- didn’t implement hashCode, because it is not necessary
- Line 303: it’s not necessary for the class to implement serialVerisonUID

class ModelPermGroup:
- it’s not necessary for the class to implement serialVerisonUID

class ModelPermission:
- it’s not necessary for the class to implement serialVerisonUID
- Line 128: removed
{code:java}
 if (permission == null) {
            Debug.logError("No ModelService found with the name [" + 
permissionServiceName + "]", module);
            return false;
{code}
 because, permission cannot be null at this point
- Line 146: put null inside the logError instead of failMessage, because 
failMessage can only be null at this point; this is easier to read and to 
understand

class ModelService:
- Line 85: it’s not necessary for the class to implement serialVerisonUID
- Line 335: added a if phrase, to check if there is a next object, otherwise 
will throw a new NoSuchElementException; ignored the circular class dependency
- Line 391: changed the method inheritedParameters() to synchronized
- Line 489: removed if phrase, because params cannot be null at this point
- Line 998: removed if phrase, permission cannot be null at this point
- Line 1005: removed else if; unnecessary, thisService cannot be null at this 
point
- Line 1260: removed if phrase, because param cannot be null at this point
- Line 1312: removed implementation of {{documentation}} because now it was 
double implemented
- Line 1304: removed if phrase, because outParam cannot be null at this point

class ModelServiceReader:
- Line 60: it’s not necessary for the class to implement serialVerisonUID
- Line 154: removed unnecessary if phrase, because parameter service cannot be 
null at this point
- also removed the belonging else phrase, because there was no more if
- Line 111: deleted 
{code:java}
if (this.isFromURL) {// utilTimer.timerString("Before getDocumentElement in 
file " + readerURL);
        } else {// utilTimer.timerString("Before getDocumentElement in " + 
handler);
        }
{code}
 because the code in this section was commented out and the whole part had no 
purpose anymore
- Line 442: removed if phrase, because fieldsIter cannot be null at this point

class RunningService:
- Line 59: returned a clone instead of the original object
- Line 63: returned a clone instead of the original object
- Line 72: didn’t implement hashCode() because it seems not to be necessary here

class ServiceContainer:
- Line 38: circular dependency isn’t really an issue
- Line 57: a non static method, which cannot be made static because it is 
overridden, writes in a static field. maybe this can be done somehow different, 
i will ask this in jira
At line 96, dispatcherFactory is used, but it may not have been initialized at 
this point

class ServiceDispatcher:
- Line 73, 76, 77, 78: changed the parameters from protected to private
- Line 102: removed one if phrase, because this if phrase and the own above 
(Line 95) check the same
- Line 122: added an if phrase, to prevent {{NullPointerException}}
- Line 426: added default Locale to {{toUpperCase}}
- Line 464: deleted nullcheck, because this if phrase is inside another if 
phrase, which already has the same nullcheck after which nothing is changed in 
{{errMsg}}
- Line 464: this findbugs-error only occurs, because inside of the if phrase is 
no code to be executed
- Line 1026: implemented hashCode() in class RunningService; it uses the 
Object.hashCode() since the developers didn’t think RunningService needed its 
own hashCode() and with this, it specifically uses the hashCode() method from 
object, which is less confusing for the reader and won’t produce findbugs 
errors in multiple classes 

class ServiceSynchroniztion:
- Line 54: removed if phrase, because getInstance() returns a not null variable 
or throws an exception
- Line 59: removed if phrase, because getInstance() returns a not null variable 
or throws an exception

class ServiceUtil:
- Line 555: added nullcheck for job, removed nullcheck for cancelDate, which 
cannot be null, if job isn’t null
- Line 562: changed {{+ job}} to {{+ null}} because job can only be null at 
this point
- Line 592: removed the declaration of cancelDate, which is only used to do a 
nullcheck and made a nullcheck with {{job}} instead
- Line 638: added a nullcheck for args, which prevents a random 
NullpointerException and instead throws a IllegalArgumentException

class ServiceXaWrapper:
- added a default case, which warns about a false input type


> [FB] Package org.apache.ofbiz.service
> -------------------------------------
>
>                 Key: OFBIZ-9638
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-9638
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: framework
>    Affects Versions: Trunk
>            Reporter: Dennis Balkir
>            Priority: Minor
>         Attachments: OFBIZ-9638_org.apache.ofbiz.service_bugfixes.patch
>
>
> - DispatchContext.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
> Se: The field org.apache.ofbiz.service.DispatchContext.loader 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.
> - DispatchContext.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
> Se: The field org.apache.ofbiz.service.DispatchContext.dispatcher 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.
> - DispatchContext.java:56, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.DispatchContext 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.
> - DispatchContext.java:209, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of serviceMap, which is known to be non-null in 
> org.apache.ofbiz.service.DispatchContext.getModelService(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - DispatchContext.java:273, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of serviceMap, which is known to be non-null in 
> org.apache.ofbiz.service.DispatchContext.getGlobalServiceMap()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - GeneralServiceException.java:63, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of 
> org.apache.ofbiz.base.util.GeneralException.getNested(), which is known to be 
> non-null in 
> org.apache.ofbiz.service.GeneralServiceException.returnError(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - GenericAbstractDispatcher.java:86, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.service.GenericAbstractDispatcher.schedule(String, String, 
> String, Map, long, int, int, int, long, int)
> 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 ...
>   }
> - GenericDispatcherFactory.java:32, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.GenericDispatcherFactory.ecasDisabled should be 
> package protected
> A mutable static field could be changed by malicious code or by accident. The 
> field could be made package protected to avoid this vulnerability.
> - GenericDispatcherFactory.java:49, SIC_INNER_SHOULD_BE_STATIC
> SIC: Should 
> org.apache.ofbiz.service.GenericDispatcherFactory$GenericDispatcher be a 
> _static_ inner class?
> This class is an inner class, but does not use its embedded reference to the 
> object which created it.  This reference makes the instances of the class 
> larger, and may keep the reference to the creator object alive longer than 
> necessary.  If possible, the class should be made static.
> - GenericDispatcherFactory.java:72, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD
> ST: Write to static field 
> org.apache.ofbiz.service.GenericDispatcherFactory.ecasDisabled from instance 
> method 
> org.apache.ofbiz.service.GenericDispatcherFactory$GenericDispatcher.disableEcas()
> This instance method writes to a static field. This is tricky to get correct 
> if multiple instances are being manipulated, and generally bad practice.
> - GenericDispatcherFactory.java:77, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD
> ST: Write to static field 
> org.apache.ofbiz.service.GenericDispatcherFactory.ecasDisabled from instance 
> method 
> org.apache.ofbiz.service.GenericDispatcherFactory$GenericDispatcher.enableEcas()
> This instance method writes to a static field. This is tricky to get correct 
> if multiple instances are being manipulated, and generally bad practice.
> - GenericResultWaiter.java:29, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.GenericResultWaiter 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.
> - GenericResultWaiter.java:52, NO_NOTIFY_NOT_NOTIFYALL
> No: Using notify rather than notifyAll in 
> org.apache.ofbiz.service.GenericResultWaiter.receiveResult(Map)
> This method calls notify() rather than notifyAll().  Java monitors are often 
> used for multiple conditions.  Calling notify() only wakes up one thread, 
> meaning that the thread woken up might not be the one waiting for the 
> condition that the caller just satisfied.
> - GenericResultWaiter.java:64, NO_NOTIFY_NOT_NOTIFYALL
> No: Using notify rather than notifyAll in 
> org.apache.ofbiz.service.GenericResultWaiter.receiveThrowable(Throwable)
> This method calls notify() rather than notifyAll().  Java monitors are often 
> used for multiple conditions.  Calling notify() only wakes up one thread, 
> meaning that the thread woken up might not be the one waiting for the 
> condition that the caller just satisfied.
> - ModelParam.java:41, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelParam 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.
> - ModelParam.java:209, HE_EQUALS_USE_HASHCODE
> HE: org.apache.ofbiz.service.ModelParam 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
>   }
> - ModelParam.java:209, EQ_SELF_USE_OBJECT
> Eq: org.apache.ofbiz.service.ModelParam defines equals(ModelParam) 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.
> - ModelParam.java:297, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelParam$ModelParamValidator 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.
> - ModelPermGroup.java:32, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelPermGroup 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.
> - ModelPermission.java:35, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelPermission 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.
> - ModelPermission.java:108, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.service.ModelPermission.evalRoleMember(GenericValue)
> 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).
> - ModelPermission.java:129, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of permission, which is known to be non-null in 
> org.apache.ofbiz.service.ModelPermission.evalPermissionService(ModelService, 
> DispatchContext, Map)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelPermission.java:150, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.service.ModelPermission.evalPermissionService(ModelService, 
> DispatchContext, Map)
> 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).
> - ModelService.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.ModelService defines non-transient 
> non-serializable instance field implServices
> This Serializable class defines a non-primitive instance field which is 
> neither transient, Serializable, or java.lang.Object, and does not appear to 
> implement the Externalizable interface or the readObject() and writeObject() 
> methods.  Objects of this class will not be deserialized correctly if a 
> non-Serializable object is stored in this field.
> - ModelService.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.ModelService defines non-transient 
> non-serializable instance field internalGroup
> This Serializable class defines a non-primitive instance field which is 
> neither transient, Serializable, or java.lang.Object, and does not appear to 
> implement the Externalizable interface or the readObject() and writeObject() 
> methods.  Objects of this class will not be deserialized correctly if a 
> non-Serializable object is stored in this field.
> - ModelService.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.ModelService defines non-transient 
> non-serializable instance field metrics
> This Serializable class defines a non-primitive instance field which is 
> neither transient, Serializable, or java.lang.Object, and does not appear to 
> implement the Externalizable interface or the readObject() and writeObject() 
> methods.  Objects of this class will not be deserialized correctly if a 
> non-Serializable object is stored in this field.
> - ModelService.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.ModelService defines non-transient 
> non-serializable instance field notifications
> This Serializable class defines a non-primitive instance field which is 
> neither transient, Serializable, or java.lang.Object, and does not appear to 
> implement the Externalizable interface or the readObject() and writeObject() 
> methods.  Objects of this class will not be deserialized correctly if a 
> non-Serializable object is stored in this field.
> - ModelService.java:84, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelService 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.
> - ModelService.java:329, IT_NO_SUCH_ELEMENT
> It: org.apache.ofbiz.service.ModelService$1$1.next() can't throw 
> NoSuchElementException
> This class implements the java.util.Iterator interface.  However, its next() 
> method is not capable of throwing java.util.NoSuchElementException.  The 
> next() method should be changed so it throws NoSuchElementException if is 
> called when there are no more elements to return.
> - ModelService.java:383, IS2_INCONSISTENT_SYNC
> IS: Inconsistent synchronization of 
> org.apache.ofbiz.service.ModelService.inheritedParameters; locked 50% of time
> The fields of this class appear to be accessed inconsistently with respect to 
> synchronization.  This bug report indicates that the bug pattern detector 
> judged that
> The class contains a mix of locked and unlocked accesses,
> The class is not annotated as javax.annotation.concurrent.NotThreadSafe,
> At least one locked access was performed by one of the class's own methods, 
> and
> The number of unsynchronized field accesses (reads and writes) was no more 
> than one third of all accesses, with writes being weighed twice as high as 
> reads
> A typical bug matching this bug pattern is forgetting to synchronize one of 
> the methods in a class that is intended to be thread-safe.
> You can select the nodes labeled "Unsynchronized access" to show the code 
> locations where the detector believed that a field was accessed without 
> synchronization.
> Note that there are various sources of inaccuracy in this detector; for 
> example, the detector cannot statically detect all situations in which a lock 
> is held.  Also, even when the detector is accurate in distinguishing locked 
> vs. unlocked accesses, the code in question may still be correct.
> - ModelService.java:480, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of params, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.updateDefaultValues(Map, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelService.java:991, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of permission, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.evalPermission(DispatchContext, Map)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelService.java:998, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of thisService, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.evalPermission(DispatchContext, Map)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelService.java:1141, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of model, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.interfaceUpdate(DispatchContext)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelService.java:1245, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of inParam, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.getWSDL(Definition, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelService.java:1291, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of outParam, which is known to be non-null in 
> org.apache.ofbiz.service.ModelService.getWSDL(Definition, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelServiceReader.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.ModelServiceReader defines non-transient 
> non-serializable instance field delegator
> This Serializable class defines a non-primitive instance field which is 
> neither transient, Serializable, or java.lang.Object, and does not appear to 
> implement the Externalizable interface or the readObject() and writeObject() 
> methods.  Objects of this class will not be deserialized correctly if a 
> non-Serializable object is stored in this field.
> - ModelServiceReader.java:60, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.ModelServiceReader 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.
> - ModelServiceReader.java:111, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.service.ModelServiceReader.getModelServices()
> This method contains a useless control flow statement, where control flow 
> continues onto the same place regardless of whether or not the branch is 
> taken. For example, this is caused by having an empty statement block for an 
> if statement:
>     if (argv.length == 0) {
>     // TODO: handle this case
>     }
> - ModelServiceReader.java:154, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of service, which is known to be non-null in 
> org.apache.ofbiz.service.ModelServiceReader.getModelServices()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ModelServiceReader.java:450, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of fieldsIter, which is known to be non-null in 
> org.apache.ofbiz.service.ModelServiceReader.createAutoAttrDef(Element, 
> ModelService)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - RunningService.java:59, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.RunningService.getStartStamp() may expose 
> internal representation by returning RunningService.startStamp
> 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.
> - RunningService.java:63, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.RunningService.getEndStamp() may expose internal 
> representation by returning RunningService.endStamp
> 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.
> - RunningService.java:72, HE_EQUALS_USE_HASHCODE
> HE: org.apache.ofbiz.service.RunningService 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
>   }
> - ServiceContainer.java:57, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD
> ST: Write to static field 
> org.apache.ofbiz.service.ServiceContainer.dispatcherFactory from instance 
> method org.apache.ofbiz.service.ServiceContainer.init(List, String, String)
> This instance method writes to a static field. This is tricky to get correct 
> if multiple instances are being manipulated, and generally bad practice.
> - ServiceDispatcher.java:73, MS_SHOULD_BE_FINAL
> MS: org.apache.ofbiz.service.ServiceDispatcher.dispatchers 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.
> - ServiceDispatcher.java:76, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.ServiceDispatcher.enableJM should be package 
> protected
> A mutable static field could be changed by malicious code or by accident. The 
> field could be made package protected to avoid this vulnerability.
> - ServiceDispatcher.java:77, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.ServiceDispatcher.enableJMS should be package 
> protected
> A mutable static field could be changed by malicious code or by accident. The 
> field could be made package protected to avoid this vulnerability.
> - ServiceDispatcher.java:78, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.ServiceDispatcher.enableSvcs should be package 
> protected
> A mutable static field could be changed by malicious code or by accident. The 
> field could be made package protected to avoid this vulnerability.
> - ServiceDispatcher.java:118, NP_NULL_ON_SOME_PATH
> NP: Possible null pointer dereference of delegator in new 
> org.apache.ofbiz.service.ServiceDispatcher(Delegator, boolean, boolean)
> 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.
> - ServiceDispatcher.java:425, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.service.ServiceDispatcher.runSync(String, ModelService, Map, 
> boolean)
> 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.
> - ServiceDispatcher.java:463, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of errMsg, which is known to be non-null in 
> org.apache.ofbiz.service.ServiceDispatcher.runSync(String, ModelService, Map, 
> boolean)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ServiceDispatcher.java:464, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.service.ServiceDispatcher.runSync(String, ModelService, Map, 
> boolean)
> This method contains a useless control flow statement, where control flow 
> continues onto the same place regardless of whether or not the branch is 
> taken. For example, this is caused by having an empty statement block for an 
> if statement:
>     if (argv.length == 0) {
>     // TODO: handle this case
>     }
> - ServiceDispatcher.java:1025, HE_USE_OF_UNHASHABLE_CLASS
> HE: org.apache.ofbiz.service.RunningService doesn't define a hashCode() 
> method but is used in a hashed data structure in 
> org.apache.ofbiz.service.ServiceDispatcher.logService(String, ModelService, 
> int)
> A class defines an equals(Object) method but not a hashCode() method, and 
> thus doesn't fulfill the requirement that equal objects have equal hashCodes. 
> An instance of this class is used in a hash data structure, making the need 
> to fix this problem of highest importance.
> - ServiceSynchronization.java:55, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of sync, which is known to be non-null in 
> org.apache.ofbiz.service.ServiceSynchronization.registerCommitService(DispatchContext,
>  String, String, Map, boolean, boolean)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ServiceSynchronization.java:62, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of sync, which is known to be non-null in 
> org.apache.ofbiz.service.ServiceSynchronization.registerRollbackService(DispatchContext,
>  String, String, Map, boolean, boolean)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ServiceUtil.java:557, NP_NULL_ON_SOME_PATH
> NP: Possible null pointer dereference of job in 
> org.apache.ofbiz.service.ServiceUtil.cancelJob(DispatchContext, Map)
> 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.
> - ServiceUtil.java:595, NP_NULL_ON_SOME_PATH
> NP: Possible null pointer dereference of job in 
> org.apache.ofbiz.service.ServiceUtil.cancelJobRetries(DispatchContext, Map)
> 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.
> - ServiceUtil.java:648, NP_NULL_PARAM_DEREF
> NP: Null passed for nonnull parameter of 
> org.apache.ofbiz.base.util.UtilMisc.toMap(Object[]) in 
> org.apache.ofbiz.service.ServiceUtil.makeContext(Object[])
> This method call passes a null value for a non-null method parameter. Either 
> the parameter is annotated as a parameter that should always be non-null, or 
> analysis has shown that it will always be dereferenced.
> - ServiceXaWrapper.java:258, SF_SWITCH_NO_DEFAULT
> SF: Switch statement found in 
> org.apache.ofbiz.service.ServiceXaWrapper.runService(String, Map, boolean, 
> int, int) where default case is missing
> This method contains a switch statement where default case is missing. 
> Usually you need to provide a default case.
> Because the analysis only looks at the generated bytecode, this warning can 
> be incorrect triggered if the default case is at the end of the switch 
> statement and the switch statement doesn't contain break statements for other 
> cases.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to