This section helps to understand the supported code checks for Java language. Embold code checks can be enabled and disabled according to user requirement.

Note: Currently, we support Embold Java Issues only; and does not support PMD issues and SpotBugs issues.

Critical Level Issues

RuleDescriptionExampleKPITags
Compile Regex OncePattern object compiles the regular expressions which are passed to them and this compilation happens in the memory. If a regular expression is used many times, then this compilation should be performed only once.Non-compliant code:
Class Foo {
// This method can call multiple times
void findText(String inputText) {
Pattern pattern = Pattern.compile("(.*)(\\d+)(.*)");
Matcher matcher = pattern.matcher(inputText);
if(matcher.find()) {
MatchResult result = matcher.toMatchResult();
}
}
}


Compliant code:
Class Foo {
Pattern pattern = Pattern.compile("(.*)(\\d+)(.*)");

void findText(String inputText) {
Matcher matcher = pattern.matcher(inputText);
if(matcher.find()) {
MatchResult result = matcher.toMatchResult();
}
}
}
Efficiency
Disabled Spring Securitys CSRFCSRF protection is enabled by default in the Java configuration. Disabling CSRF protection can create a major security threat.
Cross-Site Request Forgery (CSRF) is an attack that forces an end user to execute unwanted actions on a web application in which they're currently authenticated.

More Info - OWASP Top 10 A6:2017 - Security Misconfiguration
CWE-352 - Cross-Site Request Forgery (CSRF)
Non-compliant code:
@Configuration
@EnableWebSecurity
public class SecurityConfig extends WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
http.csrf().disable();
}
}


Compliant code:
@Configuration
@EnableWebSecurity
public class SecurityConfig extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
// Do not disable CSRF
// http.csrf().disable();
}
}
Security
Resource LeakResource should be closed in finally block. Another way is to use try-with-resource. In case of either exception or no exception, close() should always be put in finally clause.Non-compliant code:
class Demo {

public void process1() {
try {
PrintWriter out = new PrintWriter(new BufferedWriter(new FileWriter("out.txt", true)));
out.println("the text");
out.close(); //close() is in try clause
} catch (IOException e) {
logger.error("resource is closed in try block",e);
}
}

//Non-compliant code
//resource is not closed anywhere
public void process2() {
try {
PrintWriter out = new PrintWriter(new BufferedWriter(new FileWriter("out.txt", true)));
out.println("the text");
} catch (IOException e) {
logger.error("Resource is not closed anywhere.",e);
}
}
}


Compliant code:
class Demo {
public void process2() {
printWriter out = null;
try {
out = new PrintWriter(new BufferedWriter(new FileWriter("out.txt", true)));
out.println("the text");
} catch (IOException e) {
logger.error("Resource is closed in finally block",e);
} finally {
if (out != null) {
out.close(); //close() is in finally clause
}
}
}

//compliant code
//try-with-resource statement
public void process3() {
try (PrintWriter out2 = new PrintWriter(new BufferedWriter(new FileWriter("out.txt", true)))) {
out2.println("the text");
} catch (IOException e) {
logger.error("try-with-resource",e);
}
}
}
Resource Utilization
Weak Cipher AlgorithmUse strong cryptographic algorithms as they are less vulnerable to brute force attacks among others.
For more information : : OWASP Top 10 2017 Category A3 - Sensitive Data Exposure
MITRE, CWE-327 - Use of a Broken or Risky Cryptographic Algorithm
Non-compliant code:
class CipherExample
{
public void foo()
{
try
{
Cipher c = Cipher.getInstance("DES"); // Or DESede, RC2, RC4 as these are known to be vulnerable
} catch (NoSuchAlgorithmException | NoSuchPaddingException e)
{
logger.error("Invalid algorithm",e);
}
}
}


Compliant code:

class CipherExample
{
public void foo()
{
try
{
Cipher c = Cipher.getInstance("AES/GCM/NoPadding"); // Compliant
} catch(NoSuchAlgorithmException|NoSuchPaddingException e)
{
logger.error("Invalid algorithm",e);
}
}
}
Security
Possible Thread Leak In Executor Service Thread leak can be possible if ExecutorService is not getting shutdown.There is a queue of tasks between client threads and thread pool. When the normal thread finishes the "run" method (Runnable or Callable), it will be passed to garbage collector for collection. But with ExecuterService, threads will be simply put on hold and they will not be selected for garbage collection. Therefore, the shutdown is needed for ExecutorService.
Non-compliant code:
class Demo {

//ExecutorService is not closed anywhere
public void process1() {
ExecutorService executorService = Executors.newSingleThreadExecutor();
executorService.execute(new Runnable() {
@Override
public void run() {
try {
doTask();
} catch (Exception e) {
logger.error("indexing failed", e);
}
}
});
}
}


Compliant code:
class Demo {

public void process2() {
ExecutorService executorService = Executors.newSingleThreadExecutor();
executorService.execute(new Runnable() {
@Override
public void run() {
try {
doTask();
} catch (Exception e) {
logger.error("indexing failed", e);
}
}
});
executorService.shutdown();
}
}

Resource Utilization
Non Private Field In Synchronized BlockNon-private, non-final field accessed in synchronized block indicates possibly partial synchronization, and does not protect direct access to the field from other parts of the system without synchronization.
Make the field private and/or final, and provide accessors which can enforce synchronization.

More Info - CWE-820 - Missing synchronization
Non-compliant code:
class NonPrivateFieldAccessInSynchronizedBlock {

protected List statuses = new ArrayList<>(); // or public
private Object lock = new Object();

void foo() {
// Some processing
synchronized(lock) {
statuses.add("Running");
}
}
// Some more processing
}


Compliant code
class NonPrivateFieldAccessInSynchronizedBlock {

private List statuses = new ArrayList<>();
private Object lock = new Object();

void foo() {
// Some processing
synchronized(lock) {
statuses.add("Running");
}

// Some more processing
}

void updateStatus(String status) {
synchronized(lock) {
statuses.add(status);
}
}
}
Robustness, Security
Database Should Be Password ProtectedDatabase connection should always be password protected. Attacker can access the sensitive data from the database, in case it is not secured by password.

More Info
- [OWASP Top 10 2017 Category A3] - Sensitive Data Exposure
- [MITRE, CWE-521] - Weak Password Requirements
Non-compliant code:
class CipherExample {
public void foo() {

Connection conn1 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true", "AppLogin", "");
Connection conn2 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true?user=user&password=");
Connection conn3 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true?user=user");
}
}


Compliant code
class CipherExample {
public void foo() {
Connection conn4 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true", "AppLogin", "password");
Connection conn5 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true?user=user&password=text");
Connection conn6 = DriverManager.getConnection("jdbc:derby:memory:myDB;create=true&password=text", "AppLogin", "");
}
}
Security
Should Not Use getRequestedSessionId APIThis method returns the session ID specified by the client and may not be the same as the ID of the current valid session for this request. If the client does not mention a session ID, it returns null.
The session ID returned is either transmitted in a cookie or a URL parameter, hence by definition nothing prevents the end-user from manually updating the value of this session ID in the HTTP request.

More Info
- [CWE-807] - Reliance on Untrusted Inputs in a Security Decision

- [OWASP] - Broken Authentication

class Demo{
// Non-compliant code
public void method(HttpServletRequest request) {
if(isActiveSession(request.getRequestedSessionId()) ){

}
}
}

Security
Web Application Contains Main MethodThis method returns the session ID specified by the client and may not be the same as the ID of the current valid session for this request. If the client does not mention a session ID, it returns null.
The session ID returned is either transmitted in a cookie or a URL parameter, hence by definition nothing prevents the end-user from manually updating the value of this session ID in the HTTP request.

More Info

- [CWE-489] - Leftover Debug Code

- [OWASP] - Sensitive Data Exposure

class Demo{
// Non-compliant code
public void method(HttpServletRequest request) {
if(isActiveSession(request.getRequestedSessionId()) ){

}
}
}


```
Security
Authenticate LDAP Connection Anonymous binds and unauthenticated binds allow access to information in the LDAP directory without providing a password. Therefore using these binds is strongly discouraged.

More Info

- [OWASP Top 10 2017 Category A2]- Broken Authentication

- [CWE-521] - Weak Password Requirements

- [ldapwiki.com] - Simple Authentication

//Non-compliant code
Hashtable env = new Hashtable();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
// Use anonymous authentication
env.put(Context.SECURITY_AUTHENTICATION, "none"); // Noncompliant

// Compliant code
Hashtable env = new Hashtable();
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.SECURITY_AUTHENTICATION, "simple");

Security
Potential Command InjectionAvoid using unfiltered input to process executor APIs because that can lead to arbitrary command execution.

More Info

- [OWASP] - Command Injection

- [OWASP: Top 10 2013 A1] -Injection

- [CWE-78] - Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')


// Non-compliant code
import java.lang.Runtime;
class Demo{
Runtime r = Runtime.getRuntime();
r.exec("/bin/sh -c sometool" + input);
}

Security
Potential Path Traversal Path traversal is a web security vulnerability that allows an attacker to read arbitrary files on the server that is running an application.
This might include application code and data, credentials for back-end systems, and sensitive operating system files.

More Info
- [CWE-22] - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')

- [OWASP] - Path Traversal

// Non-compliant code
class Demo{
@GET
@Path("/images/{image}")
@Produces("images/*")
public Response getImage(@javax.ws.rs.PathParam("image") String image) {
File file = new File("resources/images/", image); //Weak point

if (!file.exists()) {
return Response.status(Status.NOT_FOUND).build();
}

return Response.ok().entity(new FileInputStream(file)).build();
}

// Compliant code
}
import org.apache.commons.io.FilenameUtils;
class Demo1{
@GET
@Path("/images/{image}")
@Produces("images/*")
public Response getImage(@javax.ws.rs.PathParam("image") String image) {
File file = new File("resources/images/", FilenameUtils.getName(image)); //Fix

if (!file.exists()) {
return Response.status(Status.NOT_FOUND).build();
}

return Response.ok().entity(new FileInputStream(file)).build();
}
}


Security
Accessing Android external storage is security-sensitiveFiles or data stored in the android application should be security-sensitive. Files can be globally readable or writable as they are stored on external storage.
Avoid storing sensitive information on the external storage as anyone can tamper with the data or remove the files by using any application.

**More Info**
- [OWASP Top 10 2017 Category A1](https://owasp.org/www-project-top-ten/2017/A1_2017-Injection.html) - Injection
- [OWASP Top 10 2017 Category A3](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) - Sensitive Data Exposure
- [CWE-312](https://cwe.mitre.org/data/definitions/312.html) - Cleartext Storage of Sensitive Information
- [CWE-20](https://cwe.mitre.org/data/definitions/20.html) - Improper Input Validation accessing android external storage


```java
//non-compliant
import android.content.Context;
import android.os.Environment;
public class AccessExternalFiles {
public void accessFiles(Context context) {
Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_PICTURES); // non-compliant
context.getExternalFilesDir(Environment.DIRECTORY_PICTURES);
// non-compliant
}
}


Security
Using unsafe Jackson deserialization configuration is security-sensitiveJThe untrusted data could cause abuse to application logic. Jackson's deserialization should be configured securely.
Implement @JsonTypeName and @JsonSubTypes to prevent serialized data from an untrusted source. It can help to safeguard the data from external attackers.

**More Info**
- [OWASP Top 10 2017 Category A8](https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization) - Insecure Deserialization
- [CWE-502](https://cwe.mitre.org/data/definitions/502.html) - Deserialization of Untrusted Data


//non-compliant
ObjectMapper mapper = new ObjectMapper();
mapper.enableDefaultTyping(); // non-compliant
@JsonTypeInfo(use = Id.CLASS) // non-compliant
abstract class PhoneNumber {
}



Security
Setting JavaBean properties is security-sensitiveThe JavaBean property uses a set or get functions that are exposed to other applications. An attacker can modify its properties, attack malicious code that can be risky. Avoid storing sensitive information under this JavaBean property as it may help the user to retain its software integrity.

**More Info**
- [OWASP Top 10 2017 Category A8](https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization) - Insecure Deserialization
- [CWE-502](https://cwe.mitre.org/data/definitions/502.html) - Deserialization of Untrusted Data


//non-compliant
class Demo{
Company bean = new Company();
HashMap map = new HashMap();
Enumeration names = request.getParameterNames();
while (names.hasMoreElements()) {
String name = (String) names.nextElement();
map.put(name, request.getParameterValues(name));
}
BeanUtils.populate(bean, map); // "map" is populated with data coming from user input, here "request.getParameterNames()"
}



Security
Run Should Not Be Called DirectlyThe program accidentally calls the Thread.run() method and causes the code to be executed in the current thread, just like any other method call. Instead, use Thread.start() to actually create a new thread so that the runnable's 'run()' method is executed in parallel.

More Info
[CWE-572] - Call to Thread run() instead of start()
Non-compliant code
class Demo {
Thread myThread = new Thread(runnable);
myThread.run();

}


Compliant code
class Demo {
Thread myThread = new Thread(runnable);
myThread.start();
}
Functionality, Efficiency
Exclude SpringBootApplication And ComponentScan From The Default PackageExclude "SpringBootApplication" and "@ComponentScan" from the default package.Non-compliant code
import org.springframework.boot.SpringApplication;
@SpringBootApplication
public class SpringBootApplicationAndComponentScanNotBeUsedInDefaultPackage {
public static void main(String[] args)
{
SpringApplication.run(SpringBootApplicationAndComponentScanNotBeUsedInDefaultPackage.class, args);
}
}


Compliant code
package javacodechecker;
import org.springframework.boot.SpringApplication;
@SpringBootApplication
public class SpringBootApplicationAndComponentScanNotBeUsedInDefaultPackage {
public static void main(String[] args)
{
SpringApplication.run(SpringBootApplicationAndComponentScanNotBeUsedInDefaultPackage.class, args);
}
}
Efficiencyspring

High-Level Issues

RuleDescriptionExampleKPI
Initialization Of Secure Random At Method Level"SecureRandom" should not be initialized in method. Every SecureRandom generation is seeded from some entropy pool. Creating a new SecureRandom method on every call might slow down the application as it might block the creation of seed. Use a statically created instance instead.Non-compliant code
class Demo {

public int generateSecureKey() {
SecureRandom secureRandom = new SecureRandom();
return secureRandom.nextInt();
}
}


Compliant code
class SecureRandomGenerator {
static SecureRandom secureRandom = new SecureRandom();
public int generateSecureKey() {
return secureRandom.nextInt();
}
}
Security
Security Sensitive RegexRegular expressions are security-sensitive. Evaluating regular expressions with input string can be an extremely CPU-intensive task. The regular expression naive algorithm builds a Nondeterministic Finite Automaton (NFA), the attacker might use the knowledge of states of finite state machine to look for applications that use regular expressions, containing an Evil Regex, and send a well-crafted input, that will hang the system. Alternatively, if a Regex itself is affected by a user input, the attacker can inject an Evil Regex, and make the system vulnerable.
Examples of Evil Patterns:
Examples of Evil pattern : 1) (a+)+ 2) ([a-zA-Z]+)* 3) (a|aa)+ 4) (a|a?)+ 5) (.*a){2} for x > 10

E.g
Pattern: /(a+)+b/;
Input string:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab
Instead use Pattern:(/a+b/) //fixed the hard-coded regex pattern to avoid possible captures, possessive quantifiers and back-references

To avoid possible attacks, if the regex pattern is defined with an user-controlled input, it should be sanitized in order to escape characters which are part of the regular expression syntax.

Non-compliant code
class Demo { public boolean validate1(javax.servlet.http.HttpServletRequest request) { String regex = " /(a+)+b/"; String input = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab"; input.matches(regex); //Noncompliant
}
}


Compliant code
class Demo { public boolean
{
validate2(javax.servlet.http.HttpServletRequest request)
{ String regex = "/a+b/"; String input = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab";
input.matches(Pattern.quote(regex)); // Compliant
} }

Efficiency, Security
Use Of System.err.println 1. System.err.println is an IO-operation and therefore is time consuming.
2. The better approach is to use a logging framework for a message queue.
3. Moreover, you can configure separate log files for different purposes.
Non-compliant code
class Demo {
public void process1() {
System.err.println("Error while processing");
}
}


Compliant code
class Demo {
public void process2() {
logger.error("Error while processing",e);
}
}
Analyzability
Invalid Logging Class Name Avoid incorrect class names while creating logger. Ignoring this may create confusion while analysing logs.
Non-compliant code
public class LoggingClass extends BaseChecker {
private static Logger logger = LogManager.getLogger(InvalidLoggingClass.class.getName());
}


Compliant code
public class LoggingClass extends BaseChecker {
private static Logger logger = LogManager.getLogger(LoggingClass.class.getName());
}

Analyzability
Use Of System.out.println 1.Sending messages to stdout is usually inappropriate in a production environment. E.g. If you are coding a GUI app, the information should be presented to the user and not to the stdout method anywhere.
2.System.out.println is an IO-operation and therefore is time consuming. The better approach is to use a logging framework for a message queue.
3.Moreover, you can configure separate log files for different purposes.
Non-compliant code
class Demo {
public void process1() {
//some statements
System.out.println("Some text");
}


Compliant code
public void process2() {
//some statements
logger.info("Some text");
}
}

Analyzability
Complex Regex PatternAvoid usage of complex Regex pattern as it involves heavy processing.In some cases complex Regex performance test shows that regex is significantly inefficient. //Non-compliant code
class Demo {
String regex = "(.*)(\\d+)(.*)";
Pattern p = Pattern.compile(regex);
}

Efficiency
Empty Catch BlockEmpty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this ignores an exception which should either be acted on or reported. Avoid keeping the catch block empty. // Non-compliant code
class Demo {

public void process1() {
try {
FileInputStream fis = new FileInputStream("/tmp/bugger");
} catch (IOException ioe) {
}
}


// compliant code
public void process2() {
try {
FileInputStream fis = new FileInputStream("/tmp/bugger");
} catch (IOException ex) {
logger.error("Inside catch block",ex);
}
}
}
Analyzability
Empty Catch BlockEmpty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this ignores an exception which should either be acted on or reported. Avoid keeping the catch block empty. // Non-compliant code
class Demo {

public void process1() {
try {
FileInputStream fis = new FileInputStream("/tmp/bugger");
} catch (IOException ioe) {
}
}


// compliant code
public void process2() {
try {
FileInputStream fis = new FileInputStream("/tmp/bugger");
} catch (IOException ex) {
logger.error("Inside catch block",ex);
}
}
}
Analyzability
Sensitive Info LoggedLogger is used for recording application activity. Information displayed on the logs is visible to different stakeholders. Avoid logging sensitive information such as password, key, social security number etc. Not complying may compromise the system security.Non-compliant code:
class Demo {
void process1(String password) {
//Displaying sensitive information in logs
logger.info("Password received "+ password);
}
}


Compliant code
class Demo {
void process1(String password) {
//doSomething
logger.info("Password received");
}
}
Security
Main Should Not Throw AnythingThe main method should not throw any checked exception, instead, it needs to handle properly. If a non-checked exception is thrown (and not catch) in the main method, it will terminate.Non-compliant code:

public static void main(String args[]) throws Exception{
System.out.println("Hello");
}

}


Compliant code
public static void main(String[] args) {
try {
int data=50/0;
}
catch(ArithmeticException e){
System.out.println(e);
}
}
Functionality
Avoid Synchronized At Method LevelAvoid synchronized at method level. When new code is added to the existing method then method-level synchronization can cause problems so to avoid these kind of problems use block-level synchronization. Block-level synchronization helps to ensure that only the code that needs synchronization gets it.Non-compliant code:

public class SynchronizedCounter {
// Non-compliant code
private int c = 0;
public synchronized void increment() {
c++;
}
}


Compliant code
public class SynchronizedCounter {
// compliant code
private int c = 0;
public void increment() {
synchronized(Test.class) {
c++;
}
}
}
Efficiency
Shortcircuit Logic Should Be Used In Boolean ContextsUsing non-short-curcuit logic is a mistake it can also cause serious problem error.Non-compliant code:

if(getTrue() | getFalse()) {
...
}


Compliant code

if(getTrue() || getFalse()) {
...
}
Efficiency

Medium Level Issues

RuleDescriptionExampleKPI
Return Empty Array Or Collection Instead Of Null Returning null value instead of an empty array or collection can lead to denial-of-service vulnerabilities when the client code fails to explicitly handle the null return value. For methods that return a set of values using an array or collection, returning an empty array or collection is an excellent alternative to returning a null value.

More Info
MET55-J - Return an empty array or collection instead of a null value for methods that return an array or collection

MSC19-C - For functions that return an array, prefer returning an empty array over a null value
Non-compliant code:
class Demo {
public static List getList() {
return null; // Non-compliant code
}

public static void main(String[] args) {
List results = getList();
for (Result result: results) { } // Need to add Nullity check to prevent NPE
}
}


Compliant code:
class Demo {
public static List getList() {
return Collections.emptyList(); // Compliant code
}

public static void main(String[] args) {
List results = getList();
for (Result result: results) { } // No need to add Nullity check to prevent NPE
}
}
Robustness
Preserve Stack Trace In LogsPreserve stack trace in logs. This will help to analyse the logs in case of any exception. Non-compliant code:
class Demo {
public void process1() {
try{
//some statements
} catch(Exception e) {
logger.error("Error while processing");
}
}
}


Compliant code:
class Demo {
public void process2() {
try {
//some statements
} catch(Exception e) {
logger.error("Error while processing",e);
}
}
}

Analyzability
Read Only TransactionSpring components support database transactions using "@Transactional" annotation. If readOnly attribute is not explicitly set to true, we will have read/write transactions for select queries. Hence, it is always recommended to explicitly specify the readOnly attribute.Non-Compliant Code:
class Demo {
public class UserRepository {
@Query("select username from users")
public List getAllUsers() {
}
}
}


Compliant Code:
class Demo {
@Transactional(readOnly=true)
public class UserRepository {
@Query("select username from users")
public List getAllUsers() {
}
}
//Compliant Code
public class UserRepository {
@Transactional(readOnly=true)
@Query("select username from users")
public List getAllUsers() {
}
}
}
Robustness
Unusual REST PracticeThe best practices while creating REST API's are :
1. URL should contain resources (E.g. nouns) only; not actions or verbs.

2. Singular and plural noun should not be mixed together.

3. Use plural noun only for all the resources.

4. Use GET method, instead of the POST method to fetch the data.

5. Use PUT, POST and DELETE methods to alter the state.
Non-compliant code
class Demo {
@RequestMapping("/users")
public class UserController{

@RequestMapping(value="/getUser")
public void getUser(){
}
}
}


Compliant code
class Demo {
@RequestMapping("/users")
public class UserController{

@RequestMapping(value="/{user}")
public void getUser() {
}
}
}
Maintainability
Variables Should Not Be Self AssignedSelf-assignment of the variables can be confusing and leads to bugs; however, one should not assign a variable to itself. Hence, this statement can be redundant and removed.Non-compliant code
class Demo {
public void demo(String name, String surName) {
String surName = surName;
name = name;
}
}


Compliant code
class Demo {
public void setName(String name) {
this.name = name;
}
}
Functionality
Externalizable Must Have No Arguments ConstructorExternalizable interface cannot be deserialized without a non-argument constructor,so non-argument constructor must be implemented.Non-compliant code
public class Car implements Externalizable {
public Car(String color,String model,int avg){
}
}


Compliant code
public class Car implements Externalizable {
public Car() {}
public Car (String color,String model,int avg){
}
Efficiency
Getters And Setters Should Access The Expected FieldsGetter and Setter methods must access the expected fields. For each instance variable, a getter method returns its value, while a setter method sets or updates its value.
For example, 'active' is an instance variable and 'setActive' (boolean value) is a setter method. Instead of unexpectedly updating any field, it must update or set the active variable.
Non-compliant code
class Demo {
// Non-compliant code
private boolean active;
public void setActive(boolean b)
{
this.Y = b;
}


Compliant code
private boolean active;
public void setActive(boolean b)
{
this.active = b;
}
}
Functionality
RunFinalizersOnExit Should Not Be CalledRemove Runtime::runFinalizersOnExit and System::runFinalizersOnExit methods. It can be enabled with "System.runFinalizersOnExit" and "Runtime.runFinalizersOnExit".It may result in finalizers being called on live objects while other threads are concurrently manipulating those objects, resulting in unexpected behavior or deadlock.

- [CWE-586]- Explicit Call to Finalize()
Non-compliant code
public static void main(String [] args) {

System.runFinalizersOnExit(true); // Noncompliant

}


Compliant code
public static void main(String [] args) {
Runtime.addShutdownHook(new Runnable() {
public void run(){
doSomething();
}
});
Efficiency
Big Integer InstantiationUse already existing BigIntegers (BigInteger.ZERO, BigInteger.ONE, BigInteger.TEN).Instead of creating a new object with new BigInteger better use one static object which is created once when the BigInteger class is loaded. It is likely to yield significantly better space and time performance. Zero and One is probably the most fundamental number in mathematics. Using static objects avoids the allocation about 48 bytes and the need to collect them back later in a tight loop that can matter. Non-compliant code
BigInteger bigInteger = new BigInteger("1");
int n=4;
for (int i = 2; i <=n ; i++){
bigInteger = bigInteger.multiply(BigInteger.valueOf(i));
}
System.out.println("Factorial of 4 : "+bigInteger);


Compliant code
BigInteger bigInteger = BigInteger.ONE;
int n=4;
for (int i = 2; i <=n ; i++){
bigInteger = bigInteger.multiply(BigInteger.valueOf(i));
}
System.out.println("Factorial of 4 : "+bigInteger);

Efficiency
Maps With Enum Values Replace With EnumMapIf Map has all the key values from the same Enum then Map should be replaced with EnumMap because the underlying data structure is a simple array so it will be more efficient than other sets.Non-compliant code
public class MyClass {
public enum CITY {
PUNE, MUMBAI, GOA, NAGPUR;
}
public void mapMood() {
Map moodMap = new HashMap ();
}
}


Compliant code
public class MyClass {
public enum CITY {
PUNE, MUMBAI, GOA, NAGPUR;
}
public void mapMood() {
EnumMap moodMap = new EnumMap<> (CITY.class);
}
}
Functionality
Mismatch Regex Boundaries Should Not Be UsedIn the regular expression by switching $ and ^ boundaries it will never match and it can be misused.Noncompliant Code

pattern.compile("$[a-z]+^");

Compliant Code

pattern.compile("^[a-z]+$");
Functionality
Avoid Concatenating Char As StringAvoid concatenating characters as strings because using string rather than char creates unnecessarily space accommodation in heap space. Appending a character as a char will always be faster than appending it as a String.Noncompliant Code

class Demo {
StringBuffer sb = new StringBuffer("Hi ");
sb.append("i");
System.out.println(sb);
}


Compliant Code

class Demo {
StringBuffer sb = new StringBuffer("Hi ");
sb.append('i');
System.out.println(sb);
}

Functionality
Empty String Should Not Be UsedConcatenating empty string with literals during conversion is inefficient.Noncompliant Code

String s = "" + 456;

Compliant Code

String t = Integer.toString(456);
Functionality
Exceptions Should Not Be Thrown In Finally BlockException that is thrown in finally block will mask any previous exception in try or catch block and the stack trace and exception message will be lost. Noncompliant Code

try {
throw new IllegalArgumentException();
} finally {
throw new RuntimeException();
}


Compliant Code

try {
throw new IllegalArgumentException();
} finally {

}

Functionality

Low-Level Issues

RuleDescriptionExampleKPI
Log Level Info In Catch BlockThe catch block handles exception that occurs in associated try block. In this block, the logger level is expected to be as "error". This is a good list describing the log levels :
1. Debug : Used for development and testing.
2. Information : Used to output information that is useful to run and manage your system.
3. Warning : Used for handled 'exceptions' or other important log events.
4. Error : Used to log all unhandled exceptions. This is typically logged inside a catch block at the boundary of your application.
5. Fatal : Used for special exceptions/conditions where it is imperative that we can quickly pick out these events.
Non-compliant code:
class Demo {
public void process1() {
try {
//statements where exceptions may occur
} catch(Exception ex) {
logger.info("some information",ex);
}
}
}


Compliant code:
class Demo {
public void process2() {
try {
//statements where exceptions may occur
} catch(Exception ex) {
logger.error("some information",ex);
}
}
}
Analyzability
Non Thread Safe Field DeclarationA field declared inside the spring component should be thread safe. By default, beans in spring are singleton. Multiple threads accessing the same component may produce inconsistent results, if they access fields declared globally.Non-compliant code:
class Demo {
@Service
public class LoggingClass extends BaseChecker {
private Map map = new HashMap();
void modifyMap(){
map.add("key","value");
}
}
}


Compliant code:
class Demo {
@Service
public class LoggingClass extends BaseChecker {
void modifyMap(){
Map map = new HashMap();
map.add("key","value");
}
}
}

Robustness

Info Level Issues

RuleDescriptionExampleKPI
String Concatenation In LoggingInstead of string concatenation use parameterized logging. Concatenation is calculated before the condition check. If you call your logging framework multiple times which turns to be false, this effort will be a waste of time. E.g. if you call your logging framework 10K times conditionally and all of them evaluates to be false, concatenation will happen 10K times.Non-compliant code:
class Demo {
//If log level is not debug, then also string concatenation will happen.
public void process1() {
logger.debug("Comparing objects: " + object1 + " and " + object2);
}
}


Compliant code:
class Demo {
public void process2() {
//Compliant code
logger.debug("Comparing objects: {} and {}",object1, object2);
}
}
Efficiency
Tightly Coupled ClassA spring component such as repository, service and controller should auto-wire the interface, instead of its implementation class. Ignoring this, will increase tight coupling among the classes Non-compliant code:
class Demo {
@RequestMapping("/users")
public class UserController
{
@Autowire
UserServiceImpl userServiceImpl;
}
}

Compliant code:
class Demo {
@RequestMapping("/users")
public class UserController
{
@Autowire
UserService userService;
}
}
Maintainability