n-y-z-o / nyzoVerifier

Verifier for the Nyzo cryptocurrency
The Unlicense
73 stars 42 forks source link

Private data returned by public functions #8

Closed glemercier closed 4 years ago

glemercier commented 5 years ago

In the Node class, private variables identifier and ipAddress are returned by reference in their getter functions:

https://github.com/n-y-z-o/nyzoVerifier/blob/master/src/main/java/co/nyzo/verifier/Node.java#L31

public byte[] getIdentifier() {
  return identifier;
}
...
public byte[] getIpAddress() {
  return ipAddress;
}

As a result these private variables can be altered by calling functions after getting access to the private variable reference. While this is a minor issue that does not translate in any undefined behavior from wherever it's called in the rest of the code, it might be good to fix it for the sake of best practices and to prevent any potential future misbehavior.

This article describes the best practice in this case, which recommends copying the data to return into a new array as follows:

public byte[] getIdentifier() {
    byte[] copy = new byte[this.identifier.length];
    System.arraycopy(this.identifier, 0, copy, 0, copy.length);
    return copy;
}
n-y-z-o commented 4 years ago

Returning a copy of these arrays would adversely impact performance without improving the maintainability of the code in any appreciable way. Encapsulation is important in object-oriented programming, but this is a case where any modification of the returned arrays would be such an obvious mistake that it would simply not be done by a competent programmer.