4

In Josh Bloch's excellent book Effective Java under Item 39 he says:

"[D]efensive copies are made before checking the validity of the parameters, and the validity check is performed on the copies rather than on the originals."

The example given is as follows:

public Period(Date start, Date end) {
   this.start = new Date(start.getTime());
   this.end = new Date(end.getTime());

   if(this.start.compareTo(this.end) > 0)
      throw new IllegalArgumentException("...");
   }
}

The problem with doing the validity checks after the defensive copy is that an invalid parameter can cause the creation of the copy to fail. For example, the class above will throw a NullPointerException if you pass it a null for start or end.

If I move the validity check before the defensive copy, I am vulnerable to the time-of-check/time-of-use attack that Bloch cites as the reason for doing the defensive copy first.

My question is what is a way around this? I can't believe I'm the first person to see this problem in a well read book (though the errata for the book says nothing about it), so maybe I'm just missing something.

4 Answers 4

1

As others said, you check for null before copying the parameters.

If I move the validity check before the defensive copy, I am vulnerable to the time-of-check/time-of-use attack that Bloch cites as the reason for doing the defensive copy first.

No, a hacker won't be able to change a reference to an actual instance to a null reference, or vice-versa. The copying is done to avoid changes to the internal state of the arguments from another thread.

4
  • I think this gets at the part I was missing but let me ask a clarifying question. If I call Period p = new Period(someStart, someEnd) with someStart as a non-null value and then I set someStart=null after the null check in the constructor and before the defensive copy, it won't throw a NullPointerException when it gets to the start.getTime() call?
    – stand
    Sep 2, 2011 at 0:41
  • someStart is just a reference to an object (let's say the object at Ox123). When you call the constructor with argument someStart, you are pointing its start parameter to the actual object 0x123. It does not matter if someone sets someStart = null in some other thread; the constructor just knows about the object 0x123. What could be dangerous is if someStart.changeState(5) is called from another thread, which really does change the state of object 0x123.
    – toto2
    Sep 2, 2011 at 0:50
  • Java always passes parameters by copying them -- "call by value". An integer, say, is copied directly. For an object reference, the address in the reference is copied. Once you have that address in your method's parameter list, the object cannot go away. If the caller uses an async operation to clear his copy of the reference, it has no effect on what you copied. And even if Java did do "call by reference" and the pointer could be changed, you'd only need to make a local copy of the pointer first, before checking it for null.
    – Hot Licks
    Sep 2, 2011 at 1:09
  • I see what you're saying that null checks can be done safely, thanks. I'm also trying to think of the more general case where the act of making a defensive copy might throw an exception because of an invalid input parameter (not necessarily just a null). I admit I can't think of any examples though so I think it's not an issue.
    – stand
    Sep 2, 2011 at 4:35
1

Defensive copy is a good strategy BUT it has its prerequisites... One is having something that you can really copy...

IMHO this means the check for null has to be done BEFORE the copy and if it fails throw an appropriate exception...

1

You don't need to be "defensive" about testing the validity of a pointer. The pointer can't change to a null or a different object, it's only the contents of what it points to that can change.

When making "defensive copies" you need to use a "stick" of sorts to test the terrain before you step on it -- check every pointer for validity before you use it, limit check bounds values, etc. It's not hard, just tedious, and it requires a bit of a mind for details.

[Also, there is no great harm in simply allowing the NullPointerExceptions to "bubble up".]

0

I don't understand the problem. The exception framework already accounts for invalid arguments.

1
  • There might be cases where you would build an object even when you have null arguments, with some default values. But I admit that would be poor design.
    – toto2
    Sep 2, 2011 at 0:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.