Addresses issue 193

README.md

@@ -20,20 +20,31 @@ The methods enterCriticalSection and leaveCritialSection are available to suppor
 The library is available in [Maven Central org.spdx:java-spdx-library](https://search.maven.org/artifact/org.spdx/java-spdx-library).
 
 If you are using Maven, you can add the following dependency in your POM file:
-```
+```xml
 <dependency>
   <groupId>org.spdx</groupId>
   <artifactId>java-spdx-library</artifactId>
   <version>(,1.0]</version>
 </dependency>
 ```
 
-[API JavaDocs are available here.](https://spdx.github.io/Spdx-Java-Library/)
+[API JavaDocs are available here](https://spdx.github.io/Spdx-Java-Library/).
 
 There are a couple of static classes that help common usage scenarios:
 
-- org.spdx.library.SPDXModelFactory supports the creation of specific model objects
-- org.spdx.library.model.license.LicenseInfoFactory supports the parsing of SPDX license expressions, creation, and comparison of SPDX licenses
+- `org.spdx.library.SPDXModelFactory` supports the creation of specific model objects
+- `org.spdx.library.model.license.LicenseInfoFactory` supports the parsing of SPDX license expressions, creation, and comparison of SPDX licenses
+
+## Configuration options
+
+`Spdx-Java-Library` can be configured using either Java system properties or a Java properties file located in the runtime CLASSPATH at `/resources/spdx-java-library.properties`.
+
+The library has these configuration options:
+1. `SPDXParser.OnlyUseLocalLicenses` - a boolean that controls whether the (potentially out of date) listed license information bundled inside the JAR is used (true), vs the library downloading the latest files from the SPDX website (false). Default is false (always download the latest files from the SPDX website).
+2. `org.spdx.storage.listedlicense.enableCache` - a boolean that enables or disables a local cache for downloaded listed license information. Defaults to `false` (the cache is disabled). The cache location is determined as per the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) (i.e. `${XDG_CACHE_HOME}/Spdx-Java-Library` or `${HOME}/.cache/Spdx-Java-Library`).
+3. `org.spdx.storage.listedlicense.cacheCheckIntervalSecs` - a long that controls how often each cache entry is rechecked for staleness, in units of seconds. Defaults to 86,400 seconds (24 hours). Set to 0 (zero) to have each cache entry checked every time (note: this will result in a lot more network I/O and negatively impact performance, albeit there is still a substantial performance saving vs not using the cache at all).
+
+Note that these configuration options can only be modified prior to first use of Spdx-Java-Library. Once the library is initialized, subsequent changes will have no effect.
 
 ## Update for new properties or classes
 To update Spdx-Java-Library, the following is a very brief checklist:

resources/spdx-java-library.properties.sample

@@ -0,0 +1,8 @@
+# if true, use licenses from stored set of licenses rather than from the spdx.org/licenses website
+SPDXParser.OnlyUseLocalLicenses=false
+
+# if true, enable a local download cache for listed license files downloaded from the SPDX website
+org.spdx.storage.listedlicense.enableCache=false
+
+# local download cache re-check interval, in seconds
+org.spdx.storage.listedlicense.cacheCheckIntervalSecs=86400

src/main/java/org/spdx/Configuration.java

@@ -0,0 +1,119 @@
+/**
+ * Copyright (c) 2023 Source Auditor Inc.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ */
+package org.spdx;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Properties;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The configuration class for the Spdx-Java-Library. When a caller attempts to retrieve a configuration property, it
+ * will first be checked in the Java system properties (i.e. set via `-D` command line options to the JVM, or by
+ * programmatic calls to `System.setProperty()` in code), and will then fallback on a properties file in the classpath.
+ * That file must be called `/resources/spdx-java-library.properties`.
+ *
+ * Please see the documentation for specifics on what configuration options Spdx-Java-Library supports, and how they
+ * impact the library's behavior.
+ */
+public final class Configuration {
+    private static final Logger logger = LoggerFactory.getLogger(Configuration.class.getName());
+    private static final String PROPERTIES_DIR = "resources";
+    private static final String CONFIGURATION_PROPERTIES_FILENAME = PROPERTIES_DIR + "/" + "spdx-java-library.properties";
+    private static final String DEPRECATED_CONFIGURATION_PROPERTIES_FILENAME = PROPERTIES_DIR + "/" + "licenses.properties";   // Deprecated filename
+
+    private static Configuration singleton;
+    private final Properties properties;
+
+    private Configuration() {
+        Properties tmpProperties = loadProperties(CONFIGURATION_PROPERTIES_FILENAME);
+        if (tmpProperties == null) {
+            // This is to preserve backwards compatibility with version 1.1.7 of the library and earlier
+            tmpProperties = loadProperties(DEPRECATED_CONFIGURATION_PROPERTIES_FILENAME);
+            if (tmpProperties != null) {
+                logger.warn("You are using a deprecated configuration properties filename ('" + DEPRECATED_CONFIGURATION_PROPERTIES_FILENAME + "'). Please consider migrating to the new name ('" + CONFIGURATION_PROPERTIES_FILENAME + "').");
+            }
+        }
+        properties = tmpProperties;
+    }
+
+    /**
+     * @return The singleton instance of the Configuration class.
+     */
+    public static Configuration getInstance() {
+        if (singleton == null) {
+            singleton = new Configuration();
+        }
+        return singleton;
+    }
+
+    /**
+     * @param propertyName The name of the configuration property to retrieve.
+     * @return The value of the given property name, or null if it wasn't found.
+     */
+    public String getProperty(final String propertyName) {
+        return getProperty(propertyName, null);
+    }
+
+    /**
+     * @param propertyName The name of the configuration property to retrieve.
+     * @param defaultValue The default value to return, if the property isn't found.
+     * @return The value of the given property name, or defaultValue if it wasn't found.
+     */
+    public String getProperty(final String propertyName, final String defaultValue) {
+        return System.getProperty(propertyName, properties == null ? defaultValue : properties.getProperty(propertyName, defaultValue));
+    }
+
+    /**
+     * Tries to load properties from the CLASSPATH, using the provided filename, ignoring errors
+     * encountered during the process (e.g., the properties file doesn't exist, etc.).
+     *
+     * @param propertiesFileName the name of the file to load, including path (if any)
+     * @return a (possibly empty) set of properties, or null if the properties file doesn't exist on the CLASSPATH
+     */
+    private static Properties loadProperties(final String propertiesFileName) {
+        Properties result = null;
+        if (propertiesFileName != null) {
+            InputStream in = null;
+            try {
+                in = Configuration.class.getResourceAsStream("/" + propertiesFileName);
+                if (in != null) {
+                    result = new Properties();
+                    result.load(in);
+                }
+            } catch (IOException e) {
+                // Ignore it and fall through
+                logger.warn("IO Exception reading configuration properties file '" + propertiesFileName + "': " + e.getMessage(), e);
+                result = null;
+            } finally {
+                if (in != null) {
+                    try {
+                        in.close();
+                    } catch (IOException e) {
+                        // Ignore it and fall through
+                        logger.warn("Unable to close configuration properties file '" + propertiesFileName + "': " + e.getMessage(), e);
+                    }
+                }
+            }
+        }
+        return result;
+    }
+
+}

src/main/java/org/spdx/library/model/license/ListedLicenses.java

@@ -28,6 +28,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.spdx.Configuration;
 import org.spdx.library.InvalidSPDXAnalysisException;
 import org.spdx.library.SpdxConstants;
 import org.spdx.library.model.SpdxModelFactory;
@@ -45,10 +46,7 @@
 public class ListedLicenses {
 
 	static final Logger logger = LoggerFactory.getLogger(ListedLicenses.class.getName());
-	private static final String PROPERTIES_DIR = "resources";
-	private static final String LISTED_LICENSE_PROPERTIES_FILENAME = PROPERTIES_DIR + "/" + "licenses.properties";
 
-	Properties licenseProperties;
     boolean onlyUseLocalLicenses;
 	private IListedLicenseStore licenseModelStore;
 	private static ListedLicenses listedLicenses = null;
@@ -61,46 +59,14 @@ public class ListedLicenses {
 	 * This constructor should only be called by the getListedLicenses method
 	 */
 	private ListedLicenses() {
-		licenseProperties = loadLicenseProperties();
-		onlyUseLocalLicenses = Boolean.parseBoolean(
-	            System.getProperty("SPDXParser.OnlyUseLocalLicenses", licenseProperties.getProperty("OnlyUseLocalLicenses", "false")));
+		// Note: this code confusingly uses different property values depending on the source (Java system property vs properties file),
+		// so we have to check both names in order to not break downstream consumers' legacy configurations.  This is _NOT_ recommended for
+		// any new code that leverages the Configuration class.
+		onlyUseLocalLicenses = Boolean.parseBoolean(Configuration.getInstance().getProperty("SPDXParser.OnlyUseLocalLicenses",
+				                                    Configuration.getInstance().getProperty("OnlyUseLocalLicenses", "false")));
 		initializeLicenseModelStore();
 	}
-
-	/**
-	 * Tries to load properties from LISTED_LICENSE_PROPERTIES_FILENAME, ignoring errors
-	 * encountered during the process (e.g., the properties file doesn't exist, etc.).
-	 * 
-	 * @return a (possibly empty) set of properties
-	 */
-    private static Properties loadLicenseProperties() {
-        listedLicenseModificationLock.writeLock().lock();
-        try {
-            Properties licenseProperties = new Properties();
-            InputStream in = null;
-            try {
-                in = ListedLicenses.class.getResourceAsStream("/" + LISTED_LICENSE_PROPERTIES_FILENAME);
-                if (in != null) {
-                    licenseProperties.load(in);
-                }
-            } catch (IOException e) {
-                // Ignore it and fall through
-                logger.warn("IO Exception reading listed license properties file: " + e.getMessage(), e);
-            } finally {
-                if (in != null) {
-                    try {
-                        in.close();
-                    } catch (IOException e) {
-                        logger.warn("Unable to close listed license properties file: " + e.getMessage(), e);
-                    }
-                }
-            }
-            return licenseProperties;
-        } finally {
-            listedLicenseModificationLock.writeLock().unlock();
-        }
-    }
-
+
     private void initializeLicenseModelStore() {
         listedLicenseModificationLock.writeLock().lock();
         try {
@@ -125,8 +91,6 @@ private void initializeLicenseModelStore() {
         }
 	}
 
-
-
 	public static ListedLicenses getListedLicenses() {
 
 	    ListedLicenses retval = null;

src/main/java/org/spdx/storage/listedlicense/SpdxListedLicenseWebStore.java

@@ -19,61 +19,33 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.HttpURLConnection;
 import java.net.URL;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-import java.util.Objects;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.spdx.library.InvalidSPDXAnalysisException;
 import org.spdx.library.SpdxConstants;
+import org.spdx.utility.DownloadCache;
 
 /**
- * @author gary
+ * @author gary   Original code
+ * @author pmonks Optional caching of downloaded files
  *
  */
 public class SpdxListedLicenseWebStore extends SpdxListedLicenseModelStore {
 
-	private static final int READ_TIMEOUT = 5000;
-
-	static final List<String> WHITE_LIST = Collections.unmodifiableList(Arrays.asList(
-			"spdx.org", "spdx.dev", "spdx.com", "spdx.info")); // Allowed host names for the SPDX listed licenses
+	private static final Logger logger = LoggerFactory.getLogger(SpdxListedLicenseModelStore.class);
+
 
 	/**
 	 * @throws InvalidSPDXAnalysisException
 	 */
 	public SpdxListedLicenseWebStore() throws InvalidSPDXAnalysisException {
 		super();
 	}
-
-	private InputStream getUrlInputStream(URL url) throws IOException {
-		HttpURLConnection connection = (HttpURLConnection)url.openConnection();
-		connection.setReadTimeout(READ_TIMEOUT);
-		int status = connection.getResponseCode();
-		if (status != HttpURLConnection.HTTP_OK && 
-			(status == HttpURLConnection.HTTP_MOVED_TEMP || status == HttpURLConnection.HTTP_MOVED_PERM
-						|| status == HttpURLConnection.HTTP_SEE_OTHER)) {
-				// redirect
-			String redirectUrlStr = connection.getHeaderField("Location");
-			if (Objects.isNull(redirectUrlStr) || redirectUrlStr.isEmpty()) {
-				throw new IOException("Empty redirect URL response");
-			}
-			URL redirectUrl;
-			try {
-				redirectUrl = new URL(redirectUrlStr);
-			} catch(Exception ex) {
-				throw new IOException("Invalid redirect URL", ex);
-			}
-			if (!redirectUrl.getProtocol().toLowerCase().startsWith("http")) {
-				throw new IOException("Invalid redirect protocol");
-			}
-			if (!WHITE_LIST.contains(redirectUrl.getHost())) {
-				throw new IOException("Invalid redirect host - not on the allowed 'white list'");
-			}
-			connection = (HttpURLConnection)redirectUrl.openConnection();
-		}
-		return connection.getInputStream();
+
+	private InputStream getUrlInputStream(final URL url) throws IOException {
+		return DownloadCache.getInstance().getUrlInputStream(url);
 	}
 
 	@Override