ResourceHttpRequestHandler throwing IllegalArgumentException if resource doesn't end with slash breaks some third-party libraris

[philwebb]

Originally raised in spring-projects/spring-boot#42929 by @milazzo-g. This looks like a problem introduced by #33712, but I'm not sure if the fix should be in framework or the third-party library.

---

During context loading the ResourceHttpRequestHandler class scans all possible paths and if these do not end with "/" an IllegalArgumentException type exception is thrown with message "Resource location does not end with slash: …" (see ResourceHttpRequestHandler@282 the call to ResourceHttpRequestHandler::assertLocationPath) this cause several issue with various jar included (in our case camunda-webapp-webjar 7.22.0 - included by camunda-bpm-spring-boot-starter-root 7.22.0).

The path provided by the library is "classpath:/META-INF/resources/webjars/camunda" which as you can see does not contain the final "/". makes it unusable with spring-boot 6.2.0-RC3.

We temporary fixed creating a copy of org.springframework.web.servlet.resource.ResourceHttpRequestHandler in our src folder and modifying starting from line 282 from:

ResourceHandlerUtils.assertLocationPath(location);

to:

try {
 ResourceHandlerUtils.assertLocationPath(location);
} catch (Exception e) {
 if (org.apache.commons.lang3.StringUtils.containsIgnoreCase(e.getMessage(), "resource location does not end with slash")){
  location += "/";
 }else {
  throw e;
 }
}

this solved the issue. Could you please fix it in a next minor?

KR.

Giuseppe.

STACKTRACE.txt

[milazzo-g]

Hi @jhoeller, IMHO I thik that this kind of issue should be fixed (or handled) by framework otherwise, waiting for the update of all other possible third part libraries can cause the biblical time to make the new version usable easily.

I totally agree about notNull, but killing everything for a "/" that can easily be handled by code (perhaps logging a WARN) seems excessive to me.

KR.

Giuseppe.

[bclozel]

Thanks for raising this @milazzo-g @cagliostro92 and sorry for the inconvenience. This is the intent behind #33712.

Previous behavior

Let's take an example application here, a simple Spring Boot application that contributes the following:

import org.springframework.context.annotation.Configuration;
import org.springframework.web.servlet.config.annotation.ResourceHandlerRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

@Configuration(proxyBeanMethods = false)
public class WebConfig implements WebMvcConfigurer {

	@Override
	public void addResourceHandlers(ResourceHandlerRegistry registry) {
		registry.addResourceHandler("/**").addResourceLocations("classpath:/META-INF/resources/webjars/camunda");
	}
}

with the following structure under src/main/resources

tree
.
├── META-INF
│   └── resources
│       └── webjars
│           ├── camunda
│           │   └── camunda.txt
│           └── other
│               └── other.txt
├── application.properties
├── static
└── templates

8 directories, 3 files

Here are a few requests showing the previous behavior in this situation:

The intended configuration doesn't work:

http :8080/camunda.txt
HTTP/1.1 404

You'll need to change the request path to make it work:

http :8080/camunda/camunda.txt
HTTP/1.1 200

You can't resolve other resources because Spring prevents escaping the configured location:

http :8080/other/other.txt
HTTP/1.1 404

See logs:

o.s.w.s.resource.PathResourceResolver    : "Resource path "other/other.txt" was successfully resolved but resource "class path resource [META-INF/resources/webjars/other/other.txt]" is neither under the current location "class path resource [META-INF/resources/webjars/camunda]" nor under any of the allowed locations [class path resource [META-INF/resources/webjars/camunda]]"

If we were to try the same using the Spring Boot auto-configuration and configuration properties:

spring.web.resources.static-locations=classpath:/META-INF/resources/webjars/camunda

This would work because Spring Boot would automatically suffix the given location with a /.

Possible outcomes

1. Silently add a / for all configured locations

I don't think we can do that, as this would silently break existing applications. "GET /camunda/camunda.txt" would not work anymore. From our experience, adding a WARN log doesn't change the experience and developers won't act on it. This will be raised as a regression.

2. Keep rejecting such locations at startup

This is inconvenient, but this signals invalid configuration to developers and acting on it is easy enough and is likely to fix issues with the application. In the case of the Camunda starter, I guess the issue comes from here? In this case, I'm wondering if the favicon.ico file can be resolved by browsers. So this might be a bug?

I would really like to improve the upgrade experience here, but we're stuck between a silent behavior change that will break applications and a noisy (but healthy) exception during starting pointing to a likely invalid setup.