6

ImmutableList's documentation says:

Although this class is not final, it cannot be subclassed as it has no public or protected constructors.

I know it's a little bit far-fetched, but it's possible to create a subclass of ImmutableList in com.google.common.collect package (since its constructor is not private, but package private) which is mutable. From that point, no one who gets a reference to an ImmutableList can be sure that it really is immutable. Does this not break the purpose of ImmutableList?

dimo414
  • 47,227
  • 18
  • 148
  • 244
Katona
  • 4,816
  • 23
  • 27
  • 3
    How would the subclass call the parentclass constructor, if there's none visible? – Kayaman Jul 30 '13 at 09:05
  • 1
    @Kayaman the constructor is package private so you can access it from within the package. – assylias Jul 30 '13 at 09:13
  • 1
    @Kayaman: it's constructor is package private, which is accessible if you declare the subclass in ImmutableList's package which is com.google.common.collect – Katona Jul 30 '13 at 09:14
  • 17
    Even if the constructor was private, the list could be mutated using reflection. Immutability is not a security mechanism. It's an OO design way to avoid shooting yourself in the foot. Of course, breaking all the rules by using reflection or defining a subclass of something which should not be, in a package that you don't own, is a sure way to shoot yourself in the foot. Just don't do it. – JB Nizet Jul 30 '13 at 09:58
  • 3
    @JBNizet: good point, but accessing fields by reflection could be prevented by SecurityManagers. On the other hand, packages can also be defended with sealing, but it's not sealed. – Katona Jul 30 '13 at 11:19
  • What is this question about? I'm thinking there is no question in there: only a try to start a debate. You want to make sure that you don't get anything else than an actual immutable list? Check that your classpath doesn't contain `com.google.common.collect` besides Guava. ImmutableList is a contract. People have to follow the contract. It's the same with interfaces. You can say that a method may not throw a specific exception yet any implementation allows you to do so. So I clearly don't see the point of this "question". If you don't follow the contract, you're responsible for the mess. – Olivier Grégoire Jul 30 '13 at 23:43
  • 2
    the problem I mentioned could be prevented by either making the class final, or sealing the package, so, I don't understand why isn't this done so? – Katona Jul 31 '13 at 06:16
  • 2
    2 things: first this is awfully close to http://code.google.com/p/guava-libraries/issues/detail?id=1485. Then regarding the "final", take a look at ImmutableList. It is abstract. This means it is intended to be subclassed. Just not by anyone. This allows some optimizations based on the number of elements a list contains. For instance there is a case where the size is zero: the same list is always returned for this case. Another case is where the size is one: a singleton list is available (but is inefficient, apparently). And the rest is a more regular ImmutableList implementation. – Olivier Grégoire Jul 31 '13 at 09:36

2 Answers2

5

At the end of the day, Guava is just a bunch of binary you're executing. You can do anything you want with it, including violate the contracts that code provides.

You can:

  • Run a custom JVM that breaks Guava
  • Run a separate application and manipulate the JVM's physical memory and break Guava
  • Manipulate the Guava bytecode and break Guava
  • Configure the classloaders to load other bytecode and break Guava
  • Make calls to sun.misc.Unsafe to break Guava
  • Use reflection to break Guava
  • Abuse the package hierarchy and break Guava

Just to name a few. None of these are novel, unique to Guava, or unique to Java. It's your machine, you should be able to do these things. Of course what you can do and what you should do are very different things. Abusing Java in any of these ways will lead to problems for you and anyone else who tries to run your code.

The Java type system, including visibility restrictions, is not a police officer. It exists to help you write quality code that's easy to work with. Guava takes advantage of the documented behavior of Java to give you even more tools that enable you to write more quality code that's even easier to work with. If you choose to break the tools that's your prerogative. But don't expect anyone to want to work with your code.

To specifically address this point:

it's possible to create a subclass of ImmutableList in com.google.common.collect package (since its constructor is not private, but package private) which is mutable.

You can, without very much work, manipulate a private-constructor class into being mutable in much the same way. Abusing the package hierarchy, as I mentioned above, is just one of many things you simply should not do. If you don't trust a piece of third-party code to be well-behaved in this regard, you should not use it.

Community
  • 1
  • 1
dimo414
  • 47,227
  • 18
  • 148
  • 244
  • What I pointed out in the question was a completely valid way of creating a mutable `ImmutableList`, on any standard compliant JVM, not mine. Why is this interesting? Let's say you create java library which accepts an `ImmutableList` and use it with the assumption that's really immutable, but it may not be, so your library may be compromised. But I guess, guava made the packages sealed, so the problem doesn't exist anymore. – Katona Jul 28 '15 at 15:11
  • Your library is only compromised if you run code that maliciously breaks the invariants of your code. This is true, again, not just of Guava or Java, but of any language and environment. It is not possible, short of very restricted sandbox environments, to prevent the behavior you're describing. Any "immutable" code can be made mutable through malice. – dimo414 Jul 28 '15 at 16:32
  • I'll also mention some of the above ways Java invariants can be violated are controllable with [security managers](https://docs.oracle.com/javase/tutorial/essential/environment/security.html). Despite the name, it's still necessary to trust the code you're executing, but they will catch some more-egregious behavior. – dimo414 Aug 25 '15 at 00:16
3

Even if you extend ImmutableList you will not be able to make it mutable because all its mutator methods are final, like this one:

  public final void add(int index, E element) {
    throw new UnsupportedOperationException();
  }

and its iterator returns UnmodifiableIterator whose remove method is final too

zEro
  • 1,255
  • 14
  • 24
Evgeniy Dorofeev
  • 133,369
  • 30
  • 199
  • 275
  • 2
    sure I can make it mutable, by declaring another, let's say doAdd(E element) method, which will add the element to the list – Katona Jul 30 '13 at 11:08
  • 1
    -1 It's only necessary to override the other methods like `get(int)` to let the list appear to be changed (just suppose that `get` returns a random object on every access). – Philipp Wendler Jul 30 '13 at 12:51
  • @Phillipp, in that case the list is not actually modified. In your own words it would only appear to be changed. But, yes, that would be an obscure but possible way of breaking the contract. – Kevin Welker Jul 30 '13 at 15:10
  • 1
    @Katona, your point is that when you *receive* an ImmutableList (in the question). This means that you receive an ImmutableList, not a MutableChildOfImmutableList that has the doAdd method. The doAdd method is inaccessible unless you *know* it's a MutableChildOfImmutableList. In that case, as the receiver I cannot add anything to it. – Olivier Grégoire Jul 30 '13 at 15:17
  • 1
    @ogregoire well, my point is that there is no point in receiving an ImmutableList, because there is no guarantee that the actual implementation is really immutable, you must create your defensive copy. – Katona Jul 30 '13 at 15:23
  • @KevinWelker Well, the whole point about immutability is exactly that it appears unchanged to clients. Whether the fields of the object actually change is irrelevant (consider the hashCode field of String, which changes throught object lifetime, although String is immutable). – Philipp Wendler Jul 31 '13 at 07:00