refactor: NPE in case of un-exploded WAR deployments #100

* Removed the "getJavascriptSourceFile()" method from the ConfigurationProvider interface.
* Better error handling (narrower exception catching and explicit null check)

Author: forgedhallpass

csrfguard/src/main/java/org/owasp/csrfguard/config/ConfigurationProvider.java

@@ -160,11 +160,6 @@ public interface ConfigurationProvider {
      */
     List<IAction> getActions();
 
-    /**
-     * @return the overridden path to the configured CSRFGuard JavaScript logic, or <b>null</b> if the default is used
-     */
-    String getJavascriptSourceFile();
-
     /**
      * @return true if tokens should only be injected into links that have the same domain from which the HTML originates,
      * false if subdomains are also permitted

csrfguard/src/main/java/org/owasp/csrfguard/config/NullConfigurationProvider.java

@@ -153,11 +153,6 @@ public List<IAction> getActions() {
         return Collections.emptyList();
     }
 
-    @Override
-    public String getJavascriptSourceFile() {
-        return null;
-    }
-
     @Override
     public boolean isJavascriptDomainStrict() {
         return false;

csrfguard/src/main/java/org/owasp/csrfguard/config/PropertiesConfigurationProvider.java

@@ -46,10 +46,8 @@
 import org.slf4j.LoggerFactory;
 
 import javax.servlet.ServletConfig;
-
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
 import java.security.*;
 import java.time.Duration;
 import java.util.*;
@@ -113,8 +111,6 @@ public class PropertiesConfigurationProvider implements ConfigurationProvider {
 
 	private String javascriptTemplateCode;
 
-	private String javascriptSourceFile;
-
 	private boolean javascriptDomainStrict;
 
 	private String javascriptCacheControl;
@@ -288,11 +284,6 @@ public void initializeJavaScriptConfiguration() {
 		this.javascriptInitParamsIfNeeded();
 	}
 
-	@Override
-	public String getJavascriptSourceFile() {
-		return this.javascriptSourceFile;
-	}
-
 	@Override
 	public boolean isJavascriptDomainStrict() {
 		return this.javascriptDomainStrict;
@@ -550,44 +541,45 @@ private void javascriptInitParamsIfNeeded() {
 				this.javascriptRefererMatchProtocol = getProperty(JavaScriptConfigParameters.REFERER_MATCH_PROTOCOL, servletConfig);
 				this.javascriptRefererMatchDomain = getProperty(JavaScriptConfigParameters.REFERER_MATCH_DOMAIN, servletConfig);
 				this.javascriptUnprotectedExtensions = getProperty(JavaScriptConfigParameters.UNPROTECTED_EXTENSIONS, servletConfig);
-				this.javascriptSourceFile = getProperty(JavaScriptConfigParameters.SOURCE_FILE, servletConfig);
 				this.javascriptXrequestedWith = getProperty(JavaScriptConfigParameters.X_REQUESTED_WITH, servletConfig);
 
-				if (StringUtils.isBlank(this.javascriptSourceFile)) {
-					this.javascriptTemplateCode = CsrfGuardUtils.readResourceFileContent("META-INF/csrfguard.js");
-				} else if (this.javascriptSourceFile.startsWith("META-INF/")) {
-					this.javascriptTemplateCode = CsrfGuardUtils.readResourceFileContent(this.javascriptSourceFile);
-				} else if (this.javascriptSourceFile.startsWith("classpath:")) {
-					final String location = this.javascriptSourceFile.substring("classpath:".length()).trim();
-					this.javascriptTemplateCode = CsrfGuardUtils.readResourceFileContent(location);
-				} else if (this.javascriptSourceFile.startsWith("file:")) {
-					final String location = this.javascriptSourceFile.substring("file:".length()).trim();
-					this.javascriptTemplateCode = CsrfGuardUtils.readFileContent(location);
-				} else if (servletConfig.getServletContext().getRealPath(this.javascriptSourceFile) != null) {
-					this.javascriptTemplateCode = CsrfGuardUtils.readFileContent(servletConfig.getServletContext().getRealPath(this.javascriptSourceFile));
-				} else {
-					try( final InputStream inputStream = getResourceStream(this.javascriptSourceFile, servletConfig)){
-						this.javascriptTemplateCode = CsrfGuardUtils.readInputStreamContent(inputStream);	
-					} catch (final Exception e) {
-						throw new IllegalStateException("getRealPath failed for file " + this.javascriptSourceFile);
-					}
-				}
+				final String javascriptSourceFileLocation = getProperty(JavaScriptConfigParameters.SOURCE_FILE_LOCATION, servletConfig);
+				this.javascriptTemplateCode = retrieveJavaScriptTemplateCode(servletConfig, javascriptSourceFileLocation);
 
 				this.javascriptParamsInitialized = true;
 			}
 		}
 	}
-	
-	private InputStream getResourceStream(final String resourcePath, final ServletConfig servletConfig) throws MalformedURLException  {
-		InputStream inputStream = null;
-		
-		if(servletConfig.getServletContext().getResource("/" + this.javascriptSourceFile) != null) {
-			inputStream = servletConfig.getServletContext().getResourceAsStream("/" + this.javascriptSourceFile);
+
+	private static String retrieveJavaScriptTemplateCode(ServletConfig servletConfig, String jsSourceFileLocation) {
+		String result = null;
+
+		if (StringUtils.isBlank(jsSourceFileLocation)) {
+			result = CsrfGuardUtils.readResourceFileContent("META-INF/csrfguard.js");
+		} else if (jsSourceFileLocation.startsWith("META-INF/")) {
+			result = CsrfGuardUtils.readResourceFileContent(jsSourceFileLocation);
+		} else if (jsSourceFileLocation.startsWith("classpath:")) {
+			final String location = jsSourceFileLocation.substring("classpath:".length()).trim();
+			result = CsrfGuardUtils.readResourceFileContent(location);
+		} else if (jsSourceFileLocation.startsWith("file:")) {
+			final String location = jsSourceFileLocation.substring("file:".length()).trim();
+			result = CsrfGuardUtils.readFileContent(location);
+		} else {
+			try (final InputStream inputStream = servletConfig.getServletContext().getResourceAsStream('/' + jsSourceFileLocation)) {
+				if (inputStream != null) {
+					result = CsrfGuardUtils.readInputStreamContent(inputStream);
+				}
+			} catch (final IOException e) {
+				throw new IllegalStateException(String.format("Error while trying to close the '%s' resource.", jsSourceFileLocation));
+			}
 		}
-		
-		return inputStream;
-	}
 
+		if (StringUtils.isBlank(result)) {
+			throw new IllegalStateException("Error while trying to retrieve the JavaScript source code!");
+		}
+
+		return result;
+	}
 
 	private <T> T getProperty(final JsConfigParameter<T> jsConfigParameter, final ServletConfig servletConfig) {
 		return jsConfigParameter.getProperty(servletConfig, this.propertiesCache);

csrfguard/src/main/java/org/owasp/csrfguard/config/properties/javascript/JavaScriptConfigParameters.java

@@ -42,7 +42,7 @@ private JavaScriptConfigParameters() {}
     public static final StringJsConfigParameter CACHE_CONTROL = new StringJsConfigParameter("cache-control", "org.owasp.csrfguard.JavascriptServlet.cacheControl", "private, max-age=28800");
     public static final StringJsConfigParameter REFERER_PATTERN  = new StringJsConfigParameter("referer-pattern", "org.owasp.csrfguard.JavascriptServlet.refererPattern", DEFAULT_REFERER_PATTERN);
     public static final StringJsConfigParameter UNPROTECTED_EXTENSIONS = new StringJsConfigParameter("unprotected-extensions", "org.owasp.csrfguard.JavascriptServlet.UnprotectedExtensions", StringUtils.EMPTY);
-    public static final StringJsConfigParameter SOURCE_FILE = new StringJsConfigParameter("source-file", "org.owasp.csrfguard.JavascriptServlet.sourceFile", null);
+    public static final StringJsConfigParameter SOURCE_FILE_LOCATION = new StringJsConfigParameter("source-file", "org.owasp.csrfguard.JavascriptServlet.sourceFile", null);
     public static final StringJsConfigParameter X_REQUESTED_WITH = new StringJsConfigParameter("x-requested-with", "org.owasp.csrfguard.JavascriptServlet.xRequestedWith", "OWASP CSRFGuard Project");
     public static final StringJsConfigParameter DYNAMIC_NODE_CREATION_EVENT_NAME = new StringJsConfigParameter("dynamic-node-creation-event", "org.owasp.csrfguard.JavascriptServlet.dynamicNodeCreationEventName", null);