0

I'm developing a new method for TestClass called setList. The method takes a List of String parameter and sets a member variable of TestClass. Assume the list is large, so it's impractical to copy it.

Now a user of my class decides to cast an ArrayList of Integer as an untyped List and passes it to my method. Compiler doesn't complain. At runtime, there isn't any error until the client attempts to retrieve an element from the list.

What's the best practice for my method to protect the class from being corrupted by this user input? I thought of checking if list is empty and then checking type of all elements in list, but that seems hacky. Even more so if the elements are Generics themselves, etc.

@Test
public void test() {
    Map<String, Object> map = new HashMap<>();
    List input =  new ArrayList<>(Arrays.asList(1, 2, 3));
    input.add("12345");
    map.put("key", input);
    TestClass testClass = new TestClass();
    testClass.setList((List) map.get("key"));
    List<String> list = testClass.getList();
    String element = list.get(0);
}
 
public static class TestClass {
    private List<String> list;
 
    public void setList(List<String> list) {
        this.list = list;
    }
 
    public List<String> getList() {
        return list;
    }
}
Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
user219694
  • 33
  • 4
  • 3
    If some code running in the same JVM as yours is intentionally malicious or utterly idiotic, then they will respectively succeed in hacking the system / causing loads of bugs. There is nothing you're going to do about it. No matter how hard you try. Thus, the answer is a very simple and nice: Don't worry about it. – rzwitserloot Jan 23 '23 at 02:30
  • And typically, you would do defensive copying when setting and returning an immutable view in the getter - but note that one could also just pass a `List` not adhering to the `List` specification if they really wanted to. – dan1st Jan 23 '23 at 06:20
  • The compiler generates an error ("incompatible types") if you pass a method expecting List an argument of List. But it does not generate an error if you pass the same method a List. Why is this? This is a good question. – ktm5124 Jan 23 '23 at 21:17

1 Answers1

1

Due to type erasure, you can't prevent someone from circumventing type checking by using a raw list, but you can force the caller to use a particular implementation whose type cannot be circumvented:

public class StringList extends ArrayList<String> {
    public StringList(int initialCapacity) {
        super(initialCapacity);
    }

    public StringList() {
        super();
    }

    public StringList(@NotNull Collection<? extends String> c) {
        super(c);
    }

    @Override
    public String set(int index, String element) {
        return super.set(index, element);
    }

    @Override
    public boolean add(String s) {
        return super.add(s);
    }

    @Override
    public void add(int index, String element) {
        super.add(index, element);
    }

    @Override
    public boolean addAll(Collection<? extends String> c) {
        return super.addAll(c);
    }

    @Override
    public boolean addAll(int index, Collection<? extends String> c) {
        return super.addAll(index, c);
    }

}

And use that instead of List<String>:

public static class TestClass {
    private StringList list;
 
    public void setList(StringList list) {
        this.list = list;
    }
 
    public StringList getList() {
        return list;
    }
}
Bohemian
  • 412,405
  • 93
  • 575
  • 722
  • 1
    Subclassing an implementation class, to override every method and hoping you didn’t forget one or the maintainer of the class won’t add new methods in the next version, clearly is an anti-pattern. It’s not as if this doesn’t [backfire in real life](https://stackoverflow.com/a/26841569/2711488). And it’s [quite easy to find subversions for your class](https://ideone.com/dDOy0n). You can try to fix them all, if you like playing whack-a-mole. The clean solution is to use delegation instead of inheritance. Which has been done already, `Collections.checkedList(new ArrayList<>(), String.class);`… – Holger Jan 24 '23 at 08:34