[java-identity-provider] branch main updated: Fix null checking issues.

Scott Cantor cantor.2 at osu.edu
Thu Jan 19 18:21:59 UTC 2023


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

scantor pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=8854f7963d224076ee79da66fb51474d12b61bde

The following commit(s) were added to refs/heads/main by this push:
     new 8854f7963 Fix null checking issues.
8854f7963 is described below

commit 8854f7963d224076ee79da66fb51474d12b61bde
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Jan 19 13:21:56 2023 -0500

    Fix null checking issues.
---
 .../factory/FlowDefinitionRegistryFactoryBean.java | 54 +++++++++++++++-------
 .../factory/FlowDefinitionResourceFactory.java     | 19 ++++++--
 .../spring/factory/FlowModelFlowBuilder.java       |  2 +-
 .../spring/factory/FlowRelativeResourceLoader.java |  8 ++--
 .../spring/logic/RelyingPartyIdPredicateTest.java  |  3 +-
 5 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionRegistryFactoryBean.java b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionRegistryFactoryBean.java
index 93f497b10..1cb92dc76 100644
--- a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionRegistryFactoryBean.java
+++ b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionRegistryFactoryBean.java
@@ -31,6 +31,7 @@ import javax.annotation.Nullable;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.AbstractFactoryBean;
 import org.springframework.webflow.config.FlowDefinitionResource;
 import org.springframework.webflow.core.collection.LocalAttributeMap;
@@ -79,9 +80,6 @@ public class FlowDefinitionRegistryFactoryBean extends AbstractFactoryBean<FlowD
 
     /** Optional parent reference. */
     @Nullable private FlowDefinitionRegistry parent;
-
-    /** Overriden resource factory, the whole reason for this class. */
-    @Nullable private FlowDefinitionResourceFactory flowResourceFactory;
     
     /** Constructor. */
     public FlowDefinitionRegistryFactoryBean() {
@@ -159,35 +157,48 @@ public class FlowDefinitionRegistryFactoryBean extends AbstractFactoryBean<FlowD
 
     /** {@inheritDoc} */
     @Override
-    protected FlowDefinitionRegistry createInstance() throws Exception {
+    @Nonnull protected FlowDefinitionRegistry createInstance() throws Exception {
 
-        flowResourceFactory = new FlowDefinitionResourceFactory(flowBuilderServices.getApplicationContext());
+        if (flowBuilderServices == null) {
+            throw new BeanCreationException("FlowBuilderServices instance was null");
+        }
+        
+        assert flowBuilderServices != null;
+        
+        // This is the whole reason for this class.
+        final FlowDefinitionResourceFactory flowResourceFactory =
+                new FlowDefinitionResourceFactory(flowBuilderServices.getApplicationContext());
         
         final DefaultFlowRegistry flowRegistry = new DefaultFlowRegistry();
         flowRegistry.setParent(this.parent);
 
-        registerFlowLocations(flowRegistry);
-        registerFlowLocationPatterns(flowRegistry);
+        registerFlowLocations(flowRegistry, flowResourceFactory);
+        registerFlowLocationPatterns(flowRegistry, flowResourceFactory);
 
         return flowRegistry;
     }
 
     /** {@inheritDoc} */
-    @Override protected void destroyInstance(final FlowDefinitionRegistry instance) throws Exception {
-        ((DefaultFlowRegistry) instance).destroy();
+    @Override protected void destroyInstance(@Nullable final FlowDefinitionRegistry instance) throws Exception {
+        if (instance != null) {
+            ((DefaultFlowRegistry) instance).destroy();
+        }
     }
 
     /**
      * Register explicit flow mappings.
      * 
      * @param flowRegistry the flow registry
+     * @param resourceFactory the flow definition resource factory
      */
-    private void registerFlowLocations(@Nonnull final DefaultFlowRegistry flowRegistry) {
+    private void registerFlowLocations(@Nonnull final DefaultFlowRegistry flowRegistry, 
+            @Nonnull final FlowDefinitionResourceFactory resourceFactory) {
         for (final Map.Entry<String,String> location : flowLocations.entrySet()) {
             final LocalAttributeMap<Object> attributes = new LocalAttributeMap<>();
             updateFlowAttributes(attributes);
+            
             final FlowDefinitionResource resource =
-                    flowResourceFactory.createResource(basePath, location.getValue(), attributes, location.getKey());
+                    resourceFactory.createResource(basePath, location.getValue(), attributes, location.getKey());
             registerFlow(resource, flowRegistry);
         }
     }
@@ -196,15 +207,24 @@ public class FlowDefinitionRegistryFactoryBean extends AbstractFactoryBean<FlowD
      * Register flows derived from resource patterns.
      * 
      * @param flowRegistry the flow registry
+     * @param resourceFactory the flow definition resource factory
      */
-    private void registerFlowLocationPatterns(@Nonnull final DefaultFlowRegistry flowRegistry) {
+    private void registerFlowLocationPatterns(@Nonnull final DefaultFlowRegistry flowRegistry,
+            @Nonnull final FlowDefinitionResourceFactory resourceFactory) {
         for (final Map.Entry<String,String> pattern : flowLocationPatterns.entrySet()) {
             final LocalAttributeMap<Object> attributes = new LocalAttributeMap<>();
             updateFlowAttributes(attributes);
             final Collection<FlowDefinitionResource> resources;
             try {
-                resources = flowResourceFactory.createResources(
-                        pattern.getValue() != null ? pattern.getValue() : basePath, pattern.getKey(), attributes);
+                final String base;
+                if (pattern.getValue() != null) {
+                    base = pattern.getValue();
+                } else if (basePath != null) {
+                    base = basePath;
+                } else {
+                    base = "";
+                }
+                resources = resourceFactory.createResources(base, pattern.getKey(), attributes);
             } catch (final IOException e) {
                 throw new IllegalStateException(
                         "An I/O Exception occurred resolving the flow location pattern '" + pattern.getKey() + "'", e);
@@ -236,7 +256,8 @@ public class FlowDefinitionRegistryFactoryBean extends AbstractFactoryBean<FlowD
     private void registerFlow(@Nonnull final FlowDefinitionResource resource,
             @Nonnull final DefaultFlowRegistry flowRegistry) {
         FlowModelBuilder flowModelBuilder = null;
-        if (resource.getPath().getFilename().endsWith(".xml")) {
+        final String fname = resource.getPath().getFilename();
+        if (fname != null && fname.endsWith(".xml")) {
             flowModelBuilder = new XmlFlowModelBuilder(resource.getPath(), flowRegistry.getFlowModelRegistry());
         } else {
             throw new IllegalArgumentException(resource
@@ -260,7 +281,8 @@ public class FlowDefinitionRegistryFactoryBean extends AbstractFactoryBean<FlowD
      * @param attributes attribute map to update
      */
     private void updateFlowAttributes(@Nonnull final LocalAttributeMap<Object> attributes) {
-        if (flowBuilderServices.getDevelopment()) {
+        
+        if (flowBuilderServices != null && flowBuilderServices.getDevelopment()) {
             attributes.put("development", true);
         }
     }
diff --git a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionResourceFactory.java b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionResourceFactory.java
index 5c3334363..783264041 100644
--- a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionResourceFactory.java
+++ b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowDefinitionResourceFactory.java
@@ -83,7 +83,7 @@ public class FlowDefinitionResourceFactory {
      * 
      * @return the flow definition resource
      */
-    public FlowDefinitionResource createResource(@Nullable final String basePath,
+    @Nonnull public FlowDefinitionResource createResource(@Nullable final String basePath,
             @Nonnull @NotEmpty final String path, @Nonnull final AttributeMap<Object> attributes,
             @Nonnull @NotEmpty final String flowId) {
         Constraint.isNotEmpty(path, "Flow path cannot be null or empty");
@@ -153,6 +153,7 @@ public class FlowDefinitionResourceFactory {
         
         final Collection<FlowDefinitionResource> flowResources = new ArrayList<>(resources.length);
         for (final Resource resource : resources) {
+            assert resource != null;
             flowResources.add(new FlowDefinitionResource(getFlowId(basePath, resource), resource, attributes));
         }
         return flowResources;
@@ -186,7 +187,12 @@ public class FlowDefinitionResourceFactory {
             filePath = truncateFilePath(flowResource.getURL().getPath(), localBasePath);
         } else {
             // Default to the filename.
-            return StringUtils.stripFilenameExtension(flowResource.getFilename());
+            final String fname = flowResource.getFilename();
+            if (fname != null) {
+                return StringUtils.stripFilenameExtension(fname);
+            } else {
+                throw new IOException("Unable to obtain filename from Resource of type " + flowResource.getClass().getName());
+            }
         }
 
         int beginIndex = 0;
@@ -205,7 +211,12 @@ public class FlowDefinitionResourceFactory {
             endIndex = filePath.lastIndexOf(SLASH);
         } else {
             // There is no path info, default to the filename.
-            return StringUtils.stripFilenameExtension(flowResource.getFilename());
+            final String fname = flowResource.getFilename();
+            if (fname != null) {
+                return StringUtils.stripFilenameExtension(fname);
+            } else {
+                throw new IOException("Unable to obtain filename from Resource of type " + flowResource.getClass().getName());
+            }
         }
         return filePath.substring(beginIndex, endIndex);
     }
@@ -253,7 +264,7 @@ public class FlowDefinitionResourceFactory {
      * 
      * @return the input with the scheme removed.
      */
-    private String removeScheme(@Nonnull @NotEmpty final String path) {
+    @Nonnull private String removeScheme(@Nonnull @NotEmpty final String path) {
         if (path.startsWith(CLASSPATH_SCHEME)) {
             return path.substring(CLASSPATH_SCHEME.length());
         } else if (path.startsWith(FILESYSTEM_SCHEME)) {
diff --git a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowModelFlowBuilder.java b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowModelFlowBuilder.java
index 5135e1a2d..d0f9a7fa0 100644
--- a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowModelFlowBuilder.java
+++ b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowModelFlowBuilder.java
@@ -69,7 +69,7 @@ public class FlowModelFlowBuilder extends org.springframework.webflow.engine.bui
      * 
      * @return the imported resource paths
      */
-    private String[] parseContextResources(@Nullable final List<BeanImportModel> beanImports) {
+    @Nonnull private String[] parseContextResources(@Nullable final List<BeanImportModel> beanImports) {
         if (beanImports != null && !beanImports.isEmpty()) {
             final String[] resources = new String[beanImports.size()];
             return beanImports.stream()
diff --git a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowRelativeResourceLoader.java b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowRelativeResourceLoader.java
index 07855d530..ce743803e 100644
--- a/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowRelativeResourceLoader.java
+++ b/idp-spring/src/main/java/net/shibboleth/idp/profile/spring/factory/FlowRelativeResourceLoader.java
@@ -19,6 +19,8 @@ package net.shibboleth.idp.profile.spring.factory;
 
 import java.io.IOException;
 
+import javax.annotation.Nonnull;
+
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.FileSystemResource;
@@ -57,7 +59,7 @@ class FlowRelativeResourceLoader extends DefaultResourceLoader {
 
     /** {@inheritDoc} */
     @Override
-    public Resource getResource(final String location) {
+    @Nonnull public Resource getResource(@Nonnull final String location) {
         
         try {
             final Resource r = super.getResource(location);
@@ -79,7 +81,7 @@ class FlowRelativeResourceLoader extends DefaultResourceLoader {
      * @param location the resource location
      * @return a corresponding Resource handle
      */
-    private Resource createFlowRelativeResource(final String location) {
+    @Nonnull private Resource createFlowRelativeResource(@Nonnull final String location) {
         try {
             return flowResource.createRelative(location);
         } catch (final IOException e) {
@@ -100,7 +102,7 @@ class FlowRelativeResourceLoader extends DefaultResourceLoader {
      * </p>
      */
     @Override
-    protected Resource getResourceByPath(final String path) {
+    @Nonnull protected Resource getResourceByPath(@Nonnull final String path) {
         try {
             final Resource r = new FileSystemResource(path);
             if (r.exists()) {
diff --git a/idp-spring/src/test/java/net/shibboleth/idp/profile/spring/logic/RelyingPartyIdPredicateTest.java b/idp-spring/src/test/java/net/shibboleth/idp/profile/spring/logic/RelyingPartyIdPredicateTest.java
index 35255ce22..08f2a873f 100644
--- a/idp-spring/src/test/java/net/shibboleth/idp/profile/spring/logic/RelyingPartyIdPredicateTest.java
+++ b/idp-spring/src/test/java/net/shibboleth/idp/profile/spring/logic/RelyingPartyIdPredicateTest.java
@@ -35,7 +35,8 @@ public class RelyingPartyIdPredicateTest {
     
     private boolean testCandidate(final RelyingPartyIdPredicate rpIdPredicate, final String rpId) {
         ProfileRequestContext prc = new ProfileRequestContext();
-        RelyingPartyContext rpc = prc.getSubcontext(RelyingPartyContext.class, true);
+        final RelyingPartyContext rpc = prc.getSubcontext(RelyingPartyContext.class, true);
+        assert rpc != null;
         rpc.setRelyingPartyId(rpId);
         return rpIdPredicate.test(prc);
     }

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


More information about the commits mailing list