SEC-1248: @PostFilter on Collection may inadvertently modify the Collection #1498

Closed
spring-issuemaster opened this Issue Sep 21, 2009 · 2 comments

1 participant

@spring-issuemaster

Peter Mularien (Migrated from SEC-1248) said:

Just ran into this today. The implementation of Collection @PostFilter annotation may inadvertently modify the Collection returned from the annotated method. The error comes from 2 bits of code in DefaultMethodSecurityExpressionHandler. The first is where the returned object (Collection) is assigned here:

    if (filterTarget instanceof Collection) {
        Collection collection = (Collection)filterTarget;

After the retainList is calculated, the following code is executed:

        collection.clear();
        collection.addAll(retainList);

This code is modifying the original, returned object and not a copy. This is potentially dangerous!

An easy way to verify this behavior is to try to add @PostFilter to a method that returns a java.util.Collections.unmodifiableCollection (or List) - the processing of the PostFilter annotation will throw an UnsupportedOperationException because the clear() method is being called on the original Collection.

I can see that this is potentially problematic, because you'd need to create a new Collection of the correct type, etc., so it isn't clear what the proper solution to this is. Maybe it's not a bug at all and could simply be resolved through documentation. I was at least surprised by it, though!

@spring-issuemaster

Luke Taylor said:

No, I don't think this is a bug (it is how Collection filtering has always worked, even in Acegi days). As you say, it's entirely possible that a completely different Collection implementation is being used that isn't available to us. I don't think there's really anything wrong with requiring that only mutable Collections can be filtered.

@spring-issuemaster

Andras Kerekes said:

But a quick Collection.isEmpty() (and if it is true returning the original collection) would cover some of the cases.

E.g. I'm using Spring Data JPA, which returns a Collection.emptyList() in case the query has no results. Now I need to check it manually and return a new ArrayList(0) in this case.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 RC1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment