Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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 wrapper accessor methods may be used. Also, with the use of setter wrapper 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 The wrapper methods must preserve the invariants of the class and their use should not be abusedat all times.

Noncompliant Code Example

...

(public primitive/immutable field)

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|AA. Java References#Bloch 08]\]. total keeps track of the total number of elements as they are added and removed from a container using the methods add() and remove(), respectively.

Code Block
bgColor#FFCCCC
public class Widget {
  public int total; // Number of elements

  void add() {
    if (total < Integer.MAX_VALUE) {      
      total++;
      // ...
    } else {
      throw new ArithmeticException("Overflow");
    }
  }

  void remove() {  
    if (total > Integer.MIN_VALUE) {      
      total--;
      // ...
    } else {
      throw new ArithmeticException("Overflow");
    }
  }
}

Wiki Markup
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|AA. Java References#Bloch 08]\].

Compliant Solution (provide wrappers and reduce accessibility of primitive/immutable members)

...

This compliant solution declares total as private and provides a public accessor so that the class required member can be accessed beyond the current package. The method add() and remove() modifies methods modify its value without violating any class invariants.

Code Block
bgColor#ccccff
public class Widget {
  private int total; // Declared private

  void add() {
    // ...
  }

  void remove() {
    // ...
  }

  public int getTotal () {
    // ...return total;
  }
}

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 (public mutable field)

This noncompliant code example shows a static, mutable hash map with public accessibility.

Code Block
bgColor#FFCCCC
public static final HashMap<Integer, String> hm = new HashMap<Integer, String>();

Compliant Solution (provide wrappers and reduce accessibility of mutable members)

Mutable data members that are static must always be declared private.

Code Block
bgColor#ccccff
private static final HashMap<Integer, String> hm = new HashMap<Integer, String>();

public static String getElement(int key) { 
  return hm.get(key);
}

Depending on the required functionality, wrapper methods may be used to retrieve an instance of the Hashmap or a contained value. This compliant solution adds a wrapper method to return the value of an element given its index in the Hashmap.

Exceptions

Wiki Markup
*EX1:* According to Sun's Code Conventions document \[[Conventions 09|AA. Java References#Conventions 09]\]:

...

Wiki Markup
*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|AA. Java References#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

Wiki Markup
\[[JLS 06|AA. Java References#JLS 06]\] Section 6.6, Access Control
\[[SCG 07|AA. Java References#SCG 07]\] Guideline 3-2 Define wrapper methods around modifiable internal state
\[[Long 05|AA. Java References#Long 05]\] Section 2.2, Public Fields
\[[Bloch 08|AA. Java References#Bloch 08]\] Items 13: Minimize the accessibility of classes and members; 14: In public classes, use accessor methods, not public fields

...