[java-shib-attribute] branch main updated: Fix up null component handling.

Scott Cantor cantor.2 at osu.edu
Mon Nov 28 19:33:10 UTC 2022


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

scantor 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=73555073c2a432b245d1ddce60086f49ed079ef7

The following commit(s) were added to refs/heads/main by this push:
     new 73555073c Fix up null component handling.
73555073c is described below

commit 73555073c2a432b245d1ddce60086f49ed079ef7
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 28 14:33:07 2022 -0500

    Fix up null component handling.
---
 .../filter/context/AttributeFilterContext.java     |  9 ++-
 .../filter/spring/AttributeFilterFailFastTest.java | 13 +++--
 .../context/AttributeResolutionContext.java        |  4 +-
 .../impl/AttributeResolverServiceGaugeSet.java     | 68 ++++++++++++----------
 .../resolver/spring/AttributeMapperTest.java       |  6 +-
 .../resolver/spring/AttributeResolverTest.java     |  5 +-
 .../failfast/AttributeResolverFailFastTest.java    | 16 ++---
 .../impl/AttributeMappingNodeProcessor.java        |  7 ++-
 8 files changed, 68 insertions(+), 60 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 dd6beec02..36f396a31 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
@@ -46,6 +46,7 @@ import net.shibboleth.shared.annotation.constraint.Unmodifiable;
 import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 /** A context supplying input to the {@link net.shibboleth.idp.attribute.filter.AttributeFilter} interface. */
@@ -551,13 +552,11 @@ public final class AttributeFilterContext extends BaseContext {
 
         final Logger log = LoggerFactory.getLogger(AttributeFilterContext.class);
         try (final ServiceableComponent<AttributeFilter> component = attributeFilterService.getServiceableComponent()) {
-            if (null == component) {
-                log.error("Error filtering attributes: Invalid Attribute filter configuration");
-            } else {
-                component.getComponent().filterAttributes(this);
-            }
+            component.getComponent().filterAttributes(this);
         } catch (final AttributeFilterException e) {
             log.error("Error filtering attributes", e);
+        } catch (final ServiceException e) {
+            log.error("Invalid AttributeFilter configuration", e);
         }
     }
 
diff --git a/shib-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/AttributeFilterFailFastTest.java b/shib-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/AttributeFilterFailFastTest.java
index 004f27dff..6e733b367 100644
--- a/shib-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/AttributeFilterFailFastTest.java
+++ b/shib-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/AttributeFilterFailFastTest.java
@@ -17,8 +17,7 @@
 
 package net.shibboleth.idp.attribute.filter.spring;
 
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
+import static org.testng.Assert.*;
 
 import java.io.IOException;
 
@@ -30,7 +29,7 @@ import org.testng.annotations.Test;
 import net.shibboleth.idp.attribute.filter.AttributeFilter;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.service.ReloadableService;
-import net.shibboleth.shared.service.ServiceableComponent;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.spring.testing.AbstractFailFastTest;
 
 /**
@@ -69,8 +68,12 @@ public class AttributeFilterFailFastTest extends AbstractFailFastTest {
             return;
         }
         assertNotNull(service);
-        final ServiceableComponent<AttributeFilter> component = service.getServiceableComponent();
-        assertNull(component);
+        try {
+            service.getServiceableComponent();
+            fail("Should have failed");
+        } catch (final ServiceException e) {
+            // OK
+        }
     }
     private void badFilter(final Boolean failFast) throws IOException {
         badFilter(failFast, "attributeFilterBad.xml");
diff --git a/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java b/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
index 1b3c25a5a..c10fbc509 100644
--- a/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
+++ b/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
@@ -337,8 +337,10 @@ public final class AttributeResolutionContext extends BaseContext {
                 = attributeResolverService.getServiceableComponent()) {
             final AttributeResolver attributeResolver = component.getComponent();
             attributeResolver.resolveAttributes(this);
-        } catch (final ResolutionException|ServiceException e) {
+        } catch (final ResolutionException e) {
             log.error("Error resolving attributes", e);
+        } catch (final ServiceException e) {
+            log.error("Invalid AttributeResolver configuration", e);
         }
     }
     
diff --git a/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java b/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
index e6142089f..5e158caeb 100644
--- a/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
+++ b/shib-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
@@ -37,6 +37,7 @@ import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.service.ReloadableServiceGaugeSet;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 /**
@@ -65,25 +66,25 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet<
                         final Map<String,Instant> mapBuilder = new HashMap<>();
                         try (final ServiceableComponent<AttributeResolver> component =
                                 getService().getServiceableComponent()) {
-                            if (component != null) {
-                                final Object resolver = component.getComponent();
-                                if (resolver instanceof AttributeResolverImpl) {
-                                    final Collection<DataConnector> connectors =
-                                            ((AttributeResolverImpl) resolver).getDataConnectors().values();
-                                    for (final DataConnector connector: connectors) {
-                                        if (connector.getLastSuccess() != null) {
-                                            mapBuilder.put(connector.getId(), connector.getLastSuccess());
-                                        }
+                            final Object resolver = component.getComponent();
+                            if (resolver instanceof AttributeResolverImpl) {
+                                final Collection<DataConnector> connectors =
+                                        ((AttributeResolverImpl) resolver).getDataConnectors().values();
+                                for (final DataConnector connector: connectors) {
+                                    if (connector.getLastSuccess() != null) {
+                                        mapBuilder.put(connector.getId(), connector.getLastSuccess());
                                     }
-                                } else if (resolver instanceof AttributeResolver) {
-                                   log.debug("{}: Cannot get Data Connector success " +
-                                           " information from unsupported class type {}",
-                                           getLogPrefix(), resolver.getClass());
-                                } else {
-                                    log.warn("{}: Injected Service was not for an AttributeResolver ({})",
-                                            getLogPrefix(), resolver.getClass());
                                 }
+                            } else if (resolver instanceof AttributeResolver) {
+                               log.debug("{}: Cannot get Data Connector success " +
+                                       " information from unsupported class type {}",
+                                       getLogPrefix(), resolver.getClass());
+                            } else {
+                                log.warn("{}: Injected Service was not for an AttributeResolver ({})",
+                                        getLogPrefix(), resolver.getClass());
                             }
+                        } catch (final ServiceException e) {
+                            // Nothing to do.
                         }
                         return Map.copyOf(mapBuilder);
                     }
@@ -96,25 +97,25 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet<
                         final Map<String,Instant> mapBuilder = new HashMap<>();
                         try (final ServiceableComponent<AttributeResolver> component =
                                 getService().getServiceableComponent()) {
-                            if (component != null) {
-                                final Object resolver = component.getComponent();
-                                if (resolver instanceof AttributeResolverImpl) {
-                                    final Collection<DataConnector> connectors =
-                                            ((AttributeResolverImpl) resolver).getDataConnectors().values();
-                                    for (final DataConnector connector: connectors) {
-                                        if (connector.getLastFail() != null) {
-                                            mapBuilder.put(connector.getId(), connector.getLastFail());
-                                        }
+                            final Object resolver = component.getComponent();
+                            if (resolver instanceof AttributeResolverImpl) {
+                                final Collection<DataConnector> connectors =
+                                        ((AttributeResolverImpl) resolver).getDataConnectors().values();
+                                for (final DataConnector connector: connectors) {
+                                    if (connector.getLastFail() != null) {
+                                        mapBuilder.put(connector.getId(), connector.getLastFail());
                                     }
-                                } else if (resolver instanceof AttributeResolver) {
-                                   log.debug("{}: Cannot get Data Connector failure " +
-                                           " information from unsupported class type {}",
-                                           getLogPrefix(), resolver.getClass());
-                                } else {
-                                    log.warn("{}: Injected Service was not for an AttributeResolver ({})",
-                                            getLogPrefix(), resolver.getClass());
                                 }
+                            } else if (resolver instanceof AttributeResolver) {
+                               log.debug("{}: Cannot get Data Connector failure " +
+                                       " information from unsupported class type {}",
+                                       getLogPrefix(), resolver.getClass());
+                            } else {
+                                log.warn("{}: Injected Service was not for an AttributeResolver ({})",
+                                        getLogPrefix(), resolver.getClass());
                             }
+                        } catch (final ServiceException e) {
+                            // Nothing to do.
                         }
                         return Map.copyOf(mapBuilder);
                     }
@@ -137,6 +138,9 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet<
                         getLogPrefix(), component.getClass());
                 throw new ComponentInitializationException("Injected service was not for an AttributeResolver");
             }
+        } catch (final ServiceException e) {
+            log.debug("{} : Injected service has not initialized sucessfully yet. Skipping type test",
+                    getLogPrefix(), e);
         }
     }
 
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeMapperTest.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeMapperTest.java
index c061d2e47..25bd7f907 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeMapperTest.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeMapperTest.java
@@ -68,7 +68,7 @@ public class AttributeMapperTest extends OpenSAMLInitBaseTestCase {
 
     @Test public void mapper() throws ComponentInitializationException, ServiceException, ResolutionException {
 
-        GenericApplicationContext context = new GenericApplicationContext();
+        final GenericApplicationContext context = new GenericApplicationContext();
         setTestContext(context);
         context.setDisplayName("ApplicationContext: " + AttributeMapperTest.class);
 
@@ -80,7 +80,7 @@ public class AttributeMapperTest extends OpenSAMLInitBaseTestCase {
 
         context.getBeanFactory().setConversionService(service.getObject());
         
-        SchemaTypeAwareXMLBeanDefinitionReader beanDefinitionReader =
+        final SchemaTypeAwareXMLBeanDefinitionReader beanDefinitionReader =
                 new SchemaTypeAwareXMLBeanDefinitionReader(context);
 
         beanDefinitionReader.loadBeanDefinitions("net/shibboleth/idp/attribute/resolver/spring/mapperTest.xml");
@@ -89,8 +89,6 @@ public class AttributeMapperTest extends OpenSAMLInitBaseTestCase {
         final ReloadableService<AttributeTranscoderRegistry> transcoderRegistry = context.getBean(ReloadableService.class);
 
         try (final ServiceableComponent<AttributeTranscoderRegistry> serviceableComponent = transcoderRegistry.getServiceableComponent()){
-            assert serviceableComponent != null;
-            
             final IdPAttribute idpattr = new IdPAttribute("eduPersonScopedAffiliation");
             
             Collection<TranscodingRule> rulesets = serviceableComponent.getComponent().getTranscodingRules(
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeResolverTest.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeResolverTest.java
index 4e1f758c0..ef8c42523 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeResolverTest.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/AttributeResolverTest.java
@@ -217,7 +217,6 @@ public class AttributeResolverTest extends OpenSAMLInitBaseTestCase {
                 TestSources.createResolutionContext("PETER_THE_PRINCIPAL", "issuer", "recipient");
 
         try (final ServiceableComponent<AttributeResolver> serviceableComponent = attributeResolverService.getServiceableComponent()) {
-            assert serviceableComponent != null;
             final AttributeResolver resolver = serviceableComponent.getComponent();
             assertEquals(resolver.getId(), "Shibboleth.Resolver");
             resolver.resolveAttributes(resolutionContext);
@@ -329,7 +328,6 @@ public class AttributeResolverTest extends OpenSAMLInitBaseTestCase {
                 TestSources.createResolutionContext("PETER_THE_PRINCIPAL", "issuer", "recipient");
 
         try (final ServiceableComponent<AttributeResolver> serviceableComponent = attributeResolverService.getServiceableComponent()) {
-            assert serviceableComponent != null;
             final AttributeResolver resolver = serviceableComponent.getComponent();
             assertEquals(resolver.getId(), "Shibboleth.Resolver");
             resolver.resolveAttributes(resolutionContext);
@@ -385,7 +383,7 @@ public class AttributeResolverTest extends OpenSAMLInitBaseTestCase {
         context.refresh();
 
         final AttributeResolver resolver = BaseAttributeDefinitionParserTest.getResolver(context);
-        AttributeResolutionContext resolutionContext =
+        final AttributeResolutionContext resolutionContext =
                 TestSources.createResolutionContext("PETER", "issuer", "recipient");
 
         resolver.resolveAttributes(resolutionContext);
@@ -461,7 +459,6 @@ public class AttributeResolverTest extends OpenSAMLInitBaseTestCase {
                 TestSources.createResolutionContext("PETER_THE_PRINCIPAL", "issuer", "recipient");
 
         try (final ServiceableComponent<AttributeResolver> serviceableComponent = attributeResolverService.getServiceableComponent()) {
-            assert serviceableComponent != null;
             final AttributeResolver resolver = serviceableComponent.getComponent();
             assertEquals(resolver.getId(), "MultiFileResolver");
             resolver.resolveAttributes(resolutionContext);
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/failfast/AttributeResolverFailFastTest.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/failfast/AttributeResolverFailFastTest.java
index c104da3b5..bf87e71cc 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/failfast/AttributeResolverFailFastTest.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/failfast/AttributeResolverFailFastTest.java
@@ -17,9 +17,7 @@
 
 package net.shibboleth.idp.attribute.resolver.spring.failfast;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
+import static org.testng.Assert.*;
 
 import java.io.IOException;
 import java.util.Optional;
@@ -39,6 +37,7 @@ import net.shibboleth.idp.attribute.resolver.AttributeResolver;
 import net.shibboleth.idp.attribute.resolver.spring.dc.rdbms.impl.RDBMSDataConnectorParserTest;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 import net.shibboleth.shared.testing.DatabaseTestingSupport;
 import net.shibboleth.spring.testing.AbstractFailFastTest;
@@ -93,8 +92,6 @@ public class AttributeResolverFailFastTest extends AbstractFailFastTest {
        final ReloadableService<AttributeResolver > service = (ReloadableService<AttributeResolver>) bean;
        assertNotNull(service);
        final ServiceableComponent<AttributeResolver> component = service.getServiceableComponent();
-       assertNotNull(component);
-       assert component != null;
        final AttributeResolver resolver = component.getComponent();
        assertNotNull(resolver);
     }
@@ -128,8 +125,13 @@ public class AttributeResolverFailFastTest extends AbstractFailFastTest {
             return;
         }
         assertNotNull(service);
-        final ServiceableComponent<AttributeResolver> component = service.getServiceableComponent();
-        assertNull(component);
+        
+        try {
+            service.getServiceableComponent();
+            fail("Should have thrown");
+        } catch (final ServiceException e) {
+            // OK
+        }
     }
     
     @Test public void badAttributeFF()  throws IOException {
diff --git a/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java b/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
index 63dc853ad..6b4833169 100644
--- a/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
+++ b/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
@@ -43,6 +43,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.logic.ConstraintViolationException;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 import org.opensaml.core.xml.XMLObject;
@@ -97,8 +98,8 @@ public class AttributeMappingNodeProcessor implements MetadataNodeProcessor {
     @Override public void process(final XMLObject metadataNode) throws FilterException {
 
         if (metadataNode instanceof AttributeConsumingService || metadataNode instanceof EntityDescriptor) {
-            try (final ServiceableComponent<AttributeTranscoderRegistry> 
-                       component = transcoderRegistry.getServiceableComponent()) {
+            try (final ServiceableComponent<AttributeTranscoderRegistry> component =
+                    transcoderRegistry.getServiceableComponent()) {
                 if (metadataNode instanceof AttributeConsumingService) {
                     handleAttributeConsumingService(component.getComponent(), (AttributeConsumingService) metadataNode);
                 } else if (metadataNode instanceof EntityDescriptor) {
@@ -109,6 +110,8 @@ public class AttributeMappingNodeProcessor implements MetadataNodeProcessor {
                         parent = parent.getParent();
                     }
                 }
+            } catch (final ServiceException e) {
+                log.warn("Invalid AttributeTranscoderRegistry configuration", e);
             }
         }
     }

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


More information about the commits mailing list