If data members are declared public
or protected
, it is difficult to control how they are accessed. Malicious callers can manipulate such members in unintended ways. If class members need to be exposed beyond the package their class is declared in, acceessor methods may be used. Also, with the use of setter methods, modification of data members can be monitored as appropriate (e.g., by defensive copying, validating input, logging and so on). Methods that are declared public
or protected
must preserve the invariants of the class and their use should not be abused.
Noncompliant Code Example
In this noncompliant code example, the data member total
is meant to keep track of the total number of elements as they are added and removed from a container. However, as a public
data member, total
can be altered by external code, independent of these actions. This noncompiant code example violates the condition that public
classes must not expose data members by declaring them public
. It is a bad practice to expose both mutable as well as immutable fields from a public
class [[Bloch 08]].
public class Widget { public int total; void add() { if (total < Integer.MAX_VALUE) { total++; // ... } throw new ArithmeticException("Overflow"); } void remove() { if (total > Integer.MIN_VALUE) { total--; // ... } throw new ArithmeticException("Overflow"); } public int getTotal () { return total; } }
Compliant Solution
This compliant solution declares total
as private
and provides a public
accessor so that the class can be accessed beyond the current package. The method add()
modifies its value without violating any class invariants.
public class Widget { private int total; // Declared private void add() { // ... } void remove() { // ... } public int getTotal () { // ... } }
It is good practice to use wrapper methods such as add()
, remove()
and getTotal
, to manipulate private internal state because the methods can perform additional functions such as input validation and security manager checks prior to manipulating the state.
Noncompliant Code Example
This noncompliant code example shows a static
, mutable hash map with public
accessibility.
public static final HashMap<Integer, String> hm = new HashMap<Integer, String>();
Compliant Solution
Mutable data members that are static
must always be declared private
.
private static final HashMap<Integer, String> hm = new HashMap<Integer, String>();
Exceptions
EX1: According to Sun's Code Conventions document [[Conventions 09]]:
One example of appropriate
public
instance variables is the case where the class is essentially a data structure, with no behavior. In other words, if you would have used astruct
instead of a class (if Java supportedstruct
), then it's appropriate to make the class's instance variablespublic
.
EX2: "if a class is package-private or is a private
nested class, there is nothing inherently wrong with exposing its data fields - assuming they do an adequate job of describing the abstraction provided by the class. This approach generates less visual clutter than the accessor-method approach, both in the class definition and in the client code that uses it." [[Bloch 08]]. This exception applies to both mutable as well as immutable fields.
Risk Assessment
Failing to declare data members private
can break encapsulation.
Recommendation |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
OBJ00- J |
medium |
likely |
medium |
P12 |
L1 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Other Languages
This rule appears in the C++ Secure Coding Standard as OBJ00-CPP. Declare data members private.
References
[[JLS 06]] Section 6.6, Access Control
[[SCG 07]] Guideline 3-2 Define wrapper methods around modifiable internal state
[[Long 05]] Section 2.2, Public Fields
[[Bloch 08]] Items 13: Minimize the accessibility of classes and members; 14: In public classes, use accessor methods, not public fields
08. Object Orientation (OBJ) 08. Object Orientation (OBJ) OBJ01-J. Be aware that a final reference may not always refer to immutable data