[jira] [Created] (OFBIZ-9560) [FB] Package org.apache.ofbiz.base.component

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9560:


 Summary: [FB] Package org.apache.ofbiz.base.component
 Key: OFBIZ-9560
 URL: https://issues.apache.org/jira/browse/OFBIZ-9560
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


ComponentConfig.java:270, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getFullLocation(String, String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:291, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getRootLocation(String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:299, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getStream(String, String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:307, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getURL(String, String, String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:330, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.isFileResourceLoader(String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:704, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of rh, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig$KeystoreInfo.getKeyStore()

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:839, DM_CONVERT_CASE

- Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in new 
org.apache.ofbiz.base.component.ComponentConfig$WebappInfo(ComponentConfig, 
Element)

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.

ComponentConfig.java:925, EI_EXPOSE_REP

- EI: 
org.apache.ofbiz.base.component.ComponentConfig$WebappInfo.getBasePermission() 
may expose internal representation by returning 
ComponentConfig$WebappInfo.basePermission

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.

ComponentResourceHandler.java:39, SE_NO_SERIALVERSIONID

- SnVI: org.apache.ofbiz.base.component.ComponentResourceHandler 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)


[jira] [Created] (OFBIZ-9561) [FB] Package org.apache.ofbiz.base.component

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9561:


 Summary: [FB] Package org.apache.ofbiz.base.component
 Key: OFBIZ-9561
 URL: https://issues.apache.org/jira/browse/OFBIZ-9561
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


ComponentConfig.java:270, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getFullLocation(String, String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:291, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getRootLocation(String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:299, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getStream(String, String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:307, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.getURL(String, String, String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:330, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of cc, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig.isFileResourceLoader(String, 
String)

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:704, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

- RCN: Redundant nullcheck of rh, which is known to be non-null in 
org.apache.ofbiz.base.component.ComponentConfig$KeystoreInfo.getKeyStore()

This method contains a redundant check of a known non-null value against the 
constant null.

ComponentConfig.java:839, DM_CONVERT_CASE

- Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in new 
org.apache.ofbiz.base.component.ComponentConfig$WebappInfo(ComponentConfig, 
Element)

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.

ComponentConfig.java:925, EI_EXPOSE_REP

- EI: 
org.apache.ofbiz.base.component.ComponentConfig$WebappInfo.getBasePermission() 
may expose internal representation by returning 
ComponentConfig$WebappInfo.basePermission

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.

ComponentResourceHandler.java:39, SE_NO_SERIALVERSIONID

- SnVI: org.apache.ofbiz.base.component.ComponentResourceHandler 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)


[jira] [Closed] (OFBIZ-9561) [FB] Package org.apache.ofbiz.base.component

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir closed OFBIZ-9561.

Resolution: Duplicate

Sorry for the accidental duplication, I misclicked the create button the first 
time.

> [FB] Package org.apache.ofbiz.base.component
> 
>
> Key: OFBIZ-9561
> URL: https://issues.apache.org/jira/browse/OFBIZ-9561
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> ComponentConfig.java:270, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getFullLocation(String, 
> String, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:291, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getRootLocation(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:299, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getStream(String, String, 
> String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:307, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getURL(String, String, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:330, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.isFileResourceLoader(String, 
> String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:704, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of rh, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig$KeystoreInfo.getKeyStore()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:839, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> new 
> org.apache.ofbiz.base.component.ComponentConfig$WebappInfo(ComponentConfig, 
> Element)
> 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.
> ComponentConfig.java:925, EI_EXPOSE_REP
> - EI: 
> org.apache.ofbiz.base.component.ComponentConfig$WebappInfo.getBasePermission()
>  may expose internal representation by returning 
> ComponentConfig$WebappInfo.basePermission
> 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.
> ComponentResourceHandler.java:39, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.base.component.ComponentResourceHandler 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)


[jira] [Updated] (OFBIZ-9560) [FB] Package org.apache.ofbiz.base.component

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9560:
-
Attachment: OFBIZ-No_org.apache.ofbiz.base.component_bugfixes.patch

- deleted all unnecessary nullchecks
- deleted one unnecessary else-block
- did not fix the serialVersionUID issue, because it is not important to 
implement it in this class 

> [FB] Package org.apache.ofbiz.base.component
> 
>
> Key: OFBIZ-9560
> URL: https://issues.apache.org/jira/browse/OFBIZ-9560
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-No_org.apache.ofbiz.base.component_bugfixes.patch
>
>
> ComponentConfig.java:270, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getFullLocation(String, 
> String, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:291, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getRootLocation(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:299, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getStream(String, String, 
> String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:307, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.getURL(String, String, String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:330, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of cc, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig.isFileResourceLoader(String, 
> String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:704, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of rh, which is known to be non-null in 
> org.apache.ofbiz.base.component.ComponentConfig$KeystoreInfo.getKeyStore()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> ComponentConfig.java:839, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> new 
> org.apache.ofbiz.base.component.ComponentConfig$WebappInfo(ComponentConfig, 
> Element)
> 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.
> ComponentConfig.java:925, EI_EXPOSE_REP
> - EI: 
> org.apache.ofbiz.base.component.ComponentConfig$WebappInfo.getBasePermission()
>  may expose internal representation by returning 
> ComponentConfig$WebappInfo.basePermission
> 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.
> ComponentResourceHandler.java:39, SE_NO_SERIALVERSIONID
> - SnVI: org.apache.ofbiz.base.component.ComponentResourceHandler 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)


[jira] [Created] (OFBIZ-9562) [FB] Package org.apache.ofbiz.base.concurrent

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9562:


 Summary: [FB] Package org.apache.ofbiz.base.concurrent
 Key: OFBIZ-9562
 URL: https://issues.apache.org/jira/browse/OFBIZ-9562
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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."



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


[jira] [Updated] (OFBIZ-9562) [FB] Package org.apache.ofbiz.base.concurrent

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9562:
-
Description: 
ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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 
{{(a.compareTo(b)==0) == (a.equals(b))}}. 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."

  was:
ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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."


> [FB] Package org.apache.ofbiz.base.concurrent
> -
>
> Key: OFBIZ-9562
> URL: https://issues.apache.org/jira/browse/OFBIZ-9562
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
> Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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 
> {{(a.compareTo(b)==0) == (a.equals(b))}}. 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."



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


[jira] [Updated] (OFBIZ-9562) [FB] Package org.apache.ofbiz.base.concurrent

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9562:
-
Description: 
ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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."

  was:
ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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."


> [FB] Package org.apache.ofbiz.base.concurrent
> -
>
> Key: OFBIZ-9562
> URL: https://issues.apache.org/jira/browse/OFBIZ-9562
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
> Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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."



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


[jira] [Updated] (OFBIZ-9562) [FB] Package org.apache.ofbiz.base.concurrent

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9562:
-
Attachment: OFBIZ-No_org.apache.ofbiz.base.concurrent_bugfixes.patch

- implemented equals method to fix potential problems 
- implemented hashCode method because I implemented equals method
- implemented timeDiff method to keep the code DRY

> [FB] Package org.apache.ofbiz.base.concurrent
> -
>
> Key: OFBIZ-9562
> URL: https://issues.apache.org/jira/browse/OFBIZ-9562
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-No_org.apache.ofbiz.base.concurrent_bugfixes.patch
>
>
> ExecutionPool.java:122, EQ_COMPARETO_USE_OBJECT_EQUALS
> Eq: org.apache.ofbiz.base.concurrent.ExecutionPool$Pulse 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 
> {{(a.compareTo(b)==0) == (a.equals(b))}}. 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."



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


[jira] [Created] (OFBIZ-9563) [FB] Package org.apache.ofbiz.base.container

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9563:


 Summary: [FB] Package org.apache.ofbiz.base.container
 Key: OFBIZ-9563
 URL: https://issues.apache.org/jira/browse/OFBIZ-9563
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- ComponentContainer.java:140, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of componentLoadFile, which is known to be non-null in 
org.apache.ofbiz.base.container.ComponentContainer.loadComponentDirectory(String)

This method contains a redundant check of a known non-null value against the 
constant null.

- ComponentContainer.java:165, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of componentsToLoad, which is known to be non-null in 
org.apache.ofbiz.base.container.ComponentContainer.loadComponentsInDirectoryUsingLoadFile(File,
 File)

This method contains a redundant check of a known non-null value against the 
constant null.

- ComponentContainer.java:187, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
NP: Possible null pointer dereference in 
org.apache.ofbiz.base.container.ComponentContainer.loadComponentsInDirectory(File)
 due to return value of called method

The return value from a method is dereferenced without a null check, and the 
return value of that method is one that should generally be checked for null. 
This may lead to a NullPointerException when the code is executed.

- ContainerConfig.java:102, DLS_DEAD_LOCAL_STORE
DLS: Dead store to num in 
org.apache.ofbiz.base.container.ContainerConfig.getPropertyValue(ContainerConfig$Configuration,
 String, int)

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.

- ContainerConfig.java:135, DLS_DEAD_LOCAL_STORE
DLS: Dead store to num in 
org.apache.ofbiz.base.container.ContainerConfig.getPropertyValue(ContainerConfig$Configuration$Property,
 String, int)

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.



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


[jira] [Updated] (OFBIZ-9563) [FB] Package org.apache.ofbiz.base.container

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9563:
-
Attachment: OFBIZ-No_org.apache.ofbiz.base.container_bugfixes.patch

ComponentContainer.java
- deleted multiple unnecessary nullchecks
- added nullcheck, because {{sortedComponentNames}} could possibly be assigned 
as null through the method {{list()}}

ContainerConfig.java
- deleted multiple unnecessary else-blocks
- deleted two initializations to shorten the code and return the value directly

> [FB] Package org.apache.ofbiz.base.container
> 
>
> Key: OFBIZ-9563
> URL: https://issues.apache.org/jira/browse/OFBIZ-9563
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-No_org.apache.ofbiz.base.container_bugfixes.patch
>
>
> - ComponentContainer.java:140, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of componentLoadFile, which is known to be non-null 
> in 
> org.apache.ofbiz.base.container.ComponentContainer.loadComponentDirectory(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ComponentContainer.java:165, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of componentsToLoad, which is known to be non-null 
> in 
> org.apache.ofbiz.base.container.ComponentContainer.loadComponentsInDirectoryUsingLoadFile(File,
>  File)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - ComponentContainer.java:187, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
> NP: Possible null pointer dereference in 
> org.apache.ofbiz.base.container.ComponentContainer.loadComponentsInDirectory(File)
>  due to return value of called method
> The return value from a method is dereferenced without a null check, and the 
> return value of that method is one that should generally be checked for null. 
> This may lead to a NullPointerException when the code is executed.
> - ContainerConfig.java:102, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to num in 
> org.apache.ofbiz.base.container.ContainerConfig.getPropertyValue(ContainerConfig$Configuration,
>  String, int)
> 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.
> - ContainerConfig.java:135, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to num in 
> org.apache.ofbiz.base.container.ContainerConfig.getPropertyValue(ContainerConfig$Configuration$Property,
>  String, int)
> 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.



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


[jira] [Created] (OFBIZ-9564) [FB] Package org.apache.ofbiz.base.lang

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9564:


 Summary: [FB] Package org.apache.ofbiz.base.lang
 Key: OFBIZ-9564
 URL: https://issues.apache.org/jira/browse/OFBIZ-9564
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- ComparableRange.java:70, HE_EQUALS_USE_HASHCODE

HE: org.apache.ofbiz.base.lang.ComparableRange 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
  }

- ComparableRange.java:76, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.base.lang.ComparableRange.equals(Object)

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 ...
  }



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


[jira] [Updated] (OFBIZ-9564) [FB] Package org.apache.ofbiz.base.lang

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9564:
-
Attachment: OFBIZ-9564_org.apache.ofbiz.base.lang_bugfixes.patch

- implemented method {{hashCode()}} because method {{equals()}} was implemented
- changed {{Exception}} to {{RuntimeException}} because no exception was thrown 
and the only occuring exception coud be a runtimeexception

> [FB] Package org.apache.ofbiz.base.lang
> ---
>
> Key: OFBIZ-9564
> URL: https://issues.apache.org/jira/browse/OFBIZ-9564
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9564_org.apache.ofbiz.base.lang_bugfixes.patch
>
>
> - ComparableRange.java:70, HE_EQUALS_USE_HASHCODE
> HE: org.apache.ofbiz.base.lang.ComparableRange 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
>   }
> - ComparableRange.java:76, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.base.lang.ComparableRange.equals(Object)
> 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 ...
>   }



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


[jira] [Created] (OFBIZ-9565) [FB] Package org.apache.ofbiz.base.lang.test

2017-08-09 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9565:


 Summary: [FB] Package org.apache.ofbiz.base.lang.test
 Key: OFBIZ-9565
 URL: https://issues.apache.org/jira/browse/OFBIZ-9565
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- ComparableRangeTests.java:82, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
Return value of method without side effect is ignored

This code calls a method and ignores the return value. However our analysis 
shows that the method (including its implementations in subclasses if any) does 
not produce any effect other than return value. Thus this call can be removed.

We are trying to reduce the false positives as much as possible, but in some 
cases this warning might be wrong. Common false-positive cases include:

- The method is designed to be overridden and produce a side effect in other 
projects which are out of the scope of the analysis.

- The method is called to trigger the class loading which may have a side 
effect.

- The method is called just to get some exception.

If you feel that our assumption is incorrect, you can use a @CheckReturnValue 
annotation to instruct FindBugs that ignoring the return value of this method 
is acceptable.



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


[jira] [Updated] (OFBIZ-9565) [FB] Package org.apache.ofbiz.base.lang.test

2017-08-09 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9565:
-
Attachment: OFBIZ-9565_org.apache.ofbiz.base.lang.test_bugfixes.patch

- fixed some diamond operators
- didn't fix the main issue (listed in description), because the method which 
was used, was only used for testing, which makes the issue irrelevant


> [FB] Package org.apache.ofbiz.base.lang.test
> 
>
> Key: OFBIZ-9565
> URL: https://issues.apache.org/jira/browse/OFBIZ-9565
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9565_org.apache.ofbiz.base.lang.test_bugfixes.patch
>
>
> - ComparableRangeTests.java:82, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
> Return value of method without side effect is ignored
> This code calls a method and ignores the return value. However our analysis 
> shows that the method (including its implementations in subclasses if any) 
> does not produce any effect other than return value. Thus this call can be 
> removed.
> We are trying to reduce the false positives as much as possible, but in some 
> cases this warning might be wrong. Common false-positive cases include:
> - The method is designed to be overridden and produce a side effect in other 
> projects which are out of the scope of the analysis.
> - The method is called to trigger the class loading which may have a side 
> effect.
> - The method is called just to get some exception.
> If you feel that our assumption is incorrect, you can use a @CheckReturnValue 
> annotation to instruct FindBugs that ignoring the return value of this method 
> is acceptable.



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


[jira] [Created] (OFBIZ-9567) [FB] Package org.apache.ofbiz.base.metrics

2017-08-10 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9567:


 Summary: [FB] Package org.apache.ofbiz.base.metrics
 Key: OFBIZ-9567
 URL: https://issues.apache.org/jira/browse/OFBIZ-9567
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- MetricsFactory.java:236, ICAST_IDIV_CAST_TO_DOUBLE
ICAST: Integral division result cast to double or float in 
org.apache.ofbiz.base.metrics.MetricsFactory$MetricsImpl.recordServiceRate(int, 
long)

This code casts the result of an integral division (e.g., int or long division) 
operation to double or float. Doing division on integers truncates the result 
to the integer value closest to zero. The fact that the result was cast to 
double suggests that this precision should have been retained. What was 
probably meant was to cast one or both of the operands to double before 
performing the division. Here is an example:

int x = 2;
int y = 5;
// Wrong: yields result 0.0
double value1 =  x / y;

// Right: yields result 0.4
double value2 =  x / (double) y;



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


[jira] [Updated] (OFBIZ-9567) [FB] Package org.apache.ofbiz.base.metrics

2017-08-10 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9567:
-
Attachment: OFBIZ-9567_org.apache.ofbiz.base.metrics_bugfixes.patch

- changed a division with two {{long}} variables whose result was casted into a 
{{double}}; now it performs a proper {{double}} division

> [FB] Package org.apache.ofbiz.base.metrics
> --
>
> Key: OFBIZ-9567
> URL: https://issues.apache.org/jira/browse/OFBIZ-9567
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9567_org.apache.ofbiz.base.metrics_bugfixes.patch
>
>
> - MetricsFactory.java:236, ICAST_IDIV_CAST_TO_DOUBLE
> ICAST: Integral division result cast to double or float in 
> org.apache.ofbiz.base.metrics.MetricsFactory$MetricsImpl.recordServiceRate(int,
>  long)
> This code casts the result of an integral division (e.g., int or long 
> division) operation to double or float. Doing division on integers truncates 
> the result to the integer value closest to zero. The fact that the result was 
> cast to double suggests that this precision should have been retained. What 
> was probably meant was to cast one or both of the operands to double before 
> performing the division. Here is an example:
> int x = 2;
> int y = 5;
> // Wrong: yields result 0.0
> double value1 =  x / y;
> // Right: yields result 0.4
> double value2 =  x / (double) y;



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


[jira] [Commented] (OFBIZ-9567) [FB] Package org.apache.ofbiz.base.metrics

2017-08-10 Thread Dennis Balkir (JIRA)

[ 
https://issues.apache.org/jira/browse/OFBIZ-9567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16121361#comment-16121361
 ] 

Dennis Balkir commented on OFBIZ-9567:
--

Hi Jacques,
I removed the parenthesis because I learned at Uni that they're logically not 
necessary. You can of course reimplement them, if you find it easier to read, I 
didn't want to change your style of programming, that's just the way I learned 
it and the way I'm doing it everytime :D

> [FB] Package org.apache.ofbiz.base.metrics
> --
>
> Key: OFBIZ-9567
> URL: https://issues.apache.org/jira/browse/OFBIZ-9567
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Assignee: Jacques Le Roux
>Priority: Minor
> Attachments: OFBIZ-9567_org.apache.ofbiz.base.metrics_bugfixes.patch
>
>
> - MetricsFactory.java:236, ICAST_IDIV_CAST_TO_DOUBLE
> ICAST: Integral division result cast to double or float in 
> org.apache.ofbiz.base.metrics.MetricsFactory$MetricsImpl.recordServiceRate(int,
>  long)
> This code casts the result of an integral division (e.g., int or long 
> division) operation to double or float. Doing division on integers truncates 
> the result to the integer value closest to zero. The fact that the result was 
> cast to double suggests that this precision should have been retained. What 
> was probably meant was to cast one or both of the operands to double before 
> performing the division. Here is an example:
> int x = 2;
> int y = 5;
> // Wrong: yields result 0.0
> double value1 =  x / y;
> // Right: yields result 0.4
> double value2 =  x / (double) y;



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


[jira] [Created] (OFBIZ-9573) [FB] Package org.apache.ofbiz.base.start

2017-08-14 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9573:


 Summary: [FB] Package org.apache.ofbiz.base.start
 Key: OFBIZ-9573
 URL: https://issues.apache.org/jira/browse/OFBIZ-9573
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AdminClient.java:77, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.base.start.AdminClient.sendSocketCommand(AdminServer$OfbizSocketCommand,
 Config): new java.io.PrintWriter(OutputStream, boolean)

Found a call to a method which will perform a byte to String (or String to 
byte) conversion, and will assume that the default platform encoding is 
suitable. This will cause the application behaviour to vary between platforms. 
Use an alternative API and specify a charset name or Charset object explicitly.

- AdminClient.java:78, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.base.start.AdminClient.sendSocketCommand(AdminServer$OfbizSocketCommand,
 Config): new java.io.InputStreamReader(InputStream)

Found a call to a method which will perform a byte to String (or String to 
byte) conversion, and will assume that the default platform encoding is 
suitable. This will cause the application behaviour to vary between platforms. 
Use an alternative API and specify a charset name or Charset object explicitly.

- AdminServer.java:84, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.base.start.AdminServer.processClientRequest(Socket, List, 
AtomicReference): new java.io.InputStreamReader(InputStream)

Found a call to a method which will perform a byte to String (or String to 
byte) conversion, and will assume that the default platform encoding is 
suitable. This will cause the application behaviour to vary between platforms. 
Use an alternative API and specify a charset name or Charset object explicitly.

- AdminServer.java:85, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.base.start.AdminServer.processClientRequest(Socket, List, 
AtomicReference): new java.io.PrintWriter(OutputStream, boolean)

Found a call to a method which will perform a byte to String (or String to 
byte) conversion, and will assume that the default platform encoding is 
suitable. This will cause the application behaviour to vary between platforms. 
Use an alternative API and specify a charset name or Charset object explicitly.

- AdminServer.java:109, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of String.substring(int), which is known to be 
non-null in 
org.apache.ofbiz.base.start.AdminServer.determineClientCommand(String)

This method contains a redundant check of a known non-null value against the 
constant null.

- Classpath.java:104, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
NP: Possible null pointer dereference in 
org.apache.ofbiz.base.start.Classpath.addFilesFromPath(File) due to return 
value of called method

The return value from a method is dereferenced without a null check, and the 
return value of that method is one that should generally be checked for null. 
This may lead to a NullPointerException when the code is executed.

- Classpath.java:105, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.base.start.Classpath.addFilesFromPath(File)

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.

- Config.java:154, SF_SWITCH_NO_DEFAULT
SF: Switch statement found in 
org.apache.ofbiz.base.start.Config.getDefaultLocale(Properties, String) 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.

- Start.java:121, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.base.start.Start$ServerState.toString()

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.

- StartupCommandUtil.java:156, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.base.start.StartupCommandUtil.printOfbizStartupHelp(PrintStream):
 new java.io.PrintWriter(OutputStream, boolean)

Found a call to a method which will perform a byte to String

[jira] [Updated] (OFBIZ-9573) [FB] Package org.apache.ofbiz.base.start

2017-08-14 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9573:
-
Attachment: OFBIZ-9573_org.apache.ofbiz.base.start_bugfixes.patch

- fixed Diamond Operators

class AdminClient:
- Line 79: added a {{StandardCharset}} to {{OutputStream}} to prevent 
conversion problems
- Line 80: added a {{StandardCharset}} to {{InputStream}} to prevent conversion 
problems

class AdminServer:
- Line 86: added a {{StandardCharset}} to {{InputStream}} to prevent conversion 
problems
- Line 87: added a {{StandardCharset}} to {{OutputStream}} to prevent 
conversion problems
- Line 106: method {{determineClientCommand()}}:
   - put long if clause in extra method, easier to read
   - returned directly to not have to declare more variables
   - caught the "fail" with if, so the method ends naturally with the correct 
return -> easier to read
   - reversed the if-clause -> easier to read

class ClassPath:
- method {{addFilesFromPath}}:
   - added nullcheck to check for potential empty lists
   - added default Locale to {{toLowerCase}}

class Config:
- added default case with an {{IllegalArgumentException}} to prevent failures 
because of empty or to long {{locales[]}}

class Start:
- added default Locale to {{toLowerCase}}

class StartUpCommandUtil:
- initialised new {{OutputStreamWriter}} with a {{StandardCharset}} to properly 
read from {{printStream}}

class StartupControlPanel:
- Line 102: did nothing, the method was build to end all processes
- Line 122: did nothing, the method was build to end all processes
- last two bugs fixed as another try-catch was implemented to close streams 
which maybe weren't closed before (just in case, as intended by findbugs)

> [FB] Package org.apache.ofbiz.base.start
> 
>
> Key: OFBIZ-9573
> URL: https://issues.apache.org/jira/browse/OFBIZ-9573
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9573_org.apache.ofbiz.base.start_bugfixes.patch
>
>
> - AdminClient.java:77, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.base.start.AdminClient.sendSocketCommand(AdminServer$OfbizSocketCommand,
>  Config): new java.io.PrintWriter(OutputStream, boolean)
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> - AdminClient.java:78, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.base.start.AdminClient.sendSocketCommand(AdminServer$OfbizSocketCommand,
>  Config): new java.io.InputStreamReader(InputStream)
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> - AdminServer.java:84, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.base.start.AdminServer.processClientRequest(Socket, List, 
> AtomicReference): new java.io.InputStreamReader(InputStream)
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> - AdminServer.java:85, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.base.start.AdminServer.processClientRequest(Socket, List, 
> AtomicReference): new java.io.PrintWriter(OutputStream, boolean)
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> - AdminServer.java:109, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of String.substring(int), which is known to be 
> non-null in 
> org.apache.ofbiz.base.start.AdminServer.determineClientCommand(String)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - Classpath.java:104, NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
> NP: Possible null pointer dereference in 
> org.apache.ofbiz.base.start.Classpath.addFilesFromPath(File) due to return 
> v

[jira] [Created] (OFBIZ-9574) [FB] Package org.apache.ofbiz.base.test

2017-08-14 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9574:


 Summary: [FB] Package org.apache.ofbiz.base.test
 Key: OFBIZ-9574
 URL: https://issues.apache.org/jira/browse/OFBIZ-9574
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- GenericTestCaseBase.java:47, UC_USELESS_OBJECT
Useless object created

Our analysis shows that this object is useless. It's created and modified, but 
its value never go outside of the method or produce any side-effect. Either 
there is a mistake and object was intended to be used or it can be removed.

This analysis rarely produces false-positives. Common false-positive cases 
include:

- This object used to implicitly throw some obscure exception.

- This object used as a stub to generalize the code.

- This object used to hold strong references to weak/soft-referenced objects.

- GenericTestCaseBase.java:99, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.base.test.GenericTestCaseBase.assertNotEquals(String, Object, 
Object)

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

- GenericTestCaseBase.java:99, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.base.test.GenericTestCaseBase.assertNotEquals(String, Object, 
Object)

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

- GenericTestCaseBase.java:327, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.base.test.GenericTestCaseBase.assertEquals(String, Object, 
Object)

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

- GenericTestCaseBase.java:334, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.base.test.GenericTestCaseBase.assertEquals(String, Object, 
Object)

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



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


[jira] [Updated] (OFBIZ-9574) [FB] Package org.apache.ofbiz.base.test

2017-08-14 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9574:
-
Attachment: OFBIZ-9574_org.apache.ofbiz.base.test_bugfixes.patch

- fixed Diamond Operators

did nothing else, everything seems intended as it is, even though findbugs 
doesn't like it like that.
there were no critical bugs and errors too

> [FB] Package org.apache.ofbiz.base.test
> ---
>
> Key: OFBIZ-9574
> URL: https://issues.apache.org/jira/browse/OFBIZ-9574
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9574_org.apache.ofbiz.base.test_bugfixes.patch
>
>
> - GenericTestCaseBase.java:47, UC_USELESS_OBJECT
> Useless object created
> Our analysis shows that this object is useless. It's created and modified, 
> but its value never go outside of the method or produce any side-effect. 
> Either there is a mistake and object was intended to be used or it can be 
> removed.
> This analysis rarely produces false-positives. Common false-positive cases 
> include:
> - This object used to implicitly throw some obscure exception.
> - This object used as a stub to generalize the code.
> - This object used to hold strong references to weak/soft-referenced objects.
> - GenericTestCaseBase.java:99, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.test.GenericTestCaseBase.assertNotEquals(String, 
> Object, Object)
> 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).
> - GenericTestCaseBase.java:99, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.test.GenericTestCaseBase.assertNotEquals(String, 
> Object, Object)
> 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).
> - GenericTestCaseBase.java:327, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.test.GenericTestCaseBase.assertEquals(String, Object, 
> Object)
> 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).
> - GenericTestCaseBase.java:334, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.test.GenericTestCaseBase.assertEquals(String, Object, 
> Object)
> 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).



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


[jira] [Created] (OFBIZ-9575) [FB] Package org.apache.ofbiz.base.util.cache

2017-08-14 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9575:


 Summary: [FB] Package org.apache.ofbiz.base.util.cache
 Key: OFBIZ-9575
 URL: https://issues.apache.org/jira/browse/OFBIZ-9575
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- CacheSoftReference.java:29, SE_NO_SUITABLE_CONSTRUCTOR
Se: org.apache.ofbiz.base.util.cache.CacheSoftReference is Serializable but its 
superclass doesn't define an accessible void constructor

This class implements the Serializable interface and its superclass does not. 
When such an object is deserialized, the fields of the superclass need to be 
initialized by invoking the void constructor of the superclass. Since the 
superclass does not have one, serialization and deserialization will fail at 
runtime.

- CacheSoftReference.java:45, FI_PUBLIC_SHOULD_BE_PROTECTED
FI: org.apache.ofbiz.base.util.cache.CacheSoftReference.finalize() is public; 
should be protected

A class's finalize() method should have protected access, not public.

- UtilCache.java:-1, SE_BAD_FIELD
Se: Class org.apache.ofbiz.base.util.cache.UtilCache defines non-transient 
non-serializable instance field memoryTable

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.

- UtilCache.java:-1, SE_BAD_FIELD
Se: Class org.apache.ofbiz.base.util.cache.UtilCache defines non-transient 
non-serializable instance field listeners

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.

- UtilCache.java:63, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.base.util.cache.UtilCache 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.

- UtilCache.java:160, DMI_INVOKING_TOSTRING_ON_ARRAY
USELESS_STRING: Invocation of toString on propNames in 
org.apache.ofbiz.base.util.cache.UtilCache.getPropertyParam(ResourceBundle, 
String[], String)

The code invokes toString on an array, which will generate a fairly useless 
result such as [C@16f0472. Consider using Arrays.toString to convert the array 
into a readable String that gives the contents of the array. See Programming 
Puzzlers, chapter 3, puzzle 12.

- UtilCache.java:387, NP_NULL_ON_SOME_PATH_EXCEPTION
NP: Possible null pointer dereference of o in 
org.apache.ofbiz.base.util.cache.UtilCache.findSizeInBytes(Object) on exception 
path

A reference value which is null on some exception control path is dereferenced 
here.  This may lead to a NullPointerException when the code is executed.  Note 
that because FindBugs currently does not prune infeasible exception paths, this 
may be a false warning.

Also note that FindBugs considers the default case of a switch statement to be 
an exception path, since the default case is often infeasible.



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


[jira] [Updated] (OFBIZ-9575) [FB] Package org.apache.ofbiz.base.util.cache

2017-08-14 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9575:
-
Attachment: OFBIZ-9575_org.apache.ofbiz.base.util.cache_bugfixes.patch

- fixed Diamond Operators

class CacheSoftReference:
- didn't fix first bug: why is this class implementing Serializable at all? I 
didn't get it :(
- set method {{finalize}} to {{protected}}

class UtilCache:
- deleted unnecessary else blocks
- added {{Arrays.toString()}} to prevent the reading of the Array {{propNames}} 
from returning random text
- didn't fix Line 108 & 110, not really possible to fix there, hopefully 
intended as it is
- circular dependency not found, maybe there is none..?
- moved nullcheck in {{findSizeInBytes}} in front of the try-block, therefore 
no possible null inside of the try-block

> [FB] Package org.apache.ofbiz.base.util.cache
> -
>
> Key: OFBIZ-9575
> URL: https://issues.apache.org/jira/browse/OFBIZ-9575
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9575_org.apache.ofbiz.base.util.cache_bugfixes.patch
>
>
> - CacheSoftReference.java:29, SE_NO_SUITABLE_CONSTRUCTOR
> Se: org.apache.ofbiz.base.util.cache.CacheSoftReference is Serializable but 
> its superclass doesn't define an accessible void constructor
> This class implements the Serializable interface and its superclass does not. 
> When such an object is deserialized, the fields of the superclass need to be 
> initialized by invoking the void constructor of the superclass. Since the 
> superclass does not have one, serialization and deserialization will fail at 
> runtime.
> - CacheSoftReference.java:45, FI_PUBLIC_SHOULD_BE_PROTECTED
> FI: org.apache.ofbiz.base.util.cache.CacheSoftReference.finalize() is public; 
> should be protected
> A class's finalize() method should have protected access, not public.
> - UtilCache.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.base.util.cache.UtilCache defines non-transient 
> non-serializable instance field memoryTable
> 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.
> - UtilCache.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.base.util.cache.UtilCache defines non-transient 
> non-serializable instance field listeners
> 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.
> - UtilCache.java:63, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.base.util.cache.UtilCache 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.
> - UtilCache.java:160, DMI_INVOKING_TOSTRING_ON_ARRAY
> USELESS_STRING: Invocation of toString on propNames in 
> org.apache.ofbiz.base.util.cache.UtilCache.getPropertyParam(ResourceBundle, 
> String[], String)
> The code invokes toString on an array, which will generate a fairly useless 
> result such as [C@16f0472. Consider using Arrays.toString to convert the 
> array into a readable String that gives the contents of the array. See 
> Programming Puzzlers, chapter 3, puzzle 12.
> - UtilCache.java:387, NP_NULL_ON_SOME_PATH_EXCEPTION
> NP: Possible null pointer dereference of o in 
> org.apache.ofbiz.base.util.cache.UtilCache.findSizeInBytes(Object) on 
> exception path
> A reference value which is null on some exception control path is 
> dereferenced here.  This may lead to a NullPointerException when the code is 
> executed.  Note that because FindBugs currently does not prune infeasible 
> exception paths, this may be a false warning.
> Also note that FindBugs considers the default case of

[jira] [Created] (OFBIZ-9576) [FB] Package org.apache.ofbiz.base.util.cache.test

2017-08-14 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9576:


 Summary: [FB] Package org.apache.ofbiz.base.util.cache.test
 Key: OFBIZ-9576
 URL: https://issues.apache.org/jira/browse/OFBIZ-9576
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition is 
final but declares protected field 
org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition.newValue

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.

- UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal is final 
but declares protected field 
org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal.oldValue

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.

- UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is final 
but declares protected field 
org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.newValue

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.

- UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is final 
but declares protected field 
org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.oldValue

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.

- UtilCacheTests.java:39, SE_NO_SUITABLE_CONSTRUCTOR
Se: org.apache.ofbiz.base.util.cache.test.UtilCacheTests is Serializable but 
its superclass doesn't define an accessible void constructor

This class implements the Serializable interface and its superclass does not. 
When such an object is deserialized, the fields of the superclass need to be 
initialized by invoking the void constructor of the superclass. Since the 
superclass does not have one, serialization and deserialization will fail at 
runtime.

- UtilCacheTests.java:39, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.base.util.cache.test.UtilCacheTests 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.

- UtilCacheTests.java:148, HE_EQUALS_USE_HASHCODE
HE: org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Listener 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
  }

- UtilCacheTests.java:148, NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT
NP: 
org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Listener.equals(Object) 
does not check for null argument

This implementation of equals(Object) violates the contract defined by 
java.lang.Object.equals() because it does not check for null being passed as 
the argument. All equals() methods should return false if passed a null value.

- UtilCache

[jira] [Updated] (OFBIZ-9576) [FB] Package org.apache.ofbiz.base.util.cache.test

2017-08-14 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9576:
-
Attachment: OFBIZ-9576_org.apache.ofbiz.base.util.cache.test_bugfixes.patch

- fixed Diamond Operators

- Line 39: didn't fix error, {{serialVersionUID}} doesn't have to be 
implemented for this test class
- class Removal: changed {{oldValue}} from {{protected}} to {{private}}
- class Addition: changed {{newValue}} from {{protected}} to {{private}}
- class Update: changed {{newValue}} and {{oldValue}} from {{protected}} to 
{{private}}
- added type-check in {{equals}} to prevent casting errors
- implemented {{hashCode()}}, because {{equals}} was implemented (it still 
throws a findbugs bug, because it just uses the {{super.hashCode()}}
- Line 353: didn't change anything, if the declaration is changed, it won't work
- Line 354: didn't change anything, if the declaration is changed, it won't work
- put code, which was used more often in an own method called 
{{assertKeyLoop}}, because it is DRY, but didn't change the String declaration, 
because it was used to clone the String
- Line 358: didn't fix anything, method was planned this way

> [FB] Package org.apache.ofbiz.base.util.cache.test
> --
>
> Key: OFBIZ-9576
> URL: https://issues.apache.org/jira/browse/OFBIZ-9576
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9576_org.apache.ofbiz.base.util.cache.test_bugfixes.patch
>
>
> - UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
> CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition is 
> final but declares protected field 
> org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition.newValue
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.
> - UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
> CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal is 
> final but declares protected field 
> org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal.oldValue
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.
> - UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
> CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is 
> final but declares protected field 
> org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.newValue
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.
> - UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
> CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is 
> final but declares protected field 
> org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.oldValue
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.
> - UtilCacheTests.java:39, SE_NO_SUITABLE_CONSTRUCTOR
> Se: org.apache.ofbiz.base.util.cache.test.UtilCacheTests is Serializable but 
> its superclass doesn't define an accessible void constructor
> This class implements the Serializable interface and its superclass does not. 
> When such an object is deserialized, the fields of the superclass need to be 
> initialized by invoking the void constructor of the superclass. Since the 
> superclass does not have one, serialization and deserialization will fail at 
> runtime.
> - UtilCacheTests.java:39, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.base.util.cache.test.UtilCacheTests 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 n

[jira] [Created] (OFBIZ-9586) [FB] Package org.apache.ofbiz.base.util.template

2017-08-17 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9586:


 Summary: [FB] Package org.apache.ofbiz.base.util.template
 Key: OFBIZ-9586
 URL: https://issues.apache.org/jira/browse/OFBIZ-9586
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


FreeMarkerWorker.java:210, BC_UNCONFIRMED_CAST, Priorität: Niedrig
BC: Unchecked/unconfirmed cast from Appendable to java.io.Writer in 
org.apache.ofbiz.base.util.template.FreeMarkerWorker.renderTemplate(Template, 
Map, Appendable)

This cast is unchecked, and not all instances of the type casted from can be 
cast to the type it is being cast to. Check that your program logic ensures 
that this cast will not fail.



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


[jira] [Commented] (OFBIZ-9586) [FB] Package org.apache.ofbiz.base.util.template

2017-08-17 Thread Dennis Balkir (JIRA)

[ 
https://issues.apache.org/jira/browse/OFBIZ-9586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16130321#comment-16130321
 ] 

Dennis Balkir commented on OFBIZ-9586:
--

I didn't fix this FB-error.
The code which caused this is:

{noformat}
// FIXME: the casting from Appendable to Writer is a temporary fix that could 
cause a
//run time error if in the future we will pass a different class to the 
method
//(such as a StringBuffer).
Environment env = template.createProcessingEnvironment(context, (Writer) 
outWriter);
applyUserSettings(env, context);
env.process();
return env;
{noformat}

I don't know if this is just a theoretical error, or if this can really happen 
at some time, but maybe somebody can take a look at this and check if there is 
a possibility of a casting error at this point since at some point somebody has 
already looked at this and wrote the {{FIXME}} to show, that something has to 
be done.

> [FB] Package org.apache.ofbiz.base.util.template
> 
>
> Key: OFBIZ-9586
> URL: https://issues.apache.org/jira/browse/OFBIZ-9586
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> FreeMarkerWorker.java:210, BC_UNCONFIRMED_CAST, Priorität: Niedrig
> BC: Unchecked/unconfirmed cast from Appendable to java.io.Writer in 
> org.apache.ofbiz.base.util.template.FreeMarkerWorker.renderTemplate(Template, 
> Map, Appendable)
> This cast is unchecked, and not all instances of the type casted from can be 
> cast to the type it is being cast to. Check that your program logic ensures 
> that this cast will not fail.



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


[jira] [Created] (OFBIZ-9589) [FB] Package org.apache.ofbiz.base.util.string.test

2017-08-17 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9589:


 Summary: [FB] Package org.apache.ofbiz.base.util.string.test
 Key: OFBIZ-9589
 URL: https://issues.apache.org/jira/browse/OFBIZ-9589
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


FlexibleStringExpanderTests.java:196, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.fseTest(String,
 String, Map, TimeZone, Locale, String, Object, boolean)

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

- FlexibleStringExpanderTests.java:209, NM_METHOD_NAMING_CONVENTION
Nm: The method name 
org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.StaticReturnNull()
 doesn't start with a lower case letter

Methods should be verbs, in mixed case with the first letter lowercase, with 
the first letter of each internal word capitalized.

- FlexibleStringExpanderTests.java:226, NM_CLASS_NOT_EXCEPTION
Nm: Class 
org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$ThrowException
 is not derived from an Exception, even though it is named as such

This class is not derived from another exception, but ends with 'Exception'. 
This will be confusing to users of this class.

- FlexibleStringExpanderTests.java:251, SE_NO_SERIALVERSIONID
SnVI: 
org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$SpecialNumber
 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)


[jira] [Updated] (OFBIZ-9589) [FB] Package org.apache.ofbiz.base.util.string.test

2017-08-17 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9589:
-
Attachment: OFBIZ-9589_org.apache.ofbiz.base.util.string.test_bugfixes.patch

- Diamond Operators fixed

- Line 196: didn’t do anything, was intended as it is
- Line 209: changed the name from {{StaticReturnNull}} to {{staticReturnNull}}
- Line 226: class {{ThrowException}} is now derived from {{Exception}} just to 
prevent confusion
- Line 251: didn’t do anything; class {{SpecialNumber}} is Serializable, but 
implementing {{serialVersionUID}} is not necessary

> [FB] Package org.apache.ofbiz.base.util.string.test
> ---
>
> Key: OFBIZ-9589
> URL: https://issues.apache.org/jira/browse/OFBIZ-9589
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9589_org.apache.ofbiz.base.util.string.test_bugfixes.patch
>
>
> FlexibleStringExpanderTests.java:196, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.fseTest(String,
>  String, Map, TimeZone, Locale, String, Object, boolean)
> 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).
> - FlexibleStringExpanderTests.java:209, NM_METHOD_NAMING_CONVENTION
> Nm: The method name 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.StaticReturnNull()
>  doesn't start with a lower case letter
> Methods should be verbs, in mixed case with the first letter lowercase, with 
> the first letter of each internal word capitalized.
> - FlexibleStringExpanderTests.java:226, NM_CLASS_NOT_EXCEPTION
> Nm: Class 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$ThrowException
>  is not derived from an Exception, even though it is named as such
> This class is not derived from another exception, but ends with 'Exception'. 
> This will be confusing to users of this class.
> - FlexibleStringExpanderTests.java:251, SE_NO_SERIALVERSIONID
> SnVI: 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$SpecialNumber
>  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)


[jira] [Created] (OFBIZ-9590) [FB] Package org.apache.ofbiz.base.util.collections

2017-08-17 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9590:


 Summary: [FB] Package org.apache.ofbiz.base.util.collections
 Key: OFBIZ-9590
 URL: https://issues.apache.org/jira/browse/OFBIZ-9590
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


FlexibleMapAccessor.java:44, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.base.util.collections.FlexibleMapAccessor 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.

- FlexibleServletAccessor.java:47, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.base.util.collections.FlexibleServletAccessor 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.

- FlexibleServletAccessor.java:181, 
EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS
Eq: 
org.apache.ofbiz.base.util.collections.FlexibleServletAccessor.equals(Object) 
checks for operand being a String

This equals method is checking to see if the argument is some incompatible type 
(i.e., a class that is neither a supertype nor subtype of the class that 
defines the equals method). For example, the Foo class might have an equals 
method that looks like:

public boolean equals(Object o) {
if (o instanceof Foo)
return name.equals(((Foo)o).name);
else if (o instanceof String)
return name.equals(o);
else return false;
This is considered bad practice, as it makes it very hard to implement an 
equals method that is symmetric and transitive. Without those properties, very 
unexpected behavoirs are possible.

- FlexibleServletAccessor.java:208, SE_NO_SERIALVERSIONID
SnVI: 
org.apache.ofbiz.base.util.collections.FlexibleServletAccessor$AttributeAccessor
 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.

- GenericMap.java:68, HE_EQUALS_USE_HASHCODE
HE: org.apache.ofbiz.base.util.collections.GenericMap 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
}

- GenericMapValues.java:45, EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS
Eq: org.apache.ofbiz.base.util.collections.GenericMapValues.equals(Object) 
checks for operand being a java.util.List

This equals method is checking to see if the argument is some incompatible type 
(i.e., a class that is neither a supertype nor subtype of the class that 
defines the equals method). For example, the Foo class might have an equals 
method that looks like:

public boolean equals(Object o) {
if (o instanceof Foo)
return name.equals(((Foo)o).

[jira] [Updated] (OFBIZ-9590) [FB] Package org.apache.ofbiz.base.util.collections

2017-08-17 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9590:
-
Attachment: OFBIZ-9590_org.apache.ofbiz.base.util.collections_bugfixes.patch

- fixed Diamond Operators

class FlexibleMapAccessor:
- Line 44: didn’t do anything, {{serialVersionUID}} is not necessary for this 
class

class FlexibleServletAccessor:
- deleted unnecessary else-blocks
- Line 47: didn’t do anything, {{serialVersionUID}} is not necessary for this 
class
- deleted unnecessary casting in {{equals}} and added a nullcheck, to prevent a 
casting exception; findbugs bug still there, equals method isn’t really used 
for what it’s supposed to be used
- Line 210: didn’t do anything, {{serialVersionUID}} is not necessary for this 
class

class GenericMap:
- didn’t find circular dependency
- Line 68: didn’t implement {{hashCode}}, not necessary

class GenericMapValues:
- Line 45: didn’t change anything, hopefully it was intended like this

class LRUMap:
- didn’t do anything, {{serialVersionUID}} is not necessary for this class

class LifoSet:
- didn’t do anything, {{serialVersionUID}} is not necessary for this class

class MapComparator:
- class doesn’t implement Serializable, has not to be changed
- added nullcheck in {{equals}} which returns false, if a null object is given
- implemented {{hashCode()}}, because {{equals}} is implemented
- deleted unnecessary else-block

class MapContext:
- Line 341: deleted the statement, that class {{ListSet}} implements {{Set}}, 
because its superclass already does it, therefore {{ListSet}} automatically 
does it too
- changed {{listImpl}} from {{protected}} to {{private}} since the class 
{{ListSet}} is final
- deleted unnecessary else-blocks

class ResourceBundleMapWrapper:
- Line 37: didn’t do anything, {{serialVersionUID}} is not necessary for this 
class
- Line 39 & 40: couldn’t change the implementation
- Line 153: couldn’t change the implementation
- Line 221: deleted unnecessary nullcheck; if {{arg0}} is null, the thrown 
exception will be caught, if not it still returns true
- deleted unnecessary else-blocks

> [FB] Package org.apache.ofbiz.base.util.collections
> ---
>
> Key: OFBIZ-9590
> URL: https://issues.apache.org/jira/browse/OFBIZ-9590
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9590_org.apache.ofbiz.base.util.collections_bugfixes.patch
>
>
> FlexibleMapAccessor.java:44, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.base.util.collections.FlexibleMapAccessor 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.
> - FlexibleServletAccessor.java:47, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.base.util.collections.FlexibleServletAccessor 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.
> - FlexibleServletAccessor.java:181, 
> EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS
> Eq: 
> org.apache.ofbiz.base.util.collections.FlexibleServletAccessor.equals(Object) 
> checks for operand being a String
> This equals method is checking to see if the argument is some incompatible 
> type (i.e., a class that is neither a supertype nor subtype of the class that 
> defines the equals method). For example, the Foo class might have an equals 
> method that looks like:
> public boolean equals(Object o) {
> if (o instanceof Foo)
> return name.equals(((Foo)o).name);
> else if (o instanceof String)
> return name.equals(o);
> el

[jira] [Updated] (OFBIZ-9589) [FB] Package org.apache.ofbiz.base.util.string.test

2017-08-18 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9589:
-
Attachment: OFBIZ-9589_org.apache.ofbiz.base.util.string.test_bugfixes.patch

> [FB] Package org.apache.ofbiz.base.util.string.test
> ---
>
> Key: OFBIZ-9589
> URL: https://issues.apache.org/jira/browse/OFBIZ-9589
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9589_org.apache.ofbiz.base.util.string.test_bugfixes.patch, 
> OFBIZ-9589_org.apache.ofbiz.base.util.string.test_bugfixes.patch
>
>
> FlexibleStringExpanderTests.java:196, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.fseTest(String,
>  String, Map, TimeZone, Locale, String, Object, boolean)
> 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).
> - FlexibleStringExpanderTests.java:209, NM_METHOD_NAMING_CONVENTION
> Nm: The method name 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests.StaticReturnNull()
>  doesn't start with a lower case letter
> Methods should be verbs, in mixed case with the first letter lowercase, with 
> the first letter of each internal word capitalized.
> - FlexibleStringExpanderTests.java:226, NM_CLASS_NOT_EXCEPTION
> Nm: Class 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$ThrowException
>  is not derived from an Exception, even though it is named as such
> This class is not derived from another exception, but ends with 'Exception'. 
> This will be confusing to users of this class.
> - FlexibleStringExpanderTests.java:251, SE_NO_SERIALVERSIONID
> SnVI: 
> org.apache.ofbiz.base.util.string.test.FlexibleStringExpanderTests$SpecialNumber
>  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)


[jira] [Created] (OFBIZ-9626) [FB] Package org.apache.ofbiz.cmssite.multisite

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9626:


 Summary: [FB] Package org.apache.ofbiz.cmssite.multisite
 Key: OFBIZ-9626
 URL: https://issues.apache.org/jira/browse/OFBIZ-9626
 Project: OFBiz
  Issue Type: Sub-task
  Components: cmssite
Affects Versions: Trunk
Reporter: Dennis Balkir


- MultiSiteRequestWrapper.java:140, NM_CONFUSING
Nm: Confusing to have methods 
org.apache.ofbiz.cmssite.multisite.MultiSiteRequestWrapper.getRequestURI() and 
org.apache.ofbiz.webtools.artifactinfo.ControllerRequestArtifactInfo.getRequestUri()

The referenced methods have names that differ only by capitalization.

- WebSiteFilter.java:68, BC_UNCONFIRMED_CAST
BC: Unchecked/unconfirmed cast from javax.servlet.ServletRequest to 
javax.servlet.http.HttpServletRequest in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
ServletResponse, FilterChain)

This cast is unchecked, and not all instances of the type casted from can be 
cast to the type it is being cast to. Check that your program logic ensures 
that this cast will not fail.

- WebSiteFilter.java:69, BC_UNCONFIRMED_CAST
BC: Unchecked/unconfirmed cast from javax.servlet.ServletResponse to 
javax.servlet.http.HttpServletResponse in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
ServletResponse, FilterChain)

This cast is unchecked, and not all instances of the type casted from can be 
cast to the type it is being cast to. Check that your program logic ensures 
that this cast will not fail.

- WebSiteFilter.java:83, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
ServletResponse, FilterChain)

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

- WebSiteFilter.java:83, RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
RCN: Redundant nullcheck of webSite which is known to be null in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
ServletResponse, FilterChain)

This method contains a redundant check of a known null value against the 
constant null.

- WebSiteFilter.java:161, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
J2EE: Store of non serializable org.apache.ofbiz.entity.Delegator into 
HttpSession in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
 HttpServletResponse, Delegator, LocalDispatcher)

This code seems to be storing a non-serializable object into an HttpSession. If 
this session is passivated or migrated, an error will result.

- WebSiteFilter.java:162, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
J2EE: Store of non serializable org.apache.ofbiz.service.LocalDispatcher into 
HttpSession in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
 HttpServletResponse, Delegator, LocalDispatcher)

This code seems to be storing a non-serializable object into an HttpSession. If 
this session is passivated or migrated, an error will result.

- WebSiteFilter.java:163, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
J2EE: Store of non serializable org.apache.ofbiz.security.Security into 
HttpSession in 
org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
 HttpServletResponse, Delegator, LocalDispatcher)

This code seems to be storing a non-serializable object into an HttpSession. If 
this session is passivated or migrated, an error will result.



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


[jira] [Updated] (OFBIZ-9626) [FB] Package org.apache.ofbiz.cmssite.multisite

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9626:
-
Priority: Minor  (was: Major)

> [FB] Package org.apache.ofbiz.cmssite.multisite
> ---
>
> Key: OFBIZ-9626
> URL: https://issues.apache.org/jira/browse/OFBIZ-9626
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: cmssite
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9629_org.apache.ofbiz.cmssite.multisite_bugfixes.patch
>
>
> - MultiSiteRequestWrapper.java:140, NM_CONFUSING
> Nm: Confusing to have methods 
> org.apache.ofbiz.cmssite.multisite.MultiSiteRequestWrapper.getRequestURI() 
> and 
> org.apache.ofbiz.webtools.artifactinfo.ControllerRequestArtifactInfo.getRequestUri()
> The referenced methods have names that differ only by capitalization.
> - WebSiteFilter.java:68, BC_UNCONFIRMED_CAST
> BC: Unchecked/unconfirmed cast from javax.servlet.ServletRequest to 
> javax.servlet.http.HttpServletRequest in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This cast is unchecked, and not all instances of the type casted from can be 
> cast to the type it is being cast to. Check that your program logic ensures 
> that this cast will not fail.
> - WebSiteFilter.java:69, BC_UNCONFIRMED_CAST
> BC: Unchecked/unconfirmed cast from javax.servlet.ServletResponse to 
> javax.servlet.http.HttpServletResponse in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This cast is unchecked, and not all instances of the type casted from can be 
> cast to the type it is being cast to. Check that your program logic ensures 
> that this cast will not fail.
> - WebSiteFilter.java:83, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> 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).
> - WebSiteFilter.java:83, RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
> RCN: Redundant nullcheck of webSite which is known to be null in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This method contains a redundant check of a known null value against the 
> constant null.
> - WebSiteFilter.java:161, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.entity.Delegator into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.
> - WebSiteFilter.java:162, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.service.LocalDispatcher into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.
> - WebSiteFilter.java:163, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.security.Security into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.



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


[jira] [Updated] (OFBIZ-9626) [FB] Package org.apache.ofbiz.cmssite.multisite

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9626:
-
Attachment: OFBIZ-9629_org.apache.ofbiz.cmssite.multisite_bugfixes.patch

class MultiSiteRequestWrapper:
- didn’t do anything, the method’s name, which caused this FB-error, has to be 
used since the method is overriding another method

class WebSiteFilter:
- Line 69: casting errors won’t occur since ofbiz-api always uses correct 
parameter types
- Line 83: removed the unnecessary nullcheck; the parameter {{webSite}} can 
only be null at this point since it was declared null two lines above
- Lines 161, 162, 163: ofbiz-api won’t cause this to be a problem, didn’t fix it


> [FB] Package org.apache.ofbiz.cmssite.multisite
> ---
>
> Key: OFBIZ-9626
> URL: https://issues.apache.org/jira/browse/OFBIZ-9626
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: cmssite
>Affects Versions: Trunk
>Reporter: Dennis Balkir
> Attachments: 
> OFBIZ-9629_org.apache.ofbiz.cmssite.multisite_bugfixes.patch
>
>
> - MultiSiteRequestWrapper.java:140, NM_CONFUSING
> Nm: Confusing to have methods 
> org.apache.ofbiz.cmssite.multisite.MultiSiteRequestWrapper.getRequestURI() 
> and 
> org.apache.ofbiz.webtools.artifactinfo.ControllerRequestArtifactInfo.getRequestUri()
> The referenced methods have names that differ only by capitalization.
> - WebSiteFilter.java:68, BC_UNCONFIRMED_CAST
> BC: Unchecked/unconfirmed cast from javax.servlet.ServletRequest to 
> javax.servlet.http.HttpServletRequest in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This cast is unchecked, and not all instances of the type casted from can be 
> cast to the type it is being cast to. Check that your program logic ensures 
> that this cast will not fail.
> - WebSiteFilter.java:69, BC_UNCONFIRMED_CAST
> BC: Unchecked/unconfirmed cast from javax.servlet.ServletResponse to 
> javax.servlet.http.HttpServletResponse in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This cast is unchecked, and not all instances of the type casted from can be 
> cast to the type it is being cast to. Check that your program logic ensures 
> that this cast will not fail.
> - WebSiteFilter.java:83, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> 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).
> - WebSiteFilter.java:83, RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
> RCN: Redundant nullcheck of webSite which is known to be null in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.doFilter(ServletRequest, 
> ServletResponse, FilterChain)
> This method contains a redundant check of a known null value against the 
> constant null.
> - WebSiteFilter.java:161, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.entity.Delegator into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.
> - WebSiteFilter.java:162, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.service.LocalDispatcher into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.
> - WebSiteFilter.java:163, J2EE_STORE_OF_NON_SERIALIZABLE_OBJECT_INTO_SESSION
> J2EE: Store of non serializable org.apache.ofbiz.security.Security into 
> HttpSession in 
> org.apache.ofbiz.cmssite.multisite.WebSiteFilter.setWebContextObjects(HttpServletRequest,
>  HttpServletResponse, Delegator, LocalDispatcher)
> This code seems to be storing a non-serializable object into an HttpSession. 
> If this session is passivated or migrated, an error will result.



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


[jira] [Created] (OFBIZ-9627) [FB] Package org.apache.ofbiz.common.authentication

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9627:


 Summary: [FB] Package org.apache.ofbiz.common.authentication
 Key: OFBIZ-9627
 URL: https://issues.apache.org/jira/browse/OFBIZ-9627
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AuthenticationComparator.java:31, SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
Se: org.apache.ofbiz.common.authentication.AuthenticationComparator implements 
Comparator but not Serializable

This class implements the Comparator interface. You should consider whether or 
not it should also implement the Serializable interface. If a comparator is 
used to construct an ordered collection such as a TreeMap, then the TreeMap 
will be serializable only if the comparator is also serializable. As most 
comparators have little or no state, making them serializable is generally easy 
and good defensive programming.

- AuthenticationComparator.java:70, CO_COMPARETO_INCORRECT_FLOATING
compareTo()/compare() incorrectly handles float or double value

This method compares double or float values using pattern like this: val1 > 
val2 ? 1 : val1 < val2 ? -1 : 0. This pattern works incorrectly for -0.0 and 
NaN values which may result in incorrect sorting result or broken collection 
(if compared values are used as keys). Consider using Double.compare or 
Float.compare static methods which handle all the special cases correctly.



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


[jira] [Updated] (OFBIZ-9627) [FB] Package org.apache.ofbiz.common.authentication

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9627:
-
Attachment: 
OFBIZ-9627_org.apache.ofbiz.common.authentification_bugfixes.patch

- implementation of Serializable is not necessary
method compare:
- changed the comparison to {{Float.compare}}
- made a local parameter {{comp}} to save the value from {{Float.compare}} to 
use it
- instead of returning -1 or 1 it now returns the result of {{Float.compare}} 
put through {{Math.signum}} (which then only returns -1 or 1)
- deleted unnecessary else


> [FB] Package org.apache.ofbiz.common.authentication
> ---
>
> Key: OFBIZ-9627
> URL: https://issues.apache.org/jira/browse/OFBIZ-9627
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9627_org.apache.ofbiz.common.authentification_bugfixes.patch
>
>
> - AuthenticationComparator.java:31, SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
> Se: org.apache.ofbiz.common.authentication.AuthenticationComparator 
> implements Comparator but not Serializable
> This class implements the Comparator interface. You should consider whether 
> or not it should also implement the Serializable interface. If a comparator 
> is used to construct an ordered collection such as a TreeMap, then the 
> TreeMap will be serializable only if the comparator is also serializable. As 
> most comparators have little or no state, making them serializable is 
> generally easy and good defensive programming.
> - AuthenticationComparator.java:70, CO_COMPARETO_INCORRECT_FLOATING
> compareTo()/compare() incorrectly handles float or double value
> This method compares double or float values using pattern like this: val1 > 
> val2 ? 1 : val1 < val2 ? -1 : 0. This pattern works incorrectly for -0.0 and 
> NaN values which may result in incorrect sorting result or broken collection 
> (if compared values are used as keys). Consider using Double.compare or 
> Float.compare static methods which handle all the special cases correctly.



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


[jira] [Created] (OFBIZ-9628) [FB] Package org.apache.ofbiz.common.email

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9628:


 Summary: [FB] Package org.apache.ofbiz.common.email
 Key: OFBIZ-9628
 URL: https://issues.apache.org/jira/browse/OFBIZ-9628
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- EmailServices.java:547, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.common.email.EmailServices.sendMailFromScreen(DispatchContext, 
Map)

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 ...
  }

- EmailServices.java:592, BX_UNBOXING_IMMEDIATELY_REBOXED
Bx: Boxed value is unboxed and then immediately reboxed in 
org.apache.ofbiz.common.email.EmailServices.sendMailFromScreen(DispatchContext, 
Map)

A boxed value is unboxed and then immediately reboxed.

- EmailServices.java:662, UC_USELESS_OBJECT
Useless object created

Our analysis shows that this object is useless. It's created and modified, but 
its value never go outside of the method or produce any side-effect. Either 
there is a mistake and object was intended to be used or it can be removed.

This analysis rarely produces false-positives. Common false-positive cases 
include:
- This object used to implicitly throw some obscure exception.
- This object used as a stub to generalize the code.
- This object used to hold strong references to weak/soft-referenced objects.

- EmailServices.java:715, EI_EXPOSE_REP2
EI2: new 
org.apache.ofbiz.common.email.EmailServices$ByteArrayDataSource(byte[], String) 
may expose internal representation by storing an externally mutable object into 
EmailServices$ByteArrayDataSource.contentArray

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.

- NotificationServices.java:270, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.common.email.NotificationServices.setBaseUrl(Delegator, 
String, Map)

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 ...
  }



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


[jira] [Updated] (OFBIZ-9628) [FB] Package org.apache.ofbiz.common.email

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9628:
-
Attachment: OFBIZ-No_org.apache.ofbiz.common.email_bugfixes.patch

- Diamond Operators fixed
- Line 547: added another catch for {{RuntimeException}}
- Line 594: changed the if phrase with {{hideInLog}} to prevent the 
unboxing-reboxing issue
- Line 664: deleted the declaration of the second {{bodyParts}}, because this 
{{bodyParts}} and it’s value is never used
- Line 717: changed the storage of {{content}} into {{this.contentArray}} to a 
copy of content to reduce vulnerability


> [FB] Package org.apache.ofbiz.common.email
> --
>
> Key: OFBIZ-9628
> URL: https://issues.apache.org/jira/browse/OFBIZ-9628
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-No_org.apache.ofbiz.common.email_bugfixes.patch
>
>
> - EmailServices.java:547, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.common.email.EmailServices.sendMailFromScreen(DispatchContext,
>  Map)
> 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 ...
>   }
> - EmailServices.java:592, BX_UNBOXING_IMMEDIATELY_REBOXED
> Bx: Boxed value is unboxed and then immediately reboxed in 
> org.apache.ofbiz.common.email.EmailServices.sendMailFromScreen(DispatchContext,
>  Map)
> A boxed value is unboxed and then immediately reboxed.
> - EmailServices.java:662, UC_USELESS_OBJECT
> Useless object created
> Our analysis shows that this object is useless. It's created and modified, 
> but its value never go outside of the method or produce any side-effect. 
> Either there is a mistake and object was intended to be used or it can be 
> removed.
> This analysis rarely produces false-positives. Common false-positive cases 
> include:
> - This object used to implicitly throw some obscure exception.
> - This object used as a stub to generalize the code.
> - This object used to hold strong references to weak/soft-referenced objects.
> - EmailServices.java:715, EI_EXPOSE_REP2
> EI2: new 
> org.apache.ofbiz.common.email.EmailServices$ByteArrayDataSource(byte[], 
> String) may expose internal representation by storing an externally mutable 
> object into EmailServices$ByteArrayDataSource.contentArray
> 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.
> - NotificationServices.java:270, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.common.email.NotificationServices.setBaseUrl(Delegator, 
> String, Map)
> 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 ...
>   }



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


[jira] [Created] (OFBIZ-9629) [FB] Package org.apache.ofbiz.common.image

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9629:


 Summary: [FB] Package org.apache.ofbiz.common.image
 Key: OFBIZ-9629
 URL: https://issues.apache.org/jira/browse/OFBIZ-9629
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- ImageTransform.java:119, DM_STRING_TOSTRING
Dm: org.apache.ofbiz.common.image.ImageTransform.scaleImage(BufferedImage, 
double, double, Map, String, Locale) invokes toString() method on a String

Calling String.toString() is just a redundant operation. Just use the String.



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


[jira] [Updated] (OFBIZ-9629) [FB] Package org.apache.ofbiz.common.image

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9629:
-
Attachment: OFBIZ-9629_org.apache.ofbiz.common.image_bugfixes.patch

- Diamond Operators fixed
- Line 119: removed {{toString}} because {{get(„height“)}} returns a string
- Line 124: removed the {{toString}} because {{get(„width“)}} returns a string


> [FB] Package org.apache.ofbiz.common.image
> --
>
> Key: OFBIZ-9629
> URL: https://issues.apache.org/jira/browse/OFBIZ-9629
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9629_org.apache.ofbiz.common.image_bugfixes.patch
>
>
> - ImageTransform.java:119, DM_STRING_TOSTRING
> Dm: org.apache.ofbiz.common.image.ImageTransform.scaleImage(BufferedImage, 
> double, double, Map, String, Locale) invokes toString() method on a String
> Calling String.toString() is just a redundant operation. Just use the String.



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


[jira] [Created] (OFBIZ-9630) [FB] Package org.apache.ofbiz.common.login

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9630:


 Summary: [FB] Package org.apache.ofbiz.common.login
 Key: OFBIZ-9630
 URL: https://issues.apache.org/jira/browse/OFBIZ-9630
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- LoginServices.java:118, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.common.login.LoginServices.userLogin(DispatchContext, 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.

- LoginServices.java:161, DLS_DEAD_LOCAL_STORE
DLS: Dead store to loginDisableMinutes in 
org.apache.ofbiz.common.login.LoginServices.userLogin(DispatchContext, 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.

- LoginServices.java:569, DLS_DEAD_LOCAL_STORE
DLS: Dead store to resultMap in 
org.apache.ofbiz.common.login.LoginServices.createUserLogin(DispatchContext, 
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.

- LoginServices.java:661, NP_LOAD_OF_KNOWN_NULL_VALUE
NP: Load of known null value in 
org.apache.ofbiz.common.login.LoginServices.updatePassword(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).

- LoginServices.java:671, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.common.login.LoginServices.updatePassword(DispatchContext, 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.

- LoginServices.java:733, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.common.login.LoginServices.updateUserLoginId(DispatchContext, 
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.

- LoginServices.java:906, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
RCN: Nullcheck of userLogin at line 915 of value previously dereferenced in 
org.apache.ofbiz.common.login.LoginServices.checkNewPassword(GenericValue, 
String, String, String, String, List, boolean, Locale)

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.

- LoginServices.java:915, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of currentPassword, which is known to be non-null in 
org.apache.ofbiz.common.login.LoginServices.checkNewPassword(GenericValue, 
String, String, String, String, List, boolean, Locale)

This method contains a redundant check of a known non-null value against the 
constant null.

- LoginServices.java:988, DLS_DEAD_LOCAL_STORE
DLS: Dead store to messageMap in 
org.apache.ofbiz.common.login.LoginServices.checkNewPassword(GenericValue, 
String, String, String, String, List, boolean, 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 eas

[jira] [Updated] (OFBIZ-9630) [FB] Package org.apache.ofbiz.common.login

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9630:
-
Attachment: OFBIZ-9630_org.apache.ofbiz.common.login_bugfixes.patch

- Diamond Operators fixed
- removed unnecessary else
- Line 117: added a default Locale to {{toLowerCase}}
- Line 568: {{resultMap}} is used in the lines after its declaration
- Line 659: null was written into {{result}}. that was intended, but this way 
it’s easier to read 
- Line 670, 671, 672: added a default Locale to {{toLowerCase}}
- Line 731: added a default Locale to {{toLowerCase}}
- Line 912: removed {{userLogin != null && currentPassword != null && }} 
because both parameters cannot be null at this point
- Line 939: removed {{ && userLogin != null}} because the parameter cannot be 
null at this point
- Line 984: removed {{messageMap = UtilMisc.toMap("passwordPatternMessage", 
errMsg);}} as it’s never used anymore
- Line 995: removed {{userLogin != null && }} because the parameter cannot be 
null at this point
- Line 998: added default Locale to {{toUpperCase}}


> [FB] Package org.apache.ofbiz.common.login
> --
>
> Key: OFBIZ-9630
> URL: https://issues.apache.org/jira/browse/OFBIZ-9630
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9630_org.apache.ofbiz.common.login_bugfixes.patch
>
>
> - LoginServices.java:118, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.common.login.LoginServices.userLogin(DispatchContext, 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.
> - LoginServices.java:161, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to loginDisableMinutes in 
> org.apache.ofbiz.common.login.LoginServices.userLogin(DispatchContext, 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.
> - LoginServices.java:569, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to resultMap in 
> org.apache.ofbiz.common.login.LoginServices.createUserLogin(DispatchContext, 
> 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.
> - LoginServices.java:661, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.common.login.LoginServices.updatePassword(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).
> - LoginServices.java:671, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.common.login.LoginServices.updatePassword(DispatchContext, 
> 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.
> - LoginServices.java:733, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.common.login.LoginServices.updateUserLoginId(DispatchContext,
>  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.
> - LoginServices.java:906, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
> RCN: Nullcheck of userLogin at line 915 of value previously dereferenced in 
> org.apache.ofbiz.common.login.LoginServices.checkNewPassword(GenericValue, 
> String, String, String, String, List, boolean, Locale)
> A value is checke

[jira] [Created] (OFBIZ-9631) [FB] Package org.apache.ofbiz.common.period

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9631:


 Summary: [FB] Package org.apache.ofbiz.common.period
 Key: OFBIZ-9631
 URL: https://issues.apache.org/jira/browse/OFBIZ-9631
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- PeriodWorker.java:34, MS_SHOULD_BE_FINAL
MS: org.apache.ofbiz.common.period.PeriodWorker.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.



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


[jira] [Updated] (OFBIZ-9631) [FB] Package org.apache.ofbiz.common.period

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9631:
-
Attachment: OFBIZ-9631_org.apache.ofbiz.common.period_bugfixes.patch

- changed the parameter to final and private, to prevent possible vulnerability 
through external code changing


> [FB] Package org.apache.ofbiz.common.period
> ---
>
> Key: OFBIZ-9631
> URL: https://issues.apache.org/jira/browse/OFBIZ-9631
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9631_org.apache.ofbiz.common.period_bugfixes.patch
>
>
> - PeriodWorker.java:34, MS_SHOULD_BE_FINAL
> MS: org.apache.ofbiz.common.period.PeriodWorker.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.



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


[jira] [Created] (OFBIZ-9633) [FB] Package org.apache.ofbiz.common.qrcode

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9633:


 Summary: [FB] Package org.apache.ofbiz.common.qrcode
 Key: OFBIZ-9633
 URL: https://issues.apache.org/jira/browse/OFBIZ-9633
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- QRCodeEvents.java:76, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of mimeType, which is known to be non-null in 
org.apache.ofbiz.common.qrcode.QRCodeEvents.serveQRCodeImage(HttpServletRequest,
 HttpServletResponse)

This method contains a redundant check of a known non-null value against the 
constant null.

- QRCodeServices.java:77, MS_PKGPROTECT
MS: org.apache.ofbiz.common.qrcode.QRCodeServices.FORMAT_NAMES 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.

- QRCodeServices.java:79, MS_MUTABLE_COLLECTION_PKGPROTECT
Field is a mutable collection which should be package protected

A mutable collection instance is assigned to a final static field, thus can be 
changed by malicious code or by accident from another package. The field could 
be made package protected to avoid this vulnerability. Alternatively you may 
wrap this field into Collections.unmodifiableSet/List/Map/etc. to avoid this 
vulnerability.

- QRCodeServices.java:93, MS_SHOULD_BE_REFACTORED_TO_BE_FINAL
MS: org.apache.ofbiz.common.qrcode.QRCodeServices.defaultLogoImage isn't final 
but should be refactored to be so

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. However, the static initializer contains more than one 
write to the field, so doing so will require some refactoring.

- QRCodeServices.java:252, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.common.qrcode.QRCodeServices.toBufferedImage(BitMatrix, 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.



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


[jira] [Updated] (OFBIZ-9633) [FB] Package org.apache.ofbiz.common.qrcode

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9633:
-
Attachment: OFBIZ-9633_org.apache.ofbiz.common.qrcode_bugfixes.patch

- Diamond Operators fixed

class QRCodeEvents:
- Line 51: removed unnecessary casting from {{HttpServletRequest}} to 
{{HttpServletRequest}}
- Line 76: removed unnecessary nullcheck

class QRCodeServices:
- Line 77, 79: made parameters private to prevent vulnerability and external 
code violation
- Line 75: made {{defaultLogoImage}} a final parameter
- refactored the declaration of {{defaultLogoImage}} so that it can be made a 
final parameter
- Line 258: added a default Locale to {{toLowerCase}}


> [FB] Package org.apache.ofbiz.common.qrcode
> ---
>
> Key: OFBIZ-9633
> URL: https://issues.apache.org/jira/browse/OFBIZ-9633
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9633_org.apache.ofbiz.common.qrcode_bugfixes.patch
>
>
> - QRCodeEvents.java:76, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of mimeType, which is known to be non-null in 
> org.apache.ofbiz.common.qrcode.QRCodeEvents.serveQRCodeImage(HttpServletRequest,
>  HttpServletResponse)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - QRCodeServices.java:77, MS_PKGPROTECT
> MS: org.apache.ofbiz.common.qrcode.QRCodeServices.FORMAT_NAMES 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.
> - QRCodeServices.java:79, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - QRCodeServices.java:93, MS_SHOULD_BE_REFACTORED_TO_BE_FINAL
> MS: org.apache.ofbiz.common.qrcode.QRCodeServices.defaultLogoImage isn't 
> final but should be refactored to be so
> 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. However, the static initializer contains more than 
> one write to the field, so doing so will require some refactoring.
> - QRCodeServices.java:252, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.common.qrcode.QRCodeServices.toBufferedImage(BitMatrix, 
> 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.



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


[jira] [Created] (OFBIZ-9634) [FB] Package org.apache.ofbiz.common.uom

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9634:


 Summary: [FB] Package org.apache.ofbiz.common.uom
 Key: OFBIZ-9634
 URL: https://issues.apache.org/jira/browse/OFBIZ-9634
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- UomWorker.java:70, PZLA_PREFER_ZERO_LENGTH_ARRAYS
PZLA: Should org.apache.ofbiz.common.uom.UomWorker.uomTimeToCalTime(String) 
return a zero length array rather than null?

It is often a better design to return a length zero array rather than a null 
reference to indicate that there are no results (i.e., an empty list of 
results). This way, no explicit check for null is needed by clients of the 
method.

On the other hand, using null to indicate "there is no answer to this question" 
is probably appropriate. For example, File.listFiles() returns an empty list if 
given a directory containing no files, and returns null if the file is not a 
directory.

- UomWorker.java:107, DLS_DEAD_LOCAL_STORE
DLS: Dead store to svcOutMap in 
org.apache.ofbiz.common.uom.UomWorker.convertUom(BigDecimal, String, String, 
LocalDispatcher)

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.



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


[jira] [Updated] (OFBIZ-9634) [FB] Package org.apache.ofbiz.common.uom

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9634:
-
Attachment: OFBIZ-9634_org.apache.ofbiz.common.uom_bugfixes.patch

- Diamond Operators fixed
- Line 70: changes not necessary
- Line 107: deleted {{ = new LinkedHashMap<>()}} because it is not necessary at 
this point


> [FB] Package org.apache.ofbiz.common.uom
> 
>
> Key: OFBIZ-9634
> URL: https://issues.apache.org/jira/browse/OFBIZ-9634
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9634_org.apache.ofbiz.common.uom_bugfixes.patch
>
>
> - UomWorker.java:70, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> PZLA: Should org.apache.ofbiz.common.uom.UomWorker.uomTimeToCalTime(String) 
> return a zero length array rather than null?
> It is often a better design to return a length zero array rather than a null 
> reference to indicate that there are no results (i.e., an empty list of 
> results). This way, no explicit check for null is needed by clients of the 
> method.
> On the other hand, using null to indicate "there is no answer to this 
> question" is probably appropriate. For example, File.listFiles() returns an 
> empty list if given a directory containing no files, and returns null if the 
> file is not a directory.
> - UomWorker.java:107, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to svcOutMap in 
> org.apache.ofbiz.common.uom.UomWorker.convertUom(BigDecimal, String, String, 
> LocalDispatcher)
> 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.



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


[jira] [Comment Edited] (OFBIZ-9634) [FB] Package org.apache.ofbiz.common.uom

2017-08-25 Thread Dennis Balkir (JIRA)

[ 
https://issues.apache.org/jira/browse/OFBIZ-9634?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16141619#comment-16141619
 ] 

Dennis Balkir edited comment on OFBIZ-9634 at 8/25/17 1:35 PM:
---

- Diamond Operators fixed
- Line 70: changes not necessary
- Line 107: deleted {{= new LinkedHashMap<>()}} because it is not necessary at 
this point



was (Author: dennis balkir):
- Diamond Operators fixed
- Line 70: changes not necessary
- Line 107: deleted {{ = new LinkedHashMap<>()}} because it is not necessary at 
this point


> [FB] Package org.apache.ofbiz.common.uom
> 
>
> Key: OFBIZ-9634
> URL: https://issues.apache.org/jira/browse/OFBIZ-9634
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9634_org.apache.ofbiz.common.uom_bugfixes.patch
>
>
> - UomWorker.java:70, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> PZLA: Should org.apache.ofbiz.common.uom.UomWorker.uomTimeToCalTime(String) 
> return a zero length array rather than null?
> It is often a better design to return a length zero array rather than a null 
> reference to indicate that there are no results (i.e., an empty list of 
> results). This way, no explicit check for null is needed by clients of the 
> method.
> On the other hand, using null to indicate "there is no answer to this 
> question" is probably appropriate. For example, File.listFiles() returns an 
> empty list if given a directory containing no files, and returns null if the 
> file is not a directory.
> - UomWorker.java:107, DLS_DEAD_LOCAL_STORE
> DLS: Dead store to svcOutMap in 
> org.apache.ofbiz.common.uom.UomWorker.convertUom(BigDecimal, String, String, 
> LocalDispatcher)
> 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.



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


[jira] [Created] (OFBIZ-9635) [FB] Package org.apache.ofbiz.security

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9635:


 Summary: [FB] Package org.apache.ofbiz.security
 Key: OFBIZ-9635
 URL: https://issues.apache.org/jira/browse/OFBIZ-9635
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


SecurityFactory.java:-1, CI_CONFUSED_INHERITANCE, Priorität: Niedrig
CI: Class org.apache.ofbiz.security.SecurityFactory$OFBizSecurity is final but 
declares protected field 
org.apache.ofbiz.security.SecurityFactory$OFBizSecurity.simpleRoleEntity

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.



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


[jira] [Updated] (OFBIZ-9635) [FB] Package org.apache.ofbiz.security

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9635:
-
Attachment: OFBIZ-9635_org.apache.ofbiz.security_bugfixes.patch

- Diamond Operators fixed
- Line 90: changed the assigned field to private, since it is a final class and 
protected is a confusing declaration at this point and the field is only used 
in this class

PS: package security isn't available to choose from, while creating a new sub

> [FB] Package org.apache.ofbiz.security
> --
>
> Key: OFBIZ-9635
> URL: https://issues.apache.org/jira/browse/OFBIZ-9635
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9635_org.apache.ofbiz.security_bugfixes.patch
>
>
> SecurityFactory.java:-1, CI_CONFUSED_INHERITANCE, Priorität: Niedrig
> CI: Class org.apache.ofbiz.security.SecurityFactory$OFBizSecurity is final 
> but declares protected field 
> org.apache.ofbiz.security.SecurityFactory$OFBizSecurity.simpleRoleEntity
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.



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


[jira] [Created] (OFBIZ-9636) Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9636:


 Summary: Incomplete RoleMember Permission Checks for ROLE_MEMBER 
in evalRoleMember
 Key: OFBIZ-9636
 URL: https://issues.apache.org/jira/browse/OFBIZ-9636
 Project: OFBiz
  Issue Type: Improvement
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir


incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
this method always and only returning false. This method was last changed by 
jaz in the year 2009.
I am deleting the method and all of its calls, because it obviously is never 
used, since its last changes are very long ago and since then its kind of 
useless



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


[jira] [Updated] (OFBIZ-9636) Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9636:
-
Priority: Minor  (was: Major)

> Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember
> -
>
> Key: OFBIZ-9636
> URL: https://issues.apache.org/jira/browse/OFBIZ-9636
> Project: OFBiz
>  Issue Type: Improvement
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
> this method always and only returning false. This method was last changed by 
> jaz in the year 2009.
> I am deleting the method and all of its calls, because it obviously is never 
> used, since its last changes are very long ago and since then its kind of 
> useless



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


[jira] [Updated] (OFBIZ-9636) Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9636:
-
Description: 
incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
this method always and only returning false. This method was last changed by 
jaz in the year 2009.
I am deleting the method and all of its calls, because it obviously is never 
used, since its last changes are very long ago and since then its kind of 
useless

PS: you cannot choose package service, when creating new issues

  was:
incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
this method always and only returning false. This method was last changed by 
jaz in the year 2009.
I am deleting the method and all of its calls, because it obviously is never 
used, since its last changes are very long ago and since then its kind of 
useless


> Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember
> -
>
> Key: OFBIZ-9636
> URL: https://issues.apache.org/jira/browse/OFBIZ-9636
> Project: OFBiz
>  Issue Type: Improvement
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9636_incomplete-role-member-evaluation-method.patch
>
>
> incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
> this method always and only returning false. This method was last changed by 
> jaz in the year 2009.
> I am deleting the method and all of its calls, because it obviously is never 
> used, since its last changes are very long ago and since then its kind of 
> useless
> PS: you cannot choose package service, when creating new issues



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


[jira] [Updated] (OFBIZ-9636) Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9636:
-
Attachment: OFBIZ-9636_incomplete-role-member-evaluation-method.patch

class ModelPermission:
- removed method {{evalRoleMember}}; it was mostly useless, and just returned 
false every time is was called (it got changed by jaz in 2009, since then it 
doesn’t do anything except returning false, and since it wasn’t used that long 
and nobody seemed to need it for 8 years, I decided to remove it and all of its 
calls completely)
-  removed the case with permissionType {{ROLE_MEMBER}} in {{evalPermission}}
- removed the static parameter {{ROLE_MEMBER}}

class ModelServiceReader:
- removed the part for creating the role-member permissions in 
{{createGroupPermissions}}


> Incomplete RoleMember Permission Checks for ROLE_MEMBER in evalRoleMember
> -
>
> Key: OFBIZ-9636
> URL: https://issues.apache.org/jira/browse/OFBIZ-9636
> Project: OFBiz
>  Issue Type: Improvement
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9636_incomplete-role-member-evaluation-method.patch
>
>
> incomplete method {{evalRoleMember(GenericValue userLogin)}} which results in 
> this method always and only returning false. This method was last changed by 
> jaz in the year 2009.
> I am deleting the method and all of its calls, because it obviously is never 
> used, since its last changes are very long ago and since then its kind of 
> useless
> PS: you cannot choose package service, when creating new issues



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


[jira] [Created] (OFBIZ-9637) [FB] Package org.apache.ofbiz.securityext.login

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9637:


 Summary: [FB] Package org.apache.ofbiz.securityext.login
 Key: OFBIZ-9637
 URL: https://issues.apache.org/jira/browse/OFBIZ-9637
 Project: OFBiz
  Issue Type: Sub-task
  Components: securityext
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- LoginEvents.java:88, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.securityext.login.LoginEvents.saveEntryParams(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.

- LoginEvents.java:162, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.securityext.login.LoginEvents.showPasswordHint(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.

- LoginEvents.java:222, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.securityext.login.LoginEvents.emailPassword(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.

- LoginEvents.java:417, DMI_INVOKING_TOSTRING_ON_ARRAY
USELESS_STRING: Invocation of toString on cookies in 
org.apache.ofbiz.securityext.login.LoginEvents.getUsername(HttpServletRequest)

The code invokes toString on an array, which will generate a fairly useless 
result such as [C@16f0472. Consider using Arrays.toString to convert the array 
into a readable String that gives the contents of the array. See Programming 
Puzzlers, chapter 3, puzzle 12.

- LoginEvents.java:437, HRS_REQUEST_PARAMETER_TO_COOKIE
HRS: HTTP cookie formed from untrusted input in 
org.apache.ofbiz.securityext.login.LoginEvents.setUsername(HttpServletRequest, 
HttpServletResponse)

This code constructs an HTTP Cookie using an untrusted HTTP parameter. If this 
cookie is added to an HTTP response, it will allow a HTTP response splitting 
vulnerability. See http://en.wikipedia.org/wiki/HTTP_response_splitting for 
more information.

FindBugs looks only for the most blatant, obvious cases of HTTP response 
splitting. If FindBugs found any, you almost certainly have more 
vulnerabilities that FindBugs doesn't report. If you are concerned about HTTP 
response splitting, you should seriously consider using a commercial static 
analysis or pen-testing tool.



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


[jira] [Updated] (OFBIZ-9637) [FB] Package org.apache.ofbiz.securityext.login

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9637:
-
Attachment: OFBIZ-9637_org.apache.ofbiz.securityext_bugfixes.patch

- Line 89: added a default Locale to {{toLowerCase}}
- Line 92: added a default Locale to {{toLowerCase}}
- Line 163: added a default Locale to {{toLowerCase}}
- Line 223: added a default Locale to {{toLowerCase}}
- Line 248: added a default Locale to {{toLowerCase}}
- Line 253: removed unnecessary casting from String to Object
- Line 284: deleted unnecessary allocation of null -> if the try is executed 
properly, party won’t get allocated with null, it it isn’t executed properly 
and the program executes the catch, party is already null since it was declared 
as null in line 279
- Line 419: changed the code, so that {{cookies}} is casted to a String properly

- Last error: added an encoder, which prevents vulnerability through using 
untrusted parameters to construct a HTTPCookie

> [FB] Package org.apache.ofbiz.securityext.login
> ---
>
> Key: OFBIZ-9637
> URL: https://issues.apache.org/jira/browse/OFBIZ-9637
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: securityext
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9637_org.apache.ofbiz.securityext_bugfixes.patch
>
>
> - LoginEvents.java:88, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.securityext.login.LoginEvents.saveEntryParams(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.
> - LoginEvents.java:162, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.securityext.login.LoginEvents.showPasswordHint(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.
> - LoginEvents.java:222, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.securityext.login.LoginEvents.emailPassword(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.
> - LoginEvents.java:417, DMI_INVOKING_TOSTRING_ON_ARRAY
> USELESS_STRING: Invocation of toString on cookies in 
> org.apache.ofbiz.securityext.login.LoginEvents.getUsername(HttpServletRequest)
> The code invokes toString on an array, which will generate a fairly useless 
> result such as [C@16f0472. Consider using Arrays.toString to convert the 
> array into a readable String that gives the contents of the array. See 
> Programming Puzzlers, chapter 3, puzzle 12.
> - LoginEvents.java:437, HRS_REQUEST_PARAMETER_TO_COOKIE
> HRS: HTTP cookie formed from untrusted input in 
> org.apache.ofbiz.securityext.login.LoginEvents.setUsername(HttpServletRequest,
>  HttpServletResponse)
> This code constructs an HTTP Cookie using an untrusted HTTP parameter. If 
> this cookie is added to an HTTP response, it will allow a HTTP response 
> splitting vulnerability. See 
> http://en.wikipedia.org/wiki/HTTP_response_splitting for more information.
> FindBugs looks only for the most blatant, obvious cases of HTTP response 
> splitting. If FindBugs found any, you almost certainly have more 
> vulnerabilities that FindBugs doesn't report. If you are concerned about HTTP 
> response splitting, you should seriously consider using a commercial static 
> analysis or pen-testing tool.



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


[jira] [Created] (OFBIZ-9638) [FB] Package org.apache.ofbiz.service

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9638:


 Summary: [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


- 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

[jira] [Updated] (OFBIZ-9638) [FB] Package org.apache.ofbiz.service

2017-08-25 Thread Dennis Balkir (JIRA)

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

[jira] [Updated] (OFBIZ-9639) [FB] Package org.apache.ofbiz.catalina.container

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9639:
-
Attachment: OFBIZ-9639_org.apache.ofbiz.catalina_bugfixes.patch

- Diamond Operators fixed
- deleted unnecessary else-blocks
- Line 247: deleted unnecessary nullcheck of {{clusterProps}}


> [FB] Package org.apache.ofbiz.catalina.container
> 
>
> Key: OFBIZ-9639
> URL: https://issues.apache.org/jira/browse/OFBIZ-9639
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9639_org.apache.ofbiz.catalina_bugfixes.patch
>
>
> - CatalinaContainer.java:248, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of clusterProps, which is known to be non-null in 
> org.apache.ofbiz.catalina.container.CatalinaContainer.prepareTomcatClustering(Host,
>  ContainerConfig$Configuration$Property)
> This method contains a redundant check of a known non-null value against the 
> constant null.



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


[jira] [Created] (OFBIZ-9639) [FB] Package org.apache.ofbiz.catalina.container

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9639:


 Summary: [FB] Package org.apache.ofbiz.catalina.container
 Key: OFBIZ-9639
 URL: https://issues.apache.org/jira/browse/OFBIZ-9639
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- CatalinaContainer.java:248, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of clusterProps, which is known to be non-null in 
org.apache.ofbiz.catalina.container.CatalinaContainer.prepareTomcatClustering(Host,
 ContainerConfig$Configuration$Property)

This method contains a redundant check of a known non-null value against the 
constant null.



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


[jira] [Created] (OFBIZ-9640) [FB] Package org.apache.ofbiz.common.preferences

2017-08-25 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9640:


 Summary: [FB] Package org.apache.ofbiz.common.preferences
 Key: OFBIZ-9640
 URL: https://issues.apache.org/jira/browse/OFBIZ-9640
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- PreferenceWorker.java:90, RpC_REPEATED_CONDITIONAL_TEST
RpC: Repeated conditional test in 
org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
 Map)

The code contains a conditional test is performed twice, one right after the 
other (e.g., x == 0 || x == 0). Perhaps the second occurrence is intended to be 
something else (e.g., x == 0 || y == 0).

- PreferenceWorker.java:90, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of userLoginIdArg, which is known to be non-null in 
org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
 Map)

This method contains a redundant check of a known non-null value against the 
constant null.



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


[jira] [Updated] (OFBIZ-9640) [FB] Package org.apache.ofbiz.common.preferences

2017-08-25 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9640:
-
Attachment: OFBIZ-9640_org.apache.ofbiz.common.preferences_bugfixes.patch

- Diamond Operators fixed
- Line 90: removed {{userLoginIdArg != null && }}, because it was unnecessary


> [FB] Package org.apache.ofbiz.common.preferences
> 
>
> Key: OFBIZ-9640
> URL: https://issues.apache.org/jira/browse/OFBIZ-9640
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9640_org.apache.ofbiz.common.preferences_bugfixes.patch
>
>
> - PreferenceWorker.java:90, RpC_REPEATED_CONDITIONAL_TEST
> RpC: Repeated conditional test in 
> org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
>  Map)
> The code contains a conditional test is performed twice, one right after the 
> other (e.g., x == 0 || x == 0). Perhaps the second occurrence is intended to 
> be something else (e.g., x == 0 || y == 0).
> - PreferenceWorker.java:90, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of userLoginIdArg, which is known to be non-null in 
> org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
>  Map)
> This method contains a redundant check of a known non-null value against the 
> constant null.



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


[jira] [Comment Edited] (OFBIZ-9640) [FB] Package org.apache.ofbiz.common.preferences

2017-08-25 Thread Dennis Balkir (JIRA)

[ 
https://issues.apache.org/jira/browse/OFBIZ-9640?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16141717#comment-16141717
 ] 

Dennis Balkir edited comment on OFBIZ-9640 at 8/25/17 2:56 PM:
---

- Diamond Operators fixed
- Line 90: removed {{userLoginIdArg != null &&}}, because it was unnecessary



was (Author: dennis balkir):
- Diamond Operators fixed
- Line 90: removed {{userLoginIdArg != null && }}, because it was unnecessary


> [FB] Package org.apache.ofbiz.common.preferences
> 
>
> Key: OFBIZ-9640
> URL: https://issues.apache.org/jira/browse/OFBIZ-9640
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9640_org.apache.ofbiz.common.preferences_bugfixes.patch
>
>
> - PreferenceWorker.java:90, RpC_REPEATED_CONDITIONAL_TEST
> RpC: Repeated conditional test in 
> org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
>  Map)
> The code contains a conditional test is performed twice, one right after the 
> other (e.g., x == 0 || x == 0). Perhaps the second occurrence is intended to 
> be something else (e.g., x == 0 || y == 0).
> - PreferenceWorker.java:90, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of userLoginIdArg, which is known to be non-null in 
> org.apache.ofbiz.common.preferences.PreferenceWorker.checkCopyPermission(DispatchContext,
>  Map)
> This method contains a redundant check of a known non-null value against the 
> constant null.



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


[jira] [Created] (OFBIZ-9679) [FB] Package org.apache.ofbiz.base.conversion

2017-09-06 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9679:


 Summary: [FB] Package org.apache.ofbiz.base.conversion
 Key: OFBIZ-9679
 URL: https://issues.apache.org/jira/browse/OFBIZ-9679
 Project: OFBiz
  Issue Type: Sub-task
  Components: base
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- BooleanConverters.java:72, DM_CONVERT_CASE

Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.base.conversion.BooleanConverters$StringToBoolean.convert(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.

- CollectionConverters.java:46, BC_VACUOUS_INSTANCEOF

BC: instanceof will always return true for all nonnull values in 
org.apache.ofbiz.base.conversion.CollectionConverters$ArrayCreator.createConverter(Class,
 Class), since all Class are instances of Object

This instanceof test will always return true (unless the value being tested is 
null). Although this is safe, make sure it isn't an indication of some 
misunderstanding or some other logic error. If you really want to test the 
value for being null, perhaps it would be clearer to do better to do a null 
test rather than an instanceof test.

- Converters.java:39, MS_MUTABLE_COLLECTION_PKGPROTECT

Field is a mutable collection which should be package protected

A mutable collection instance is assigned to a final static field, thus can be 
changed by malicious code or by accident from another package. The field could 
be made package protected to avoid this vulnerability. Alternatively you may 
wrap this field into Collections.unmodifiableSet/List/Map/etc. to avoid this 
vulnerability.

- Converters.java:40, MS_MUTABLE_COLLECTION_PKGPROTECT

Field is a mutable collection which should be package protected

A mutable collection instance is assigned to a final static field, thus can be 
changed by malicious code or by accident from another package. The field could 
be made package protected to avoid this vulnerability. Alternatively you may 
wrap this field into Collections.unmodifiableSet/List/Map/etc. to avoid this 
vulnerability.

- Converters.java:154, REC_CATCH_EXCEPTION

REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.base.conversion.Converters.loadContainedConverters(Class)

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 ...
  }

- MiscConverters.java:90, PZLA_PREFER_ZERO_LENGTH_ARRAYS

PZLA: Should 
org.apache.ofbiz.base.conversion.MiscConverters$ByteBufferToByteArray.convert(ByteBuffer)
 return a zero length array rather than null?

It is often a better design to return a length zero array rather than a null 
reference to indicate that there are no results (i.e., an empty list of 
results). This way, no explicit check for null is needed by clients of the 
method.

On the other hand, using null to indicate "there is no answer to this question" 
is probably appropriate. For example, File.listFiles() returns an empty list if 
given a directory containing no files, and returns null if the file is not a 
directory.



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


[jira] [Updated] (OFBIZ-9679) [FB] Package org.apache.ofbiz.base.conversion

2017-09-06 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9679:
-
Attachment: OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch

- Diamond Operators fixed

class BooleanConverters:
- Line 72: added a default Locale to {{toUpperCase}}

class CollectionConverters:
- Line 46: changed the not instanceof object check to a null-check which seems 
like it was intended to be

class Converters:
- Line 39, 40: changed the parameters from {{protected}} to {{private}}
- Line 154: added a catch for a {{RuntimeException}}

> [FB] Package org.apache.ofbiz.base.conversion
> -
>
> Key: OFBIZ-9679
> URL: https://issues.apache.org/jira/browse/OFBIZ-9679
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch
>
>
> - BooleanConverters.java:72, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.base.conversion.BooleanConverters$StringToBoolean.convert(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.
> - CollectionConverters.java:46, BC_VACUOUS_INSTANCEOF
> BC: instanceof will always return true for all nonnull values in 
> org.apache.ofbiz.base.conversion.CollectionConverters$ArrayCreator.createConverter(Class,
>  Class), since all Class are instances of Object
> This instanceof test will always return true (unless the value being tested 
> is null). Although this is safe, make sure it isn't an indication of some 
> misunderstanding or some other logic error. If you really want to test the 
> value for being null, perhaps it would be clearer to do better to do a null 
> test rather than an instanceof test.
> - Converters.java:39, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:40, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:154, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.base.conversion.Converters.loadContainedConverters(Class)
> 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 ...
>   }
> - MiscConverters.java:90, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> PZLA: Should 
> org.apache.ofbiz.base.conversion.MiscConverters$ByteBufferToByteArray.convert(ByteBuffer)
>  return a zero length array rather than null?
> It is often a better design to return a length zero array rather than a null 
> reference to indicate that there are no results (i.e., an empty list of 
> results). This way, no explicit check for null is needed by clients of the 
> method.
> On the other hand, using null to indicate "there is no answer to this 
> question" is probably appropriate. For example, File.listFiles() returns an 
> empty list if given a directory containing no files, and returns null if the 
> file is not a directory.



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


[jira] [Updated] (OFBIZ-9679) [FB] Package org.apache.ofbiz.base.conversion

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9679:
-
Attachment: OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch

> [FB] Package org.apache.ofbiz.base.conversion
> -
>
> Key: OFBIZ-9679
> URL: https://issues.apache.org/jira/browse/OFBIZ-9679
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch, 
> OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch
>
>
> - BooleanConverters.java:72, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.base.conversion.BooleanConverters$StringToBoolean.convert(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.
> - CollectionConverters.java:46, BC_VACUOUS_INSTANCEOF
> BC: instanceof will always return true for all nonnull values in 
> org.apache.ofbiz.base.conversion.CollectionConverters$ArrayCreator.createConverter(Class,
>  Class), since all Class are instances of Object
> This instanceof test will always return true (unless the value being tested 
> is null). Although this is safe, make sure it isn't an indication of some 
> misunderstanding or some other logic error. If you really want to test the 
> value for being null, perhaps it would be clearer to do better to do a null 
> test rather than an instanceof test.
> - Converters.java:39, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:40, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:154, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.base.conversion.Converters.loadContainedConverters(Class)
> 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 ...
>   }
> - MiscConverters.java:90, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> PZLA: Should 
> org.apache.ofbiz.base.conversion.MiscConverters$ByteBufferToByteArray.convert(ByteBuffer)
>  return a zero length array rather than null?
> It is often a better design to return a length zero array rather than a null 
> reference to indicate that there are no results (i.e., an empty list of 
> results). This way, no explicit check for null is needed by clients of the 
> method.
> On the other hand, using null to indicate "there is no answer to this 
> question" is probably appropriate. For example, File.listFiles() returns an 
> empty list if given a directory containing no files, and returns null if the 
> file is not a directory.



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


[jira] [Comment Edited] (OFBIZ-9679) [FB] Package org.apache.ofbiz.base.conversion

2017-09-08 Thread Dennis Balkir (JIRA)

[ 
https://issues.apache.org/jira/browse/OFBIZ-9679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16155314#comment-16155314
 ] 

Dennis Balkir edited comment on OFBIZ-9679 at 9/8/17 8:48 AM:
--

- Diamond Operators fixed

class BooleanConverters:
- Line 72: added a default Locale to {{toUpperCase}}

class CollectionConverters:
- Line 46: changed the not instanceof object check to a null-check which seems 
like it was intended to be

class Converters:
- Line 39, 40: changed the parameters from {{protected}} to {{private}}


was (Author: dennis balkir):
- Diamond Operators fixed

class BooleanConverters:
- Line 72: added a default Locale to {{toUpperCase}}

class CollectionConverters:
- Line 46: changed the not instanceof object check to a null-check which seems 
like it was intended to be

class Converters:
- Line 39, 40: changed the parameters from {{protected}} to {{private}}
- Line 154: added a catch for a {{RuntimeException}}

> [FB] Package org.apache.ofbiz.base.conversion
> -
>
> Key: OFBIZ-9679
> URL: https://issues.apache.org/jira/browse/OFBIZ-9679
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: base
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch, 
> OFBIZ-9679_org.apache.ofbiz.base.conversion_bugfixes.patch
>
>
> - BooleanConverters.java:72, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.base.conversion.BooleanConverters$StringToBoolean.convert(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.
> - CollectionConverters.java:46, BC_VACUOUS_INSTANCEOF
> BC: instanceof will always return true for all nonnull values in 
> org.apache.ofbiz.base.conversion.CollectionConverters$ArrayCreator.createConverter(Class,
>  Class), since all Class are instances of Object
> This instanceof test will always return true (unless the value being tested 
> is null). Although this is safe, make sure it isn't an indication of some 
> misunderstanding or some other logic error. If you really want to test the 
> value for being null, perhaps it would be clearer to do better to do a null 
> test rather than an instanceof test.
> - Converters.java:39, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:40, MS_MUTABLE_COLLECTION_PKGPROTECT
> Field is a mutable collection which should be package protected
> A mutable collection instance is assigned to a final static field, thus can 
> be changed by malicious code or by accident from another package. The field 
> could be made package protected to avoid this vulnerability. Alternatively 
> you may wrap this field into Collections.unmodifiableSet/List/Map/etc. to 
> avoid this vulnerability.
> - Converters.java:154, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.base.conversion.Converters.loadContainedConverters(Class)
> 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 ...
>   }
> - MiscConverters.java:90, PZLA_PREFER_ZERO_LENGTH_ARRAYS
> PZLA: Should 
> org.apache.ofbiz.base.conversion.MiscConverters$ByteBufferToByteArray.convert(ByteBuffer)
>  return a zero length array rather than null?
> It is often a better design to return a length zero array rather than a null 
> reference to indicate that there are no results (i.e., an empty 

[jira] [Created] (OFBIZ-9688) [FB] Package org.apache.ofbiz.service.engine

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9688:


 Summary: [FB] Package org.apache.ofbiz.service.engine
 Key: OFBIZ-9688
 URL: https://issues.apache.org/jira/browse/OFBIZ-9688
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- GenericEngineFactory.java:67, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.service.engine.GenericEngineFactory.getGenericEngine(String)

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 ...
  }

- GroovyEngine.java:-1, CI_CONFUSED_INHERITANCE
CI: Class org.apache.ofbiz.service.engine.GroovyEngine is final but declares 
protected field org.apache.ofbiz.service.engine.GroovyEngine.EMPTY_ARGS

This class is declared to be final, but declares fields to be protected. Since 
the class is final, it can not be derived from, and the use of protected is 
confusing. The access modifier for the field should be changed to private or 
public to represent the true use for the field.

- HttpEngine.java:64, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.service.engine.HttpEngine.runSync(String, ModelService, Map)

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 ...
  }

- HttpEngine.java:137, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.service.engine.HttpEngine.httpEngine(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 ...
  }

- HttpEngine.java:185, DM_DEFAULT_ENCODING
Dm: Found reliance on default encoding in 
org.apache.ofbiz.service.engine.HttpEngine.httpEngine(HttpServletRequest, 
HttpServletResponse): String.getBytes()

Found a call to a method which will perform a byte to String (or String to 
byte) conversion, and will assume that the default platform encoding is 
suitable. This will cause the application behaviour to vary between platforms. 
Use an alternative API and specify a charset name or Charset object explicitly.

- SOAPClientEngine.java:135, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.service.engine.SOAPClientEngine.serviceInvoker(ModelService, 
Map)

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 w

[jira] [Updated] (OFBIZ-9688) [FB] Package org.apache.ofbiz.service.engine

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9688:
-
Attachment: OFBIZ-9688_org.apache.ofbiz.service.engine_bugfixes.patch

- Diamond Operators fixed

class GroovyEngine:
- Line 51: changed the parameter from protected to private

class HttpEngine:
- Line 191: added a Charset to {{getByte}}
- Line 194: added a Charset to {{getByte}}


> [FB] Package org.apache.ofbiz.service.engine
> 
>
> Key: OFBIZ-9688
> URL: https://issues.apache.org/jira/browse/OFBIZ-9688
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9688_org.apache.ofbiz.service.engine_bugfixes.patch
>
>
> - GenericEngineFactory.java:67, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.service.engine.GenericEngineFactory.getGenericEngine(String)
> 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 ...
>   }
> - GroovyEngine.java:-1, CI_CONFUSED_INHERITANCE
> CI: Class org.apache.ofbiz.service.engine.GroovyEngine is final but declares 
> protected field org.apache.ofbiz.service.engine.GroovyEngine.EMPTY_ARGS
> This class is declared to be final, but declares fields to be protected. 
> Since the class is final, it can not be derived from, and the use of 
> protected is confusing. The access modifier for the field should be changed 
> to private or public to represent the true use for the field.
> - HttpEngine.java:64, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.service.engine.HttpEngine.runSync(String, ModelService, Map)
> 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 ...
>   }
> - HttpEngine.java:137, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.service.engine.HttpEngine.httpEngine(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 ...
>   }
> - HttpEngine.java:185, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.service.engine.HttpEngine.httpEngine(HttpServletRequest, 
> HttpServletResponse): String.getBytes()
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API an

[jira] [Created] (OFBIZ-9690) [FB] Package org.apache.ofbiz.service.mail

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9690:


 Summary: [FB] Package org.apache.ofbiz.service.mail
 Key: OFBIZ-9690
 URL: https://issues.apache.org/jira/browse/OFBIZ-9690
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- JavaMailContainer.java:127, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of store, which is known to be non-null in 
org.apache.ofbiz.service.mail.JavaMailContainer.start()

This method contains a redundant check of a known non-null value against the 
constant null.

- JavaMailContainer.java:167, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.service.mail.JavaMailContainer.makeSession(ContainerConfig$Configuration$Property)

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.

- JavaMailContainer.java:245, DM_BOXED_PRIMITIVE_FOR_PARSING
Bx: Boxing/unboxing to parse a primitive 
org.apache.ofbiz.service.mail.JavaMailContainer.updateUrlName(URLName, 
Properties)

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.

- JavaMailContainer.java:269, SIC_INNER_SHOULD_BE_STATIC
SIC: Should 
org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener 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.

- JavaMailContainer.java:274, SF_SWITCH_NO_DEFAULT
SF: Switch statement found in 
org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener.notification(StoreEvent)
 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.

- MimeMessageWrapper.java:50, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.mail.MimeMessageWrapper 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.

- MimeMessageWrapper.java:68, IS2_INCONSISTENT_SYNC
IS: Inconsistent synchronization of 
org.apache.ofbiz.service.mail.MimeMessageWrapper.session; locked 75% 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.

- MimeMessageWrapper.java:69, IS2_INCONSISTENT_SYNC
IS: Inconsistent synchronization of 
org.apache.ofbiz.service.mail.MimeMessageWrapper.mailProperties; 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

[jira] [Updated] (OFBIZ-9690) [FB] Package org.apache.ofbiz.service.mail

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9690:
-
Attachment: OFBIZ-9690_org.apache.ofbiz.service.mail_bugfixes.patch

- Diamond Operator fixed

class JavaMailContainer:
- Line 127: removed unnecessary null-check
- Line 165: added a default Locale to {{toLowerCase()}}
- Line 244: changed {{valueOf()}} to {{parseInt()}} because there is no 
unnecessary boxing/unboxing this way
- Line 253: changed {{valueOf()}} to {{parseInt()}} because there is no 
unnecessary boxing/unboxing this way
- Line 256: made the inner class static
- Line 281: added a default case

class MimeMessageWrapper:
- removed unnecessary else
- Line 50: it’s not necessary for the class to implement serialVerisonUID
- Line 67: made the method {{setSession}} synchronized
- Line 79: made the method {{setMessage}} synchronized
- Line 98: changed the catches to a multicatch
- Line 142: at every call the return gets checked for null, so this is okay
- Line 152: at every call the return gets checked for null, so this is okay
- Line 162: at every call the return gets checked for null, so this is okay
- Line 172: at every call the return gets checked for null, so this is okay
- Line 182: at every call the return gets checked for null, so this is okay
- Line 226: made the method {{getContentType}} synchronized
- Line 230: made the method {{getMainPartCount}} synchronized
- Line 293: added a default Locale to {{toLowerCase}}
- Line 300: added a default Locale to {{toLowerCase}}
- Line 313: added a default Locale to {{toLowerCase}}
- Line 322: added a default Locale to {{toLowerCase}}
- Line 329: added a default Locale to {{toLowerCase}}
- Line 445: changed the dot from string to char, since it is only one char
- Line 513: added a default charset to {{new String}}

class ServiceMcaAction:
- removed unnecessary else
- Line 36: it’s not necessary for the class to implement serialVerisonUID

class ServiceMcaCondition:
- removed unnecessary else
- Line 44: it’s not necessary for the class to implement serialVerisonUID
- Line 76: added a default case to the switch

class ServiceMcaRule:
- Line 35: it’s not necessary for the class to implement serialVerisonUID


> [FB] Package org.apache.ofbiz.service.mail
> --
>
> Key: OFBIZ-9690
> URL: https://issues.apache.org/jira/browse/OFBIZ-9690
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9690_org.apache.ofbiz.service.mail_bugfixes.patch
>
>
> - JavaMailContainer.java:127, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of store, which is known to be non-null in 
> org.apache.ofbiz.service.mail.JavaMailContainer.start()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - JavaMailContainer.java:167, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.service.mail.JavaMailContainer.makeSession(ContainerConfig$Configuration$Property)
> 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.
> - JavaMailContainer.java:245, DM_BOXED_PRIMITIVE_FOR_PARSING
> Bx: Boxing/unboxing to parse a primitive 
> org.apache.ofbiz.service.mail.JavaMailContainer.updateUrlName(URLName, 
> Properties)
> 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.
> - JavaMailContainer.java:269, SIC_INNER_SHOULD_BE_STATIC
> SIC: Should 
> org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener 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.
> - JavaMailContainer.java:274, SF_SWITCH_NO_DEFAULT
> SF: Switch statement found in 
> org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener.notification(StoreEvent)
>  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.
> - MimeMessageWrapper.java:50, SE

[jira] [Created] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.mail

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9691:


 Summary: [FB] Package org.apache.ofbiz.service.mail
 Key: OFBIZ-9691
 URL: https://issues.apache.org/jira/browse/OFBIZ-9691
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- JavaMailContainer.java:127, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of store, which is known to be non-null in 
org.apache.ofbiz.service.mail.JavaMailContainer.start()

This method contains a redundant check of a known non-null value against the 
constant null.

- JavaMailContainer.java:167, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.service.mail.JavaMailContainer.makeSession(ContainerConfig$Configuration$Property)

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.

- JavaMailContainer.java:245, DM_BOXED_PRIMITIVE_FOR_PARSING
Bx: Boxing/unboxing to parse a primitive 
org.apache.ofbiz.service.mail.JavaMailContainer.updateUrlName(URLName, 
Properties)

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.

- JavaMailContainer.java:269, SIC_INNER_SHOULD_BE_STATIC
SIC: Should 
org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener 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.

- JavaMailContainer.java:274, SF_SWITCH_NO_DEFAULT
SF: Switch statement found in 
org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener.notification(StoreEvent)
 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.

- MimeMessageWrapper.java:50, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.mail.MimeMessageWrapper 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.

- MimeMessageWrapper.java:68, IS2_INCONSISTENT_SYNC
IS: Inconsistent synchronization of 
org.apache.ofbiz.service.mail.MimeMessageWrapper.session; locked 75% 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.

- MimeMessageWrapper.java:69, IS2_INCONSISTENT_SYNC
IS: Inconsistent synchronization of 
org.apache.ofbiz.service.mail.MimeMessageWrapper.mailProperties; 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

[jira] [Updated] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.calendar

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9691:
-
Description: 
- ExpressionUiHelper.java:42, MS_PKGPROTECT
MS: org.apache.ofbiz.service.calendar.ExpressionUiHelper.Occurrence 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.

- RecurrenceInfo.java:-1, SE_BAD_FIELD
Se: Class org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 
defines non-transient non-serializable instance field info

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.

- RecurrenceInfo.java:117, EI_EXPOSE_REP
EI: org.apache.ofbiz.service.calendar.RecurrenceInfo.getStartDate() may expose 
internal representation by returning RecurrenceInfo.startDate

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.

- RecurrenceInfo.java:349, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 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.

- RecurrenceRule.java:197, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.service.calendar.RecurrenceRule.getFrequencyName()

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.

- RecurrenceRule.java:321, SF_SWITCH_NO_DEFAULT
SF: Switch statement found in 
org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, long) 
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.

- RecurrenceRule.java:335, SF_SWITCH_FALLTHROUGH
SF: Switch statement found in 
org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, long) 
where one case falls through to the next case

This method contains a switch statement where one case branch will fall through 
to the next case. Usually you need to end this case with a break or return.

- RecurrenceRule.java:724, NP_NULL_ON_SOME_PATH
NP: Possible null pointer dereference of day in 
org.apache.ofbiz.service.calendar.RecurrenceRule.getCalendarDay(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.

- TemporalExpression.java:47, EQ_COMPARETO_USE_OBJECT_EQUALS
Eq: org.apache.ofbiz.service.calendar.TemporalExpression defines 
compareTo(TemporalExpression) 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 str

[jira] [Updated] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.calendar

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9691:
-
Summary: [FB] Package org.apache.ofbiz.service.calendar  (was: [FB] Package 
org.apache.ofbiz.service.mail)

> [FB] Package org.apache.ofbiz.service.calendar
> --
>
> Key: OFBIZ-9691
> URL: https://issues.apache.org/jira/browse/OFBIZ-9691
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> - JavaMailContainer.java:127, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of store, which is known to be non-null in 
> org.apache.ofbiz.service.mail.JavaMailContainer.start()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - JavaMailContainer.java:167, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.service.mail.JavaMailContainer.makeSession(ContainerConfig$Configuration$Property)
> 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.
> - JavaMailContainer.java:245, DM_BOXED_PRIMITIVE_FOR_PARSING
> Bx: Boxing/unboxing to parse a primitive 
> org.apache.ofbiz.service.mail.JavaMailContainer.updateUrlName(URLName, 
> Properties)
> 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.
> - JavaMailContainer.java:269, SIC_INNER_SHOULD_BE_STATIC
> SIC: Should 
> org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener 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.
> - JavaMailContainer.java:274, SF_SWITCH_NO_DEFAULT
> SF: Switch statement found in 
> org.apache.ofbiz.service.mail.JavaMailContainer$LoggingStoreListener.notification(StoreEvent)
>  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.
> - MimeMessageWrapper.java:50, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.mail.MimeMessageWrapper 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.
> - MimeMessageWrapper.java:68, IS2_INCONSISTENT_SYNC
> IS: Inconsistent synchronization of 
> org.apache.ofbiz.service.mail.MimeMessageWrapper.session; locked 75% 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 loc

[jira] [Updated] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.calendar

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9691:
-
Attachment: OFBIZ-No_org.apache.ofbiz.service.calendar_bugfixes.patch

- Diamond Operator fixed

class ExpressionUiHelper:
- made {{Occurrence}} private

class RecurrenceInfo:
- has a circular dependency with itself, this is intended
- Line 117: returned a clone instead of the actual object
- Line 347: won’t happen in the OfBiz api

class RecurrenceRule:
- Line 199: added a default Locale to {{toUpperCase}}
- Line 389: added a default case to the switch
- Line 323 and following: it seems to be intended, that the cases fall through, 
but this is not really good programming
- Line 712: added parenthesis, otherwise, in case of {{day}} being null, there 
will be a guaranteed {{NullPointerException}}

class TemporalExpression:
- Line 55: implemented {{equals}} and {{hashCode}} method to refine 
{{compareTo}} since, this should just return zero, if equals returns true
- Line 110: removed declaration, because it is never used and is declarated 
before the next use anyways

class TemporalExpressionWorker:
- Line 166: returned a copy of {{ExpressionTypeList}} instead of the original

class TemporalExpressions:
- Line 37: it’s not necessary for the class to implement serialVerisonUID
- Line 64: it’s not necessary for the class to implement serialVerisonUID
- Line 81: implemented {{hashCode}}
- Line 90: added a null-check to {{equals}}
- Line 191: implemented {{hashCode}}
- Line 201: added a null-check to {{equals}}
- Line 294: it’s not necessary for the class to implement serialVerisonUID
- Line 321: implemented {{hashCode}}
- Line 331: added a null-check to {{equals}}
- Line 410: it’s not necessary for the class to implement serialVerisonUID
- Line 440: implemented {{hashCode}}
- Line 449: added a null-check to {{equals}}
- Line 549: it’s not necessary for the class to implement serialVerisonUID
- Line 574: implemented {{hashCode}}
- Line 584: added a null-check to {{equals}}
- Line 660: it’s not necessary for the class to implement serialVerisonUID
- Line 670: stored a clone in start instead of the actual object
- Line 688: implemented {{hashCode}}
- Line 698: added a null-check to {{equals}}
- Line 817: it’s not necessary for the class to implement serialVerisonUID
- Line 845: implemented {{hashCode}}
- Line 855: added a null-check to {{equals}}
- Line 966: it’s not necessary for the class to implement serialVerisonUID
- Line 1006: implemented {{hashCode}}
- Line 1014: added a null-check to {{equals}}
- Line 1098: it’s not necessary for the class to implement serialVerisonUID
- Line 1126: implemented {{hashCode}}
- Line 1135: added a null-check to {{equals}}
- Line 1245: it’s not necessary for the class to implement serialVerisonUID
- Line 1275: implemented {{hashCode}}
- Line 1284: added a null-check to {{equals}}
- Line 1375: it’s not necessary for the class to implement serialVerisonUID
- Line 1402: it’s not necessary for the class to implement serialVerisonUID
- Line 1435: implemented {{hashCode}}
- Line 1446: added a null-check to {{equals}}
- Line 1523: it’s not necessary for the class to implement serialVerisonUID
- Line 1556: implemented {{hashCode}}
- Line 1565: added a null-check to {{equals}}


> [FB] Package org.apache.ofbiz.service.calendar
> --
>
> Key: OFBIZ-9691
> URL: https://issues.apache.org/jira/browse/OFBIZ-9691
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-No_org.apache.ofbiz.service.calendar_bugfixes.patch
>
>
> - ExpressionUiHelper.java:42, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.calendar.ExpressionUiHelper.Occurrence 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.
> - RecurrenceInfo.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 
> defines non-transient non-serializable instance field info
> 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.
> - RecurrenceInfo.java:117, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.calendar.RecurrenceInfo.getStartDate() may 
> expose internal representation by returning RecurrenceInfo.startDate
> Returning a reference to a mutable object value stored in one of the object's 
> fields exposes the internal representation of the o

[jira] [Updated] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.calendar

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9691:
-
Attachment: (was: 
OFBIZ-No_org.apache.ofbiz.service.calendar_bugfixes.patch)

> [FB] Package org.apache.ofbiz.service.calendar
> --
>
> Key: OFBIZ-9691
> URL: https://issues.apache.org/jira/browse/OFBIZ-9691
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9691_org.apache.ofbiz.service.calendar_bugfixes.patch
>
>
> - ExpressionUiHelper.java:42, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.calendar.ExpressionUiHelper.Occurrence 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.
> - RecurrenceInfo.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 
> defines non-transient non-serializable instance field info
> 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.
> - RecurrenceInfo.java:117, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.calendar.RecurrenceInfo.getStartDate() may 
> expose internal representation by returning RecurrenceInfo.startDate
> 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.
> - RecurrenceInfo.java:349, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 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.
> - RecurrenceRule.java:197, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.getFrequencyName()
> 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.
> - RecurrenceRule.java:321, SF_SWITCH_NO_DEFAULT
> SF: Switch statement found in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, 
> long) 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.
> - RecurrenceRule.java:335, SF_SWITCH_FALLTHROUGH
> SF: Switch statement found in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, 
> long) where one case falls through to the next case
> This method contains a switch statement where one case branch will fall 
> through to the next case. Usually you need to end this case with a break or 
> return.
> - RecurrenceRule.java:724, NP_NULL_ON_SOME_PATH
> NP: Possible null pointer dereference of day in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.getCalendarDay(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 FindBu

[jira] [Updated] (OFBIZ-9691) [FB] Package org.apache.ofbiz.service.calendar

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9691:
-
Attachment: OFBIZ-9691_org.apache.ofbiz.service.calendar_bugfixes.patch

> [FB] Package org.apache.ofbiz.service.calendar
> --
>
> Key: OFBIZ-9691
> URL: https://issues.apache.org/jira/browse/OFBIZ-9691
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9691_org.apache.ofbiz.service.calendar_bugfixes.patch
>
>
> - ExpressionUiHelper.java:42, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.calendar.ExpressionUiHelper.Occurrence 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.
> - RecurrenceInfo.java:-1, SE_BAD_FIELD
> Se: Class org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 
> defines non-transient non-serializable instance field info
> 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.
> - RecurrenceInfo.java:117, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.calendar.RecurrenceInfo.getStartDate() may 
> expose internal representation by returning RecurrenceInfo.startDate
> 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.
> - RecurrenceInfo.java:349, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.calendar.RecurrenceInfo$RecurrenceWrapper 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.
> - RecurrenceRule.java:197, DM_CONVERT_CASE
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.getFrequencyName()
> 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.
> - RecurrenceRule.java:321, SF_SWITCH_NO_DEFAULT
> SF: Switch statement found in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, 
> long) 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.
> - RecurrenceRule.java:335, SF_SWITCH_FALLTHROUGH
> SF: Switch statement found in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.validCurrent(long, long, 
> long) where one case falls through to the next case
> This method contains a switch statement where one case branch will fall 
> through to the next case. Usually you need to end this case with a break or 
> return.
> - RecurrenceRule.java:724, NP_NULL_ON_SOME_PATH
> NP: Possible null pointer dereference of day in 
> org.apache.ofbiz.service.calendar.RecurrenceRule.getCalendarDay(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.
> - Te

[jira] [Created] (OFBIZ-9693) [FB] Package org.apache.ofbiz.service.semaphore

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9693:


 Summary: [FB] Package org.apache.ofbiz.service.semaphore
 Key: OFBIZ-9693
 URL: https://issues.apache.org/jira/browse/OFBIZ-9693
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Reporter: Dennis Balkir
Priority: Minor


- ServiceSemaphore.java:77, IS2_INCONSISTENT_SYNC
IS: Inconsistent synchronization of 
org.apache.ofbiz.service.semaphore.ServiceSemaphore.lock; locked 40% 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.

- ServiceSemaphore.java:176, 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.



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


[jira] [Updated] (OFBIZ-9693) [FB] Package org.apache.ofbiz.service.semaphore

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9693:
-
Attachment: OFBIZ-9693_org.apache.ofbiz.service.semaphore_bugfixes.patch

class ServiceSemaphore:
- removed unnecessary else
- Line 73: made the method {{release}} synchronized
- Line 177: removed {{&& beganTx}} because it cannot be null at this point


> [FB] Package org.apache.ofbiz.service.semaphore
> ---
>
> Key: OFBIZ-9693
> URL: https://issues.apache.org/jira/browse/OFBIZ-9693
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9693_org.apache.ofbiz.service.semaphore_bugfixes.patch
>
>
> - ServiceSemaphore.java:77, IS2_INCONSISTENT_SYNC
> IS: Inconsistent synchronization of 
> org.apache.ofbiz.service.semaphore.ServiceSemaphore.lock; locked 40% 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.
> - ServiceSemaphore.java:176, 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.



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


[jira] [Created] (OFBIZ-9694) [FB] Package org.apache.ofbiz.service.test

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9694:


 Summary: [FB] Package org.apache.ofbiz.service.test
 Key: OFBIZ-9694
 URL: https://issues.apache.org/jira/browse/OFBIZ-9694
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- ServiceEngineTestServices.java:316, REC_CATCH_EXCEPTION
REC: Exception is caught when Exception is not thrown in 
org.apache.ofbiz.service.test.ServiceEngineTestServices.testServiceOwnTxSubServiceAfterSetRollbackOnlyInParent(DispatchContext,
 Map)

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 ...
  }

- ServiceSOAPTests.java:41, DM_FP_NUMBER_CTOR
Bx: org.apache.ofbiz.service.test.ServiceSOAPTests.testSOAPSimpleService() 
invokes inefficient new Double(String) constructor; use Double.valueOf(String) 
instead

Using new Double(double) is guaranteed to always result in a new object whereas 
Double.valueOf(double) 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.

Unless the class must be compatible with JVMs predating Java 1.5, use either 
autoboxing or the valueOf() method when creating instances of Double and Float.

- XmlRpcTests.java:41, MS_PKGPROTECT
MS: org.apache.ofbiz.service.test.XmlRpcTests.url 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.

- XmlRpcTests.java:47, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD
ST: Write to static field org.apache.ofbiz.service.test.XmlRpcTests.url from 
instance method new org.apache.ofbiz.service.test.XmlRpcTests(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.



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


[jira] [Updated] (OFBIZ-9694) [FB] Package org.apache.ofbiz.service.test

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9694:
-
Attachment: OFBIZ-9694_org.apache.ofbiz.service.test_bugfixes.patch

- Diamond Operators fixed

class ServiceSOAPTests:
- Line 41: changed the new operator to {{Double.valueOf()}} because is faster

class XmlRpcTests:
- Line 41: changed the parameter to private
- Line 47: it is possible to refactor this into a new method, but it doesn’t 
really make sense

> [FB] Package org.apache.ofbiz.service.test
> --
>
> Key: OFBIZ-9694
> URL: https://issues.apache.org/jira/browse/OFBIZ-9694
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9694_org.apache.ofbiz.service.test_bugfixes.patch
>
>
> - ServiceEngineTestServices.java:316, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.service.test.ServiceEngineTestServices.testServiceOwnTxSubServiceAfterSetRollbackOnlyInParent(DispatchContext,
>  Map)
> 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 ...
>   }
> - ServiceSOAPTests.java:41, DM_FP_NUMBER_CTOR
> Bx: org.apache.ofbiz.service.test.ServiceSOAPTests.testSOAPSimpleService() 
> invokes inefficient new Double(String) constructor; use 
> Double.valueOf(String) instead
> Using new Double(double) is guaranteed to always result in a new object 
> whereas Double.valueOf(double) 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.
> Unless the class must be compatible with JVMs predating Java 1.5, use either 
> autoboxing or the valueOf() method when creating instances of Double and 
> Float.
> - XmlRpcTests.java:41, MS_PKGPROTECT
> MS: org.apache.ofbiz.service.test.XmlRpcTests.url 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.
> - XmlRpcTests.java:47, ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD
> ST: Write to static field org.apache.ofbiz.service.test.XmlRpcTests.url from 
> instance method new org.apache.ofbiz.service.test.XmlRpcTests(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.



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


[jira] [Created] (OFBIZ-9695) [FB] Package org.apache.ofbiz.service.xmlrpc

2017-09-08 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9695:


 Summary: [FB] Package org.apache.ofbiz.service.xmlrpc
 Key: OFBIZ-9695
 URL: https://issues.apache.org/jira/browse/OFBIZ-9695
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AliasSupportedTransportFactory.java:69, SIC_INNER_SHOULD_BE_STATIC
SIC: Should 
org.apache.ofbiz.service.xmlrpc.AliasSupportedTransportFactory$AliasSupportedTransport
 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.

- XmlRpcClient.java:35, NM_SAME_SIMPLE_NAME_AS_SUPERCLASS
Nm: The class name org.apache.ofbiz.service.xmlrpc.XmlRpcClient shadows the 
simple name of the superclass org.apache.xmlrpc.client.XmlRpcClient

This class has a simple name that is identical to that of its superclass, 
except that its superclass is in a different package (e.g., alpha.Foo extends 
beta.Foo). This can be exceptionally confusing, create lots of situations in 
which you have to look at import statements to resolve references and creates 
many opportunities to accidentally define methods that do not override methods 
in their superclasses.



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


[jira] [Updated] (OFBIZ-9695) [FB] Package org.apache.ofbiz.widget.cache

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9695:
-
Summary: [FB] Package org.apache.ofbiz.widget.cache  (was: [FB] Package 
org.apache.ofbiz.service.xmlrpc)

> [FB] Package org.apache.ofbiz.widget.cache
> --
>
> Key: OFBIZ-9695
> URL: https://issues.apache.org/jira/browse/OFBIZ-9695
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> - AliasSupportedTransportFactory.java:69, SIC_INNER_SHOULD_BE_STATIC
> SIC: Should 
> org.apache.ofbiz.service.xmlrpc.AliasSupportedTransportFactory$AliasSupportedTransport
>  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.
> - XmlRpcClient.java:35, NM_SAME_SIMPLE_NAME_AS_SUPERCLASS
> Nm: The class name org.apache.ofbiz.service.xmlrpc.XmlRpcClient shadows the 
> simple name of the superclass org.apache.xmlrpc.client.XmlRpcClient
> This class has a simple name that is identical to that of its superclass, 
> except that its superclass is in a different package (e.g., alpha.Foo extends 
> beta.Foo). This can be exceptionally confusing, create lots of situations in 
> which you have to look at import statements to resolve references and creates 
> many opportunities to accidentally define methods that do not override 
> methods in their superclasses.



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


[jira] [Updated] (OFBIZ-9695) [FB] Package org.apache.ofbiz.widget.cache

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9695:
-
Description: 
- WidgetContextCacheKey.java:140, WMI_WRONG_MAP_ITERATOR
WMI: org.apache.ofbiz.widget.cache.WidgetContextCacheKey.toString() 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.

  was:
- AliasSupportedTransportFactory.java:69, SIC_INNER_SHOULD_BE_STATIC
SIC: Should 
org.apache.ofbiz.service.xmlrpc.AliasSupportedTransportFactory$AliasSupportedTransport
 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.

- XmlRpcClient.java:35, NM_SAME_SIMPLE_NAME_AS_SUPERCLASS
Nm: The class name org.apache.ofbiz.service.xmlrpc.XmlRpcClient shadows the 
simple name of the superclass org.apache.xmlrpc.client.XmlRpcClient

This class has a simple name that is identical to that of its superclass, 
except that its superclass is in a different package (e.g., alpha.Foo extends 
beta.Foo). This can be exceptionally confusing, create lots of situations in 
which you have to look at import statements to resolve references and creates 
many opportunities to accidentally define methods that do not override methods 
in their superclasses.


> [FB] Package org.apache.ofbiz.widget.cache
> --
>
> Key: OFBIZ-9695
> URL: https://issues.apache.org/jira/browse/OFBIZ-9695
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> - WidgetContextCacheKey.java:140, WMI_WRONG_MAP_ITERATOR
> WMI: org.apache.ofbiz.widget.cache.WidgetContextCacheKey.toString() 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.



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


[jira] [Updated] (OFBIZ-9695) [FB] Package org.apache.ofbiz.widget.cache

2017-09-08 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9695:
-
Attachment: OFBIZ-9695_org.apache.ofbiz.widget.cache_bugfixes.patch

- Diamond Operators fixed

class WidgetContextCacheKey:
- Line 137 and following: optimized the performance with the use of {{entrySet}}

> [FB] Package org.apache.ofbiz.widget.cache
> --
>
> Key: OFBIZ-9695
> URL: https://issues.apache.org/jira/browse/OFBIZ-9695
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9695_org.apache.ofbiz.widget.cache_bugfixes.patch
>
>
> - WidgetContextCacheKey.java:140, WMI_WRONG_MAP_ITERATOR
> WMI: org.apache.ofbiz.widget.cache.WidgetContextCacheKey.toString() 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.



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


[jira] [Created] (OFBIZ-9709) [FB] Package org.apache.ofbiz.service.job

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9709:


 Summary: [FB] Package org.apache.ofbiz.service.job
 Key: OFBIZ-9709
 URL: https://issues.apache.org/jira/browse/OFBIZ-9709
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AbstractJob.java:116, EI_EXPOSE_REP
EI: org.apache.ofbiz.service.job.AbstractJob.getStartTime() may expose internal 
representation by returning AbstractJob.startTime

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.

- GenericServiceJob.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
Se: The field org.apache.ofbiz.service.job.GenericServiceJob.dctx 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.

- GenericServiceJob.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
Se: The field org.apache.ofbiz.service.job.GenericServiceJob.requester 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.

- GenericServiceJob.java:37, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.job.GenericServiceJob 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.

- GenericServiceJob.java:37, SE_NO_SUITABLE_CONSTRUCTOR
Se: org.apache.ofbiz.service.job.GenericServiceJob is Serializable but its 
superclass doesn't define an accessible void constructor

This class implements the Serializable interface and its superclass does not. 
When such an object is deserialized, the fields of the superclass need to be 
initialized by invoking the void constructor of the superclass. Since the 
superclass does not have one, serialization and deserialization will fail at 
runtime.

- JobManager.java:564, DM_NUMBER_CTOR
Bx: org.apache.ofbiz.service.job.JobManager.schedule(String, String, String, 
String, long, int, int, int, long, int) invokes inefficient new Long(long) 
constructor; use Long.valueOf(long) 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.

- PersistedServiceJob.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
Se: The field org.apache.ofbiz.service.job.PersistedServiceJob.delegator 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.

- PersistedServiceJob.java:62, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.job.PersistedServiceJob 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 wil

[jira] [Updated] (OFBIZ-9709) [FB] Package org.apache.ofbiz.service.job

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9709:
-
Attachment: OFBIZ-9709_org.apache.ofbiz.service.job_bugfixes.patch

- Diamond Operators fixed

class AbstractJob:
- Line 116: returned a clone instead of the actual object

class GenericServiceJob:
- Line 38: it’s not necessary for the class to implement serialVerisonUID
- Line 40, 41: won’t happen in Ofbiz-api

class JobManager:
- Line 495: changed the catches to a multicatch
- Line 564: changed the new operator to the more efficient {{Long.valueOf()}}

class PersistedServiceJob:
- Line 62: this was intended this way
- Line 64: won’t happen in Ofbiz-api
- Line 205, 207: changed the new operator to the more efficient 
{{Long.valueOf()}}
- removed unnecessary else

class PurgeJob:
- Line 34: it’s not necessary for the class to implement serialVerisonUID

> [FB] Package org.apache.ofbiz.service.job
> -
>
> Key: OFBIZ-9709
> URL: https://issues.apache.org/jira/browse/OFBIZ-9709
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9709_org.apache.ofbiz.service.job_bugfixes.patch
>
>
> - AbstractJob.java:116, EI_EXPOSE_REP
> EI: org.apache.ofbiz.service.job.AbstractJob.getStartTime() may expose 
> internal representation by returning AbstractJob.startTime
> 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.
> - GenericServiceJob.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
> Se: The field org.apache.ofbiz.service.job.GenericServiceJob.dctx 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.
> - GenericServiceJob.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED
> Se: The field org.apache.ofbiz.service.job.GenericServiceJob.requester 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.
> - GenericServiceJob.java:37, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.job.GenericServiceJob 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.
> - GenericServiceJob.java:37, SE_NO_SUITABLE_CONSTRUCTOR
> Se: org.apache.ofbiz.service.job.GenericServiceJob is Serializable but its 
> superclass doesn't define an accessible void constructor
> This class implements the Serializable interface and its superclass does not. 
> When such an object is deserialized, the fields of the superclass need to be 
> initialized by invoking the void constructor of the superclass. Since the 
> superclass does not have one, serialization and deserialization will fail at 
> runtime.
> - JobManager.java:564, DM_NUMBER_CTOR
> Bx: org.apache.ofbiz.service.job.JobManager.schedule(String, String, String, 
> String, long, int, int, int, long, int) invokes inefficient new Long(long) 
> constructor; use Long.valueOf(long) 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 

[jira] [Created] (OFBIZ-9710) [FB] Package org.apache.ofbiz.widget.model

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9710:


 Summary: [FB] Package org.apache.ofbiz.widget.model
 Key: OFBIZ-9710
 URL: https://issues.apache.org/jira/browse/OFBIZ-9710
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AbstractModelAction.java:191, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$EntityAnd 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.

- AbstractModelAction.java:225, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$EntityCondition 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.

- AbstractModelAction.java:259, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$EntityOne 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.

- AbstractModelAction.java:298, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$GetRelated 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.

- AbstractModelAction.java:381, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$GetRelatedOne 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.

- AbstractModelAction.java:445, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.widget.model.AbstractModelAction$PropertyMap 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 St

[jira] [Updated] (OFBIZ-9710) [FB] Package org.apache.ofbiz.widget.model

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9710:
-
Attachment: OFBIZ-9710_org.apache.ofbiz.widget.model_bugfixes.patch

- Diamond Operators fixed

class AbstractModelAction:
- Line 83: this is intended this way
- Line 191: this was intended this way
- Line 225: this was intended this way
- Line 259: this was intended this way
- Line 298: it’s not necessary for the class to implement serialVerisonUID
- Line 381: it’s not necessary for the class to implement serialVerisonUID
- Line 445: it’s not necessary for the class to implement serialVerisonUID
- Line 523: it’s not necessary for the class to implement serialVerisonUID
- Line 607: it’s not necessary for the class to implement serialVerisonUID
- Line 657: this was intended this way
- Line 746: it’s not necessary for the class to implement serialVerisonUID

class AbstractModelCondition:
- removed unnecessary else
- Line 118: it’s not necessary for the class to implement serialVerisonUID
- Line 225: it’s not necessary for the class to implement serialVerisonUID
- Line 300: it’s not necessary for the class to implement serialVerisonUID
- Line 376: this was intended this way
- Line 406: this cannot happen in the Ofbiz-api
- Line 409: it’s not necessary for the class to implement serialVerisonUID
- Line 438: it’s not necessary for the class to implement serialVerisonUID
- Line 490: it’s not necessary for the class to implement serialVerisonUID
- Line 550: it’s not necessary for the class to implement serialVerisonUID
- Line 598: removed if phrase, because {{permService}} cannot be null at this 
point
- Line 652: it’s not necessary for the class to implement serialVerisonUID
- Line 732: it’s not necessary for the class to implement serialVerisonUID
- Line 761: it’s not necessary for the class to implement serialVerisonUID
- Line 795: it’s not necessary for the class to implement serialVerisonUID

class CommonWidgetModels:
- Line 104: removed unnecessary if phrase, because {{fieldsIter}} cannot not be 
null at this point
- Line 127: removed {{boolean includeNonPk;}} because it is never used
- Line 127: removed {{boolean includePk;}} because it is never used

class HtmlWidget:
- Line 60: this was intended this way
- Line 63: made the parameter final
- Line 205: this was intended this way
- Line 230: this was intended this way
- Line 285: it’s not necessary for the class to implement serialVerisonUID

class IterateSectionWidget:
- Line 58: it’s not necessary for the class to implement serialVerisonUID
- Line 59: made the parameter final
- Line 60: made the parameter final
- Line 115: changed the name of the parameter from {{viewSize}} to 
{{locViewSize}}, as in local view size, to reduce confusion with the class 
parameter {{viewSize}} and to solve this findbugs error
- Line 155: added a catch for RuntimeException

class ModelForm:
- removed unnecessary else
- Line 86, 87, 88, 89, 90, 91, 92, 93, 94, 96, 97, 98: made the parameter final 
to prevent violation from other classes
- Line 210: changed the method use from {{valueOf()}} to {{parseInt()}} because 
it is more efficient
- Line 607: removed the local variable {{rowCountExdr}} because it is never used
- Line 800: removed the unnecessary null-check, because {{modelEntity}} cannot 
be null at this point, since {{getModelEntity}} cannot produce null as a result
- Line 1448: changed the method use from {{valueOf()}} to {{parseInt()}} 
because it is more efficient
- Line 1472: changed the method use from {{valueOf()}} to {{parseInt()}} 
because it is more efficient
- Line 1536: this was intended this way
- Line 1605: made the field from line 129 protected, because this will cause a 
more efficient use in line 1605
- Line 1623: made the field from line 133 protected, because this will cause a 
more efficient use in line 1623, this solves the problem in line 1624 too
- Line 1628: made the field from line 143 protected, because this will cause a 
more efficient use in line 1628
- Line 1658: added a {{Debug.logInfo}} to at least not completely ignore the 
thrown exception

class ModelFormAction:
- Line 78: it’s not necessary for the class to implement serialVerisonUID
- Line 137: this was intended this way
- Line 141: removed {{= FlexibleStringExpander.getInstance(„“)}} because the 
variable {{resultMapListNameExdr}} gets declared multi times after this before 
it is read

class ModelFormField:
- removed unnecessary else
- Line 975: this is intended this way; when this method is used, the returned 
value is stored in a {{Boolean}} which cannot result in a 
{{NullPointerException}}
- Line 1312: made the field from line 115 protected, because this will cause a 
more efficient use in line 1312
- Line 1320: made the field from line 120 protected, because this will cause a 
more efficient use in line 1320
- Line 1773: made the field from line 129 protected, because this will ca

[jira] [Created] (OFBIZ-9711) [FB] Package org.apache.ofbiz.entity.cache

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9711:


 Summary: [FB] Package org.apache.ofbiz.entity.cache
 Key: OFBIZ-9711
 URL: https://issues.apache.org/jira/browse/OFBIZ-9711
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- AbstractEntityConditionCache.java:68, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of model, which is known to be non-null in 
org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.remove(GenericEntity)

This method contains a redundant check of a known non-null value against the 
constant null.

- AbstractEntityConditionCache.java:169, UCF_USELESS_CONTROL_FLOW
UCF: Useless control flow in 
org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.storeHook(boolean, 
GenericEntity, GenericEntity)

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
}

- AbstractEntityConditionCache.java:183, UCF_USELESS_CONTROL_FLOW
UCF: Useless control flow in 
org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.storeHook(String, 
boolean, List, List)

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
}



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


[jira] [Updated] (OFBIZ-9711) [FB] Package org.apache.ofbiz.entity.cache

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9711:
-
Attachment: OFBIZ-9711_org.apache.ofbiz.entity.cache_bugfixes.patch

- Diamond Operators fixed

- Line 68: removed the unnecessary if, {{model}} cannot be null at this point
- Line 167: not used if phase removed
- Line 181: not used if phrase removed

> [FB] Package org.apache.ofbiz.entity.cache
> --
>
> Key: OFBIZ-9711
> URL: https://issues.apache.org/jira/browse/OFBIZ-9711
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9711_org.apache.ofbiz.entity.cache_bugfixes.patch
>
>
> - AbstractEntityConditionCache.java:68, 
> RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of model, which is known to be non-null in 
> org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.remove(GenericEntity)
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - AbstractEntityConditionCache.java:169, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.storeHook(boolean, 
> GenericEntity, GenericEntity)
> 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
> }
> - AbstractEntityConditionCache.java:183, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.entity.cache.AbstractEntityConditionCache.storeHook(String, 
> boolean, List, List)
> 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
> }



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


[jira] [Created] (OFBIZ-9712) [FB] Package org.apache.ofbiz.service.rmi.socket.ssl

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9712:


 Summary: [FB] Package org.apache.ofbiz.service.rmi.socket.ssl
 Key: OFBIZ-9712
 URL: https://issues.apache.org/jira/browse/OFBIZ-9712
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- SSLClientSocketFactory.java:37, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLClientSocketFactory 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.

- SSLServerSocketFactory.java:43, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory 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.

- SSLServerSocketFactory.java:76, OS_OPEN_STREAM
OS: 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
 may fail to close stream

The method creates an IO stream object, does not assign it to any fields, pass 
it to other methods that might close it, or return it, and does not appear to 
close the stream on all paths out of the method.  This may result in a file 
descriptor leak.  It is generally a good idea to use a finally block to ensure 
that streams are closed.

- SSLServerSocketFactory.java:76, OBL_UNSATISFIED_OBLIGATION
OBL: 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
 may fail to clean up java.io.InputStream

This method may fail to clean up (close, dispose of) a stream, database object, 
or other resource requiring an explicit cleanup operation.

In general, if a method opens a stream or other resource, the method should use 
a try/finally block to ensure that the stream or resource is cleaned up before 
the method returns.

This bug pattern is essentially the same as the OS_OPEN_STREAM and 
ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and 
hopefully better) static analysis technique. We are interested is getting 
feedback about the usefulness of this bug pattern. To send feedback, either:

send email to findb...@cs.umd.edu
file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
In particular, the false-positive suppression heuristics for this bug pattern 
have not been extensively tuned, so reports about false positives are helpful 
to us.

See Weimer and Necula, Finding and Preventing Run-Time Error Handling Mistakes, 
for a description of the analysis technique.

- SSLServerSocketFactory.java:111, BC_UNCONFIRMED_CAST_OF_RETURN_VALUE
BC: Unchecked/unconfirmed cast from java.net.ServerSocket to 
javax.net.ssl.SSLServerSocket of return value in 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)

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.



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


[jira] [Updated] (OFBIZ-9712) [FB] Package org.apache.ofbiz.entity.finder

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9712:
-
Summary: [FB] Package org.apache.ofbiz.entity.finder  (was: [FB] Package 
org.apache.ofbiz.service.rmi.socket.ssl)

> [FB] Package org.apache.ofbiz.entity.finder
> ---
>
> Key: OFBIZ-9712
> URL: https://issues.apache.org/jira/browse/OFBIZ-9712
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
>
> - SSLClientSocketFactory.java:37, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLClientSocketFactory 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.
> - SSLServerSocketFactory.java:43, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory 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.
> - SSLServerSocketFactory.java:76, OS_OPEN_STREAM
> OS: 
> org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
>  may fail to close stream
> The method creates an IO stream object, does not assign it to any fields, 
> pass it to other methods that might close it, or return it, and does not 
> appear to close the stream on all paths out of the method.  This may result 
> in a file descriptor leak.  It is generally a good idea to use a finally 
> block to ensure that streams are closed.
> - SSLServerSocketFactory.java:76, OBL_UNSATISFIED_OBLIGATION
> OBL: 
> org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
>  may fail to clean up java.io.InputStream
> This method may fail to clean up (close, dispose of) a stream, database 
> object, or other resource requiring an explicit cleanup operation.
> In general, if a method opens a stream or other resource, the method should 
> use a try/finally block to ensure that the stream or resource is cleaned up 
> before the method returns.
> This bug pattern is essentially the same as the OS_OPEN_STREAM and 
> ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and 
> hopefully better) static analysis technique. We are interested is getting 
> feedback about the usefulness of this bug pattern. To send feedback, either:
> send email to findb...@cs.umd.edu
> file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
> In particular, the false-positive suppression heuristics for this bug pattern 
> have not been extensively tuned, so reports about false positives are helpful 
> to us.
> See Weimer and Necula, Finding and Preventing Run-Time Error Handling 
> Mistakes, for a description of the analysis technique.
> - SSLServerSocketFactory.java:111, BC_UNCONFIRMED_CAST_OF_RETURN_VALUE
> BC: Unchecked/unconfirmed cast from java.net.ServerSocket to 
> javax.net.ssl.SSLServerSocket of return value in 
> org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
> 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.



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


[jira] [Updated] (OFBIZ-9712) [FB] Package org.apache.ofbiz.entity.finder

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9712:
-
Description: 
- ByAndFinder.java:37, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.ByAndFinder 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.

- ByConditionFinder.java:39, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.ByConditionFinder 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.

- EntityFinderUtil.java:166, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$ConditionExpr 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.

- EntityFinderUtil.java:265, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$ConditionList 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.

- EntityFinderUtil.java:317, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$ConditionObject 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.

- EntityFinderUtil.java:340, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$LimitRange 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 

[jira] [Updated] (OFBIZ-9712) [FB] Package org.apache.ofbiz.entity.finder

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9712:
-
Attachment: OFBIZ-9712_org.apache.ofbiz.entity.finder_bugfixes.patch

- Diamond Operators fixed

class ByAndFinder:
- Line 37: it’s not necessary for the class to implement serialVerisonUID

class ByConditionFinder:
- Line 39: it’s not necessary for the class to implement serialVerisonUID

class EntityFinderUtil:
- removed unnecessary else
- Line 166: it’s not necessary for the class to implement serialVerisonUID
- Line 212: changed the String {{„|“}} to {{‚|‘}} because it is more efficient
- Line 214: changed the String {{„,“}} to {{‚,‘}} because it is more efficient
- Line 262: it’s not necessary for the class to implement serialVerisonUID
- Line 314: it’s not necessary for the class to implement serialVerisonUID
- Line 337: it’s not necessary for the class to implement serialVerisonUID
- Line 399: it’s not necessary for the class to implement serialVerisonUID
- Line 459: it’s not necessary for the class to implement serialVerisonUID
- Line 473: it’s not necessary for the class to implement serialVerisonUID

class PrimaryKeyFinder:
- Line 47: it’s not necessary for the class to implement serialVerisonUID

> [FB] Package org.apache.ofbiz.entity.finder
> ---
>
> Key: OFBIZ-9712
> URL: https://issues.apache.org/jira/browse/OFBIZ-9712
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: OFBIZ-9712_org.apache.ofbiz.entity.finder_bugfixes.patch
>
>
> - ByAndFinder.java:37, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.entity.finder.ByAndFinder 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.
> - ByConditionFinder.java:39, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.entity.finder.ByConditionFinder 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.
> - EntityFinderUtil.java:166, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$ConditionExpr 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.
> - EntityFinderUtil.java:265, SE_NO_SERIALVERSIONID
> SnVI: org.apache.ofbiz.entity.finder.EntityFinderUtil$ConditionList 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 c

[jira] [Created] (OFBIZ-9713) [FB] Package org.apache.ofbiz.entity.condition

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9713:


 Summary: [FB] Package org.apache.ofbiz.entity.condition
 Key: OFBIZ-9713
 URL: https://issues.apache.org/jira/browse/OFBIZ-9713
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- EntityClause.java:142, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of 
org.apache.ofbiz.entity.condition.EntityOperator.getCode(), which is known to 
be non-null in org.apache.ofbiz.entity.condition.EntityClause.toString()

This method contains a redundant check of a known non-null value against the 
constant null.

- EntityClause.java:144, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
RCN: Redundant nullcheck of Object.toString(), which is known to be non-null in 
org.apache.ofbiz.entity.condition.EntityClause.toString()

This method contains a redundant check of a known non-null value against the 
constant null.

- EntityConditionBase.java:104, EQ_UNUSUAL
Eq: org.apache.ofbiz.entity.condition.EntityConditionBase.equals(Object) is 
unusual

This class doesn't do any of the patterns we recognize for checking that the 
type of the argument is compatible with the type of the this object. There 
might not be anything wrong with this code, but it is worth reviewing.

- EntityConditionBuilder.java:43, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.condition.EntityConditionBuilder$ConditionHolder 
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.

- EntityConditionBuilder.java:90, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.entity.condition.EntityConditionBuilder.createNode(Object)

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.

- EntityConditionBuilder.java:106, DM_CONVERT_CASE
Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in 
org.apache.ofbiz.entity.condition.EntityConditionBuilder.createNode(Object, 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.

- EntityConditionFunction.java:40, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.condition.EntityConditionFunction$NOT 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.

- EntityConditionFunction.java:91, RC_REF_COMPARISON
RC: Suspicious comparison of Integer references in 
org.apache.ofbiz.entity.condition.EntityConditionFunction.equals(Object)

This method compares two reference values using the == or != operator, where 
the correct way to compare instances of this type is generally with the 
equals() method. It is possible to create distinct instances that are equal but 
do not compare as == since they are different objects. Examples of classes 
which should generally not be compared by reference are java.lang.Integer, 
java.lang.Float, etc.

- EntityConditionList.java:31, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.entity.condition.EntityConditionList 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 ch

[jira] [Updated] (OFBIZ-9713) [FB] Package org.apache.ofbiz.entity.condition

2017-09-13 Thread Dennis Balkir (JIRA)

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

Dennis Balkir updated OFBIZ-9713:
-
Attachment: OFBIZ-9713_org.apache.ofbiz.entity.condition_bugfixes.patch

- Diamond Operators fixed

class EntityClause:
- Line 142: removed {{interFieldOperation.getCode() == null ? "null" :}} since 
the method {{getCode()}} cannot produce null as a result, it can return the 
String „null“ though, which this method would have set, if {{getCode()}} would 
have returned null
- Line 144: removed {{getValue().toString() == null ? "null" :}} because 
{{toString()}} cannot return null

class EntityConditionBase:
- Line 104 & 109: very unusual implementation of {{equals}} and {{hashCode}}; 
maybe this is because, these methods should never have been used, and when they 
are, something must be wrong?

class EntityConditionBuilder:
- removed unnecessary else
- Line 43: it’s not necessary for the class to implement serialVerisonUID
- Line 91: added a default Locale to {{toLowerCase}}
- Line 107: added a default Locale to {{toLowerCase}}

class EntityConditionFunction:
- removed unnecessary else
- Line 40: it’s not necessary for the class to implement serialVerisonUID
- Line 90: changed the {{==}} operator to {{equals()}}

class EntityConditionList:
- Line 31: it’s not necessary for the class to implement serialVerisonUID

class EntityConditionParam:
- Line 34: it’s not necessary for the class to implement serialVerisonUID

class EntityConditionSubSelect:
- Line 39: it’s not necessary for the class to implement serialVerisonUID

class EntityConditionValue:
- Line 43: it’s not necessary for the class to implement serialVerisonUID

class EntityDateFilterCondition:
- Line 39, 40: made the parameters private
- Line 42: it’s not necessary for the class to implement serialVerisonUID

class EntityExpr:
- Line 44: it’s not necessary for the class to implement serialVerisonUID
- Line 144: added a default Locale to {{toUpperCase}}
- Line 226 & 252: {{valueType}} treated like {{type}} before, now it cannot be 
null at these points

class EntityFieldMap:
- Line 37: made the parameter private
- Line 40: it’s not necessary for the class to implement serialVerisonUID

class EntityFieldValue:
- Line 42: it’s not necessary for the class to implement serialVerisonUID
- removed unnecessary else

class EntityFunction:
- removed unnecessary else
- Line 68: made the parameter final
- Line 68: it’s not necessary for the class to implement serialVerisonUID
- Line 82 made the parameter final
- Line 82: it’s not necessary for the class to implement serialVerisonUID
- Line 96: made the parameter final
- Line 96: it’s not necessary for the class to implement serialVerisonUID
- Line 97: added a default Locale to {{toUpperCase}}
- Line 110: made the parameter final
- Line 110: it’s not necessary for the class to implement serialVerisonUID
- Line 111: added a default Locale to {{toLowerCase}}
- Line 159 & 161: NACHFRAGEN

class EntityJoinOperator:
- Line 40: it’s not necessary for the class to implement serialVerisonUID

class EntityOperator:
- removed unnecessary else
- Line 64: added a default Locale to {{toLowerCase}}
- Line 65: added a default Locale to {{toUpperCase}}
- Line 303: was intended this way, since no equals method was implemented

class EnitityWhereString:
- Line 45: made the parameter private
- Line 47: it’s not necessary for the class to implement serialVerisonUID

class OrderByItem:
- Line 31: this is not a problem in the Ofbiz-api
- Line 82: added a default Locale to {{toUpperCase()}}
- Line 90: added a default Locale to {{toUpperCase()}}
- Line 121: added a defsult Locale to {{toUpperCase()}}
- Line 200: implemented the method {{hashCode()}} because {{equals()}} was 
implemented

class OrderByList:
- Line 31: this is not a problem in the Ofbiz-api
- Line 107: implemented the method {{hashCode()}} because {{equals()}} was 
implemented

> [FB] Package org.apache.ofbiz.entity.condition
> --
>
> Key: OFBIZ-9713
> URL: https://issues.apache.org/jira/browse/OFBIZ-9713
> Project: OFBiz
>  Issue Type: Sub-task
>  Components: framework
>Affects Versions: Trunk
>Reporter: Dennis Balkir
>Priority: Minor
> Attachments: 
> OFBIZ-9713_org.apache.ofbiz.entity.condition_bugfixes.patch
>
>
> - EntityClause.java:142, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of 
> org.apache.ofbiz.entity.condition.EntityOperator.getCode(), which is known to 
> be non-null in org.apache.ofbiz.entity.condition.EntityClause.toString()
> This method contains a redundant check of a known non-null value against the 
> constant null.
> - EntityClause.java:144, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of Object.toString(), which is known to be non-null 
> in org.apache.ofbiz.entity.c

[jira] [Created] (OFBIZ-9714) [FB] Package org.apache.ofbiz.service.rmi.socket.ssl

2017-09-13 Thread Dennis Balkir (JIRA)
Dennis Balkir created OFBIZ-9714:


 Summary: [FB] Package org.apache.ofbiz.service.rmi.socket.ssl
 Key: OFBIZ-9714
 URL: https://issues.apache.org/jira/browse/OFBIZ-9714
 Project: OFBiz
  Issue Type: Sub-task
  Components: framework
Affects Versions: Trunk
Reporter: Dennis Balkir
Priority: Minor


- SSLClientSocketFactory.java:37, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLClientSocketFactory 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.

- SSLServerSocketFactory.java:43, SE_NO_SERIALVERSIONID
SnVI: org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory 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.

- SSLServerSocketFactory.java:76, OS_OPEN_STREAM
OS: 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
 may fail to close stream

The method creates an IO stream object, does not assign it to any fields, pass 
it to other methods that might close it, or return it, and does not appear to 
close the stream on all paths out of the method.  This may result in a file 
descriptor leak.  It is generally a good idea to use a finally block to ensure 
that streams are closed.

- SSLServerSocketFactory.java:76, OBL_UNSATISFIED_OBLIGATION
OBL: 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)
 may fail to clean up java.io.InputStream

This method may fail to clean up (close, dispose of) a stream, database object, 
or other resource requiring an explicit cleanup operation.

In general, if a method opens a stream or other resource, the method should use 
a try/finally block to ensure that the stream or resource is cleaned up before 
the method returns.

This bug pattern is essentially the same as the OS_OPEN_STREAM and 
ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and 
hopefully better) static analysis technique. We are interested is getting 
feedback about the usefulness of this bug pattern. To send feedback, either:

send email to findb...@cs.umd.edu
file a bug report: http://findbugs.sourceforge.net/reportingBugs.html
In particular, the false-positive suppression heuristics for this bug pattern 
have not been extensively tuned, so reports about false positives are helpful 
to us.

See Weimer and Necula, Finding and Preventing Run-Time Error Handling Mistakes, 
for a description of the analysis technique.

- SSLServerSocketFactory.java:111, BC_UNCONFIRMED_CAST_OF_RETURN_VALUE
BC: Unchecked/unconfirmed cast from java.net.ServerSocket to 
javax.net.ssl.SSLServerSocket of return value in 
org.apache.ofbiz.service.rmi.socket.ssl.SSLServerSocketFactory.createServerSocket(int)

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.



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


  1   2   3   4   5   6   >