1

I have been working to upgrade my Java code baseline so that it follows good security practices and have run into an issue related to generics. Say you have the following:

public class SomeClass<T>
{
    private T value;
    public T getValue()
    {
        return value;
    }

    public void setValue(T value)
    {
        this.value = value;
    }
}

I have not found a good answer on how to edit these methods so that value does not leak like it does in this example class for a generic object that does not implement Clonable and in some cases has no default constructor.

  • I think you're onto a loser here because of the way Java is structured. The Cloneable interface is a bizarre old joke, so you can't rely on that, and there's nothing in Java to allow you to declare a generic object read-only. On the plus side, the client code chooses what classes are being contained by SomeClass, so the client code can choose to create immutable versions of objects before storing them in SomeClass. Not perfect, obviously, but I don't think you can guarantee the airtight result you want. – Bobulous Nov 09 '13 at 20:26
  • Yes, unfortunately (though whether it is truly unfortunate is up for debate), C++ does this perfectly, but Java was never meant to do this. – Paul Draper Nov 09 '13 at 20:31

4 Answers4

1

As I understand it, you want to make sure that nothing outside SomeClass can mutate the object value.

In C++, you could returns a const reference (avoid copying altogether), but Java does not have that. So let's look at copying...

First, know that some objects cannot be copied. For example, stream, gui elements, etc. Thus, trying to copy all objects is a hopeless endeavor from the start.

But what about objects that are copiable?

In Java, you cannot call the copy constructor (or any other constructor) of a generic (Calling constructor of a generic type).

There is the Cloneable interface, but that is really nothing more than a promise that clone works; it does not actually expose clone publically. Thus, for generics, you have to use reflection, as shown here.

Unfortunately, there is no good solution. The only viable one (except for changing the purpose or semantics of your class) is to use the clone method as shown in the link above, and realize that some objects cannot be copied.

Ultimately, the best thing to do is find a solution that does not require this. Make a (non-generic) read-only wrapper class that exposes the non-mutating methods. Or stipulate in documentation that mutating methods must not be called.

Community
  • 1
  • 1
Paul Draper
  • 78,542
  • 46
  • 206
  • 285
0

I can see three approaches:

  1. Make copies. This of course would only work with types can can be copied (and that you know how to copy).

  2. Only support immutable types.

  3. Remove getValue(). Instead, provide methods that operate directly on this.value without exposing it outside the class. In this approach, setValue() can still be problematic (you need to make sure that the caller does not hold on to the object reference after calling setValue()).

If T can be arbitrary type that you have no control over, then options 1 and 2 won't be suitable.

NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • I appreciate the feedback. I think the right approach for the code I am changing is option 2. I do have control of the types so making the types I want to support that are not primitive wrapper objects immutable will not take that much work. – Chris Picard Nov 18 '13 at 17:07
0

I believe that i undestand you ... If you want to restrict a generic type you should use extends keyword that in generic type is not equals to general class. If you use only the class how implements Clonable are able to instantiate this class. One example:

  public class Stack {


    public static void main(String[] args) {
        SomeClass<Animal> sc = new SomeClass<>(); //This generate an error because doesnt implements Clonable interface
        SomeClass<Person> sc1 = new SomeClass<>();
    }

}
class SomeClass<T extends Comparable> //Note that extends means implements or the common extends
{
    private T value;
    public T getValue()
    {
        return value;
    }

    public void setValue(T value)
    {
        this.value = value;
    }
}

class Person implements Comparable<Person>{
    @Override
    public int compareTo(Person p){
        return 0;
    }
}
class Animal {

}

I wish i helped you. :)

oclaril
  • 33
  • 1
  • 9
0

An object whose state is encapsulated in a mutable object should generally never expose to the outside world any reference to that object, and should avoid giving the outside world a reference to any mutable object (even a copy) which claims to encapsulate its state. The problem is that given code:

Foo foo = myEntity1.getFoo();
foo.bar = 23;
myEntity2.setFoo(foo);
foo.bar = 47;
myEntity3.setFoo(foo);

there is no clear indication whether or how the change to foo.bar would affect the various entities. If the code had instead been:

Foo foo = myEntity1.getFoo();
foo = foo.withBar(23); // makes a new instance which is like foo, but where bar==23
myEntity2.setFoo(foo);
foo = foo.withBar(47); // makes a new instance which is like foo, but where bar==47
myEntity3.setFoo(foo);

it would be very clear that the bar property of myEntity1's foo will be unaffected, that of myEntity2 will be 23, and that of myEntity3 will be 47. If foo is a mutable class, the pattern should be:

Foo foo = new Foo();
myEntity1.writeTo(foo); // Copy properties from myEntity1 to the supplied instance
foo.bar = 23;
myEntity2.readFrom(foo); // Copy properties from the supplied instance to myEntity2
foo.bar = 47;
myEntity2.readFrom(foo); // Copy properties from the supplied instance to myEntity3

Here, myEntity1 isn't giving the caller an object, but is instead copying data to an object supplied by the caller. Consequently, it's much clearer that the caller shouldn't expect the writes to foo.bar to affect the entities directly, but merely change what will be written in subsequent readFrom calls.

supercat
  • 77,689
  • 9
  • 166
  • 211