FindBugs rules - miscellaneous

FindBugs rules for Experimental

Potential lost logger changes due to weak reference in OpenJDK

OpenJDK introduces a potential incompatibility.

In particular, the java.util.logging.Logger behavior has changed. Instead of using strong references, it now uses weak references internally. That's a reasonable change, but unfortunately some code relies on the old behavior - when changing logger configuration, it simply drops the logger reference.

That means that the garbage collector is free to reclaim that memory, which means that the logger configuration is lost.

For example, consider : 

public static void initLogging() throws Exception {

  Logger logger = Logger.getLogger("edu.umd.cs");

  // call to change logger configuration

  logger.addHandler(new FileHandler());  

  // another call to change logger configuration

  logger.setUseParentHandlers(false);

}

The logger reference is lost at the end of the method (it doesn't escape the method), so if you have a garbage collection cycle justafter the call to initLogging, the logger configuration is lost (because Logger only keeps weak references).

public static void main(String[] args) throws Exception {

   initLogging();  // adds a file handler to the logger

   System.gc();  // logger configuration lost

   // this isn't logged to the file as expected

   Logger.getLogger("edu.umd.cs").info("Some message");

}

Method may fail to clean up stream or resource

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 findbugs@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.

Method may fail to clean up stream or resource on checked exception

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 findbugs@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.

FindBugs rules for Internationalization

Consider using Locale parameterized version of invoked method

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.

Reliance on default encoding

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.

FindBugs rules for Malicious code vulnerability

Classloaders should only be created inside doPrivileged block

This code creates a classloader, which needs permission if a security manage is installed.

If this code might be invoked by code that does not have security permissions, then the classloader creation needs to occur inside a doPrivileged block.

Method invoked that should be only be invoked inside a doPrivileged block

This code invokes a method that requires a security permission check.

If this code will be granted security permissions, but might be invoked by code that does not have security permissions, then the invocation needs to occur inside a doPrivileged block.

May expose internal representation by returning reference to mutable object

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.

May expose internal representation by incorporating reference to mutable object

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.

Finalizer should be protected, not public

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

May expose internal static state by storing a mutable object into a static field

This code stores a reference to an externally mutable object into a static field.

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

Field isn't final and can't be protected from malicious code

A mutable static field could be changed by malicious code or by accident from another package.

Unfortunately, the way the field is used doesn't allow any easy fix to this problem.

Public static method may expose internal representation by returning array

A public static method returns a reference to an array that is part of the static state of the class.

Any code that calls this method can freely modify the underlying array. One fix is to return a copy of the array.

Field should be both final and package protected

A mutable static field could be changed by malicious code or by accident from another package.

The field could be made package protected and/or made final to avoid this vulnerability.

Field is a mutable array

A final static field references an array and can be accessed by malicious code or by accident from another package.

This code can freely modify the contents of the array.

Field is a mutable Hashtable

A final static field references a Hashtable and can be accessed by malicious code or by accident from another package.

This code can freely modify the contents of the Hashtable.

Field should be moved out of an interface and made package protected

A final static field that is defined in an interface references a mutable object such as an array or hashtable.

This mutable object could be changed by malicious code or by accident from another package.

To solve this, the field needs to be moved to a class and made package protected to avoid this vulnerability.

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

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

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