readResolve() bug?

Question

The Effective Java says that readResolve works only if all fields are transient. Isn't this a bug? Why would the Java creators do a such thing?

-- update

Sorry, I mean the More Effective Java, see slide 30.

Answer 1

I think you misunderstood Josh Bloch. From the java spec:

For Serializable and Externalizable classes, the readResolve method allows a class to replace/resolve the object read from the stream before it is returned to the caller. By implementing the readResolve method, a class can directly control the types and instances of its own instances being deserialized.

As you can see, here is nothing similar to your statement.

As I know one of the most typical use case for readResolve is when you read serialized singleton (or any object for which you don't want to create a new instance, like manually implemented enumerations i.e. without enum keyword). In readResolve you can read fields (if they exist) and decide which of already created objects (singletons as usual) should be returned instead of creating new instance.

Answer 2

For completeness, slide 29 is:

Item 77: Pop Quiz: Is This Class a Singleton?

public class Elvis implements Serializable {
    public static final Elvis INSTANCE = new Elvis();
    private Elvis() { }
    private final String[] favoriteSongs =
        { "Hound Dog", "Heartbreak Hotel" };
    public void printFavorites() {
        System.out.println(Arrays.toString(favoriteSongs));
    }
    private Object readResolve() {
        return INSTANCE;
    }
}

slide 30 is:

Answer: Unfortunately Not

The first edition oversold the power of readResolve

> Elvis has a nontransient field (favoriteSongs)
> Cleverly crafted attack can save reference to deserialized
   Elvis instance when this field is deserialized

• See ElvisStealer for details (Item 77)

> readResolve works only if all fields are transient

Item 77 is:

Item 77: For instance control, prefer enum types to readResolve

...

If the Elvis class is made to implement Serializable, the following readResolve method suffices to guarantee the singleton property:

// readResolve for instance control - you can do better!
private Object readResolve() {
  // Return the one true Elvis and let the garbage collector
  // take care of the Elvis impersonator.
  return INSTANCE;
}

This method ignores the deserialized object, returning the distinguished Elvis instance that was created when the class was initialized. Therefore, the serialized form of an Elvis instance need not contain any real data; all instance fields should be declared transient. In fact, if you depend on readResolve for instance control, all instance fields with object reference types must be declared transient. Otherwise, it is possible for a determined attacker to secure a reference to the deserialized object before its readResolve method is run, using a technique that is vaguely similar to the MutablePeriod attack in Item 76.

The attack is a bit complicated, but the underlying idea is simple. If a singleton contains a nontransient object reference field, the contents of this field will be deserialized before the singleton’s readResolve method is run. This allows a carefully crafted stream to “steal” a reference to the originally deserialized singleton at the time the contents of the object reference field are deserialized.

It's not a case of only working if all the fields are transient, it's that it's only safe if all Object references are transient. The slide you reference goes further and says is any fields are non-transient an attacker can grab a reference to the deserialized Object.

As to why: one can argue that Josh Bloch's use of readResolve() for a singleton was an unintended use of the API, one the creators didn't envision when they made it. You could also argue it's simply an unforeseen consequence. I don't think however it was a deliberate weakness.