[Esapi-dev] bug with setResourceDirectory() in rc6 and enhancement suggestion

[Jim Manico]

Nice work Josh.  I made this change to

http://owasp-esapi-java.googlecode.com/svn/trunk/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java

As well as the configurable directory change to the 1.4 branch at

http://owasp-esapi-java.googlecode.com/svn/branches/1.4/src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java  


Feedback always appreciated,

Good patches like this are appreciated even more ;),

-- 
Jim Manico
OWASP Podcast Host/Producer
OWASP ESAPI Project Manager
http://www.manico.net



> Hi,
>
> I believe I've found a bug introduced in 2.0-rc6 .Here is the environment:
>
> 1. using esapi in a web application
> 2. *not* setting "org.owasp.esapi.resources" system property
> 3. ".esapi" folder not in user.home or classpath (2.0-rc6 download didn't include it in the jar either)
>
> When I invoke ESAPI.securityConfiguration().setResourceDirectory(esapiResourceDir) rc6 blows up with a NullPointerException.  The problem appears to be that the "if ( shouldPrintProperties() ) {" block still gets executed when a file isn't found during the securityConfiguration() invocation and the properties is thus null, never allowing setResourceDirectory() to be called.  To fix this, the " if ( shouldPrintProperties() ) {" block should be moved up a few lines to be included inside the "if (properties != null) {" block.
>
> Although fixing the exception from occurring, using setResourceDirectory() from inside a web application is still somewhat useless because resourceDirectory is only used when checking ClassLoader.getSystemResource() which typically isn't your webapp context's classloader.  I would suggest adding a check to resourceDirectory inside loadConfigurationFromClasspath() when it loops through the classloaders.  I made the change locally and it seems to work well, doing this one can specify a custom directory name inside a web application's context.  Attached is a diff patch file for the details...
>
> Thanks,
> ~Josh
>
> ### Eclipse Workspace Patch 1.0
> #P ESAPI
> Index: src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java
> ===================================================================
> --- src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java(revision 1234)
> +++ src/main/java/org/owasp/esapi/reference/DefaultSecurityConfiguration.java(working copy)
> @@ -447,25 +447,26 @@
>      properties.put( key, value);
>      }
>  }
> +        if ( shouldPrintProperties() ) {
> +            
> +            //FIXME - make this chunk configurable
> +            /*
> +            logSpecial("  ========Master Configuration========", null);
> +            //logSpecial( "  ResourceDirectory: " + DefaultSecurityConfiguration.resourceDirectory );
> +            Iterator j = new TreeSet( properties.keySet() ).iterator();
> +            while (j.hasNext()) {
> +                String key = (String)j.next();
> +                // print out properties, but not sensitive ones like MasterKey and MasterSalt
> +                if ( !key.contains( "Master" ) ) {
> +                        logSpecial("  |   " + key + "=" + properties.get(key), null);
> +                }
> +            }
> +            */
> +                
> +            }
> +
>  }
>  
> -        if ( shouldPrintProperties() ) {
> -    
> -    //FIXME - make this chunk configurable
> -    /*
> -        logSpecial("  ========Master Configuration========", null);
> -        //logSpecial( "  ResourceDirectory: " + DefaultSecurityConfiguration.resourceDirectory );
> -        Iterator j = new TreeSet( properties.keySet() ).iterator();
> -        while (j.hasNext()) {
> -            String key = (String)j.next();
> -            // print out properties, but not sensitive ones like MasterKey and MasterSalt
> -            if ( !key.contains( "Master" ) ) {
> -            logSpecial("  |   " + key + "=" + properties.get(key), null);
> -        }
> -        }
> -        */
> -        
> -        }
>      }
>  
>  /**
> @@ -566,7 +567,12 @@
>  // try root
>  in = loaders[i].getResourceAsStream(fileName);
>  
> -// try .esapi folder
> +                    // try resourceDirectory folder
> +                    if (in == null) {
> +                        in = currentLoader.getResourceAsStream(resourceDirectory + "/" + fileName);
> +                    } 
> +
> +                    // try .esapi folder
>  if (in == null) {
>  in = currentLoader.getResourceAsStream(".esapi/" + fileName);
>  } 
>
>
>
>       
> _______________________________________________
> Esapi-dev mailing list
> Esapi-dev at lists.owasp.org
> https://lists.owasp.org/mailman/listinfo/esapi-dev
>