Spring Security
  1. Spring Security
  2. SEC-1248

@PostFilter on Collection may inadvertently modify the Collection

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 3.0.0 M2
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Core
    • Labels:
      None

      Description

      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!

        Activity

        Hide
        Luke Taylor added a comment -

        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.

        Show
        Luke Taylor added a comment - 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.
        Hide
        Andras Kerekes added a comment -

        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.

        Show
        Andras Kerekes added a comment - 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.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Peter Mularien
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: