Full Disclosure mailing list archives

Description
The Apache Struts2 Spring Plugin opens a Spring `ApplicationContex` via a try-with-resources block and, outside of said 
block, then continues to use the closed resource, resulting in an uncaught `IllegalStateException`.  This particular 
issue prevents proper functionality and indicates poor code quality and code review standards and processes, while not 
being, itself, otherwise excessively meaningful for a production environment (as developer mode should not be enabled 
in such an environment), although it would hamper staging, test, and developer environments that are configured in 
accordance with Apache-recommended hot-reload configurations for such environments 
(https://struts.apache.org/plugins/spring/#class-reloading).

Impact
Anyone who follows Struts' documented configuration for this plugin -- enabling `struts.devMode`, setting a non-blank 
`struts.objectFactory.spring.class.reloading.watchList`, and declaring `ClassReloadingXMLWebApplicationContext` as the 
root context class in web.xml -- will trigger this pathway on every application startup.  Since close() call `null`s 
the context's internal bean factory immediately after `refresh()` succeeds, 
`SpringObjectFactory.setApplicationContext()`call to `getBeanFactory()` throws an uncaught `IllegalStateException`, 
aborting the Struts-Spring integration startup process before it can ever finish.  Even if the exception were caught, 
`close()` has already stopped the `FilesystemAlterationMonitor` watchdog thread, so the live class-reloading capability 
would never fire a single reload.  As such, the hot-reload feature cannot and does not work with this 
Apache-recommended configuration.

Examples
Sample Code
The code below clearly shows the issue when in developer, a watch list, and hot-reloading are enabled.  The 
try-with-resources calls `.close()` when exiting, against `reloadingContext`, which is merely a pointer to 
`appContext`, thereby closing `appContext`.  Immediately after exiting the try-with-resources and wrapping conditional, 
`this. setApplicationContext` is called, passing in the now closed `appContext`.  Importantly, ` 
ClassReloadingXMLWebApplicationContext`, to which `appContext` was cast during the initialisation of the 
try-with-resources and against which the `finally` that this construct generates (try-with-resources effectively being 
syntactic sugar for a `try{...} finally{reloadingContext.close()}`), has a `close()` defined that calls to 
`super.close()` before removing and stopping the listener.

```java
        String watchList = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_WATCH_LIST);
        String acceptClasses = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_ACCEPT_CLASSES);
        String reloadConfig = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_RELOAD_CONFIG);

        if ("true".equals(devMode)
                && StringUtils.isNotBlank(watchList)
                && appContext instanceof ClassReloadingXMLWebApplicationContext) {
            //prevent class caching
            useClassCache = false;

            try (ClassReloadingXMLWebApplicationContext reloadingContext = (ClassReloadingXMLWebApplicationContext) 
appContext) {
                reloadingContext.setupReloading(watchList.split(","), acceptClasses, servletContext,
                        "true".equals(reloadConfig));
                LOG.info("Class reloading is enabled. Make sure this is not used on a production environment!\n{}",
                        watchList);

                setClassLoader(reloadingContext.getReloadingClassLoader());

                // we need to reload the context, so our isntance of the factory is picked up
                reloadingContext.refresh();
            }
        }

        this.setApplicationContext(appContext);
```
Code snippet from `struts2-spring-plugin`'s 
plugins/spring/src/main/java/org/apache/struts2/spring/StrutsSpringObjectFactory.java, lines 97-120

```java
    public void close() {
        super.close();

        if (filesystemAlterationMonitor != null) {
            filesystemAlterationMonitor.removeListener(this);
            filesystemAlterationMonitor.stop();
        }
    }
```
Snippet from `struts2-spring-plugin`'s 
plugins/spring/src/main/java/org/apache/struts2/spring/ClassReloadingXMLWebApplicationContext.java, lines 157-164, 
showing the close() override invoked by the implicit try-with-resources call
_______________________________________________
Sent through the Full Disclosure mailing list
https://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: https://seclists.org/fulldisclosure/

Current thread:

• Apache Struts2 Spring Plugin Uses a Previously Closed Resource Daniel Owens via Fulldisclosure (Nov 01)