[java-shib-attribute] branch main updated: JSATTR-12 Filter service implementation is mutating via an immutably-defined API

Rod Widdowson rdw at steadingsoftware.com
Fri Apr 21 15:38:55 UTC 2023


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-shib-attribute.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-attribute.git;a=commit;h=96a500e61f069f72bead9e1224af6d558ccb36db

The following commit(s) were added to refs/heads/main by this push:
     new 96a500e61 JSATTR-12 Filter service implementation is mutating via an immutably-defined API
96a500e61 is described below

commit 96a500e61f069f72bead9e1224af6d558ccb36db
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Apr 21 16:36:51 2023 +0100

    JSATTR-12 Filter service implementation is mutating via an immutably-defined API
    
    https://shibboleth.atlassian.net/browse/JSATTR-12
    
    Fix this by copying the Immutable Map, adding to it and then
    setuffing it back in (modulo that the getter returns a Map and
    the setter takes a Collection).
    
    Fix the Context to always use immutable objects and a few tests
    which were performing similar abuse.
---
 .../attribute/filter/context/AttributeFilterContext.java | 10 ++++++----
 .../idp/attribute/filter/impl/AttributeFilterImpl.java   | 13 +++++++++----
 .../matcher/saml/impl/AttributeInMetadataMatcher.java    |  1 +
 .../attribute/filter/impl/AttributeFilterImplTest.java   | 16 ++++++++--------
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java b/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
index ddd8093da..d7d3b82fe 100644
--- a/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
+++ b/shib-attribute-filter-api/src/main/java/net/shibboleth/idp/attribute/filter/context/AttributeFilterContext.java
@@ -122,14 +122,16 @@ public final class AttributeFilterContext extends BaseContext {
 
     /** Constructor. */
     public AttributeFilterContext() {
-        prefilteredAttributes = new HashMap<>();
-        filteredAttributes = new HashMap<>();
+        prefilteredAttributes = CollectionSupport.emptyMap();
+        filteredAttributes = CollectionSupport.emptyMap();
         
         direction = Direction.OUTBOUND;
         
         // This is type-neutral but assumes the PRC is two levels up.
-        profileRequestContextLookupStrategy = new ParentContextLookup<>(ProfileRequestContext.class).compose(
-                afc -> afc.getParent());
+        final Function<AttributeFilterContext,ProfileRequestContext> prcls =
+                new ParentContextLookup<>(ProfileRequestContext.class).compose(afc -> afc.getParent());
+        assert prcls != null;
+        profileRequestContextLookupStrategy = prcls;
     }
     
     /**
diff --git a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
index 1e3790283..fa1c0f8db 100644
--- a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
+++ b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImpl.java
@@ -19,6 +19,7 @@ package net.shibboleth.idp.attribute.filter.impl;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -76,8 +77,10 @@ public class AttributeFilterImpl extends AbstractIdentifiableInitializableCompon
         setId(engineId);
         assert policies!=null;
         filterPolicies = CollectionSupport.copyToList(policies);
-        
-        metricContextLookupStrategy = new ChildContextLookup<>(MetricContext.class).compose(new RootContextLookup<>());
+        final Function<AttributeFilterContext,MetricContext> mcls
+            = new ChildContextLookup<>(MetricContext.class).compose(new RootContextLookup<>());
+        assert mcls!= null;
+        metricContextLookupStrategy = mcls;
     }
 
     /**
@@ -122,8 +125,9 @@ public class AttributeFilterImpl extends AbstractIdentifiableInitializableCompon
                 assert entry!=null;
                 final String key = entry.getKey();
                 assert key!=null;
-                final Collection<IdPAttributeValue> filteredAttributeValues =
+                final Collection<IdPAttributeValue> filteredAttributeValues = 
                         getFilteredValues(key, filterContext);
+                final Map<String, IdPAttribute> allFilteredAttributes = new HashMap<>(filterContext.getFilteredIdPAttributes());
                 if (null != filteredAttributeValues && !filteredAttributeValues.isEmpty()) {
                     final IdPAttribute filteredAttribute;
                     try {
@@ -134,8 +138,9 @@ public class AttributeFilterImpl extends AbstractIdentifiableInitializableCompon
                     final List<IdPAttributeValue> values = new ArrayList<>(filteredAttribute.getValues());
                     values.retainAll(filteredAttributeValues);
                     filteredAttribute.setValues(values);
-                    filterContext.getFilteredIdPAttributes().put(filteredAttribute.getId(), filteredAttribute);
+                    allFilteredAttributes.put(filteredAttribute.getId(), filteredAttribute);
                 }
+                filterContext.setFilteredIdPAttributes(allFilteredAttributes.values());
             }
         } finally {
             if (timerStarted) {
diff --git a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
index 536b60c37..8cdf93694 100644
--- a/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
+++ b/shib-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
@@ -463,6 +463,7 @@ public class AttributeInMetadataMatcher extends AbstractIdentifiableInitializabl
                 logPrefix = prefix;
             }
         }
+        assert prefix!=null;
         return prefix;
     }
 
diff --git a/shib-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImplTest.java b/shib-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImplTest.java
index 44670ff5f..7eaa644d0 100644
--- a/shib-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImplTest.java
+++ b/shib-attribute-filter-impl/src/test/java/net/shibboleth/idp/attribute/filter/impl/AttributeFilterImplTest.java
@@ -134,11 +134,11 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final IdPAttribute attribute2 = new IdPAttribute("attribute2");
         attribute2.setValues(CollectionSupport.listOf(new StringAttributeValue("a"), new StringAttributeValue("b")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute2.getId(), attribute2);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final AttributeFilterImpl filter = new AttributeFilterImpl("engine", CollectionSupport.singletonList(policy));
 
@@ -171,7 +171,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         attribute1Policy.initialize();
         policy.initialize();
@@ -203,7 +203,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         attribute2Policy.initialize();
         policy.initialize();
@@ -232,7 +232,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final AttributeFilterImpl filter = new AttributeFilterImpl("engine", CollectionSupport.singletonList(policy));
 
@@ -269,7 +269,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final AttributeFilterImpl filter = new AttributeFilterImpl("engine", CollectionSupport.singletonList(policy));
 
@@ -301,7 +301,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final AttributeFilterImpl filter = new AttributeFilterImpl("engine", CollectionSupport.singletonList(policy));
 
@@ -333,7 +333,7 @@ public class AttributeFilterImplTest {
 
         final IdPAttribute attribute1 = new IdPAttribute("attribute1");
         attribute1.setValues(CollectionSupport.listOf(new StringAttributeValue("one"), new StringAttributeValue("two")));
-        filterContext.getPrefilteredIdPAttributes().put(attribute1.getId(), attribute1);
+        filterContext.setPrefilteredIdPAttributes(CollectionSupport.singleton(attribute1));
 
         final AttributeFilterImpl filter = new AttributeFilterImpl("engine", CollectionSupport.singletonList(policy));
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list