Regression in 4.5.5: SpringTemplateLoader rejects valid classpath tem…

handlebars-springmvc/src/main/java/com/github/jknack/handlebars/springmvc/SpringTemplateLoader.java

@@ -17,6 +17,7 @@
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
+import org.springframework.util.StringUtils;
 
 import com.github.jknack.handlebars.io.URLTemplateLoader;
 
@@ -58,6 +59,34 @@ public SpringTemplateLoader(final ApplicationContext applicationContext) {
 
   @Override
   protected URL getResource(final String location) throws IOException {
+    String prefix = getPrefix();
+
+    // Classpath resources: Bypass OS-level canonicalization entirely.
+    // The ClassLoader securely handles percent-encoding (returning 404),
+    // so logical path validation is 100% secure and naturally supports multi-root classpaths.
+    if (prefix.startsWith(ResourceLoader.CLASSPATH_URL_PREFIX)) {
+      String normalizedLocation = StringUtils.cleanPath(location);
+      String normalizedPrefix = StringUtils.cleanPath(prefix);
+      if (!normalizedPrefix.endsWith("/")) {
+        normalizedPrefix += "/";
+      }
+
+      if (!normalizedLocation.startsWith(normalizedPrefix)) {
+        throw new IllegalArgumentException(
+            "Path traversal attempt detected. Logical path escapes classpath prefix: " + location);
+      }
+
+      Resource resource = loader.getResource(location);
+      if (!resource.exists()) {
+        return null;
+      }
+
+      URL url = resource.getURL();
+      validateNoUnsafeUrlComponents(url);
+      return url;
+    }
+
+    // File-system / Servlet resources: strict OS-level validation
     Resource resource = loader.getResource(location);
     if (!resource.exists()) {
       return null;
@@ -83,7 +112,7 @@ protected URL getResource(final String location) throws IOException {
                 + location);
       }
     } else {
-      // Fallback containment for Classpath/URL resources
+      // Fallback containment for custom URL resources (e.g., vfs:, jndi:)
       String decodedPath = URLDecoder.decode(url.getPath(), StandardCharsets.UTF_8);
 
       // Archive URLs embed the archive location before a '!' separator
@@ -130,9 +159,6 @@ protected URL getResource(final String location) throws IOException {
 
   /**
    * Resolves the configured prefix to a real directory so file resources can be checked against it.
-   * A classpath: prefix resolves through the resource loader to the actual classpath directory
-   * (e.g. target/classes/templates), unlike a literal strip which would point at the filesystem
-   * root.
    *
    * @return The canonical base directory for the configured prefix.
    * @throws IOException If the prefix resource cannot be resolved.

handlebars-springmvc/src/test/java/com/github/jknack/handlebars/springmvc/SpringTemplateLoaderTest.java

@@ -11,6 +11,7 @@
 
 import java.io.IOException;
 import java.net.URL;
+import java.net.URLClassLoader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
@@ -75,54 +76,31 @@ public void classpathPrefixAcceptsJarEntryUrls() throws IOException {
   }
 
   @Test
-  public void shouldBlockContainsBypassInClasspath() throws IOException {
-    Resource mockResource = mock(Resource.class);
-    when(mockResource.exists()).thenReturn(true);
-    when(mockResource.isFile()).thenReturn(false);
-
-    // Simulate an attacker traversing out, but targeting a file inside another directory
-    // that happens to include the prefix string ("/templates/") in its path.
-    URL maliciousUrl = new URL("jar:file:/app.jar!/com/myapp/secret/templates/keys.hbs");
-    when(mockResource.getURL()).thenReturn(maliciousUrl);
-
-    // Spring resolves the traversal logically before Handlebars gets it
-    when(resourceLoader.getResource(
-            "classpath:/templates/../../com/myapp/secret/templates/keys.hbs"))
-        .thenReturn(mockResource);
-
+  public void shouldBlockContainsBypassInClasspath() {
+    // The logical check happens before resource resolution, so we don't need mock behavior.
+    // It catches the traversal on the logical string level immediately.
     IllegalArgumentException exception =
         assertThrows(
             IllegalArgumentException.class,
             () -> {
               templateLoader.sourceAt("../../com/myapp/secret/templates/keys");
             });
 
-    assertTrue(exception.getMessage().contains("escapes Spring base prefix"));
+    assertTrue(exception.getMessage().contains("Logical path escapes classpath prefix"));
   }
 
   @Test
-  public void shouldEnforceLogicalBoundaryAgainstTraversal() throws IOException {
-    Resource mockResource = mock(Resource.class);
-    when(mockResource.exists()).thenReturn(true);
-    when(mockResource.isFile()).thenReturn(false);
-
-    URL maliciousUrl = new URL("file:///application.properties.hbs");
-    when(mockResource.getURL()).thenReturn(maliciousUrl);
-
-    when(resourceLoader.getResource("classpath:/templates/../../application.properties.hbs"))
-        .thenReturn(mockResource);
-
-    String maliciousPath = "../../application.properties";
-
+  public void shouldEnforceLogicalBoundaryAgainstTraversal() {
+    // The logical check happens before resource resolution, so we don't need mock behavior.
+    // It catches the traversal on the logical string level immediately.
     IllegalArgumentException exception =
         assertThrows(
             IllegalArgumentException.class,
             () -> {
-              // sourceAt triggers resolve() and then getResource()
-              templateLoader.sourceAt(maliciousPath);
+              templateLoader.sourceAt("../../application.properties");
             });
 
-    assertTrue(exception.getMessage().contains("escapes Spring base"));
+    assertTrue(exception.getMessage().contains("Logical path escapes classpath prefix"));
   }
 
   @Test
@@ -223,11 +201,40 @@ public void shouldBlockPartialDirectoryMatch() throws IOException {
 
     // Use "../" to back out of the Handlebars-enforced slash, traversing into the sibling
     // directory.
-    // Handlebars resolves this to: .../templates/../templates-secret/secret.hbs
     IllegalArgumentException exception =
         assertThrows(
             IllegalArgumentException.class, () -> loader.sourceAt("../templates-secret/secret"));
 
     assertTrue(exception.getMessage().contains("escapes Spring base"));
   }
+
+  @Test
+  public void shouldResolveTemplatesAcrossMultiModuleClasspath() throws IOException {
+    Path root = Files.createTempDirectory("handlebars-multimodule");
+
+    // Simulate Module A (e.g., target/classes/)
+    Path moduleA = Files.createDirectory(root.resolve("moduleA"));
+    Path templatesA = Files.createDirectory(moduleA.resolve("templates"));
+    Files.writeString(templatesA.resolve("template-a.hbs"), "Template A");
+
+    // Simulate Module B (e.g., target/test-classes/ or a sibling module)
+    Path moduleB = Files.createDirectory(root.resolve("moduleB"));
+    Path templatesB = Files.createDirectory(moduleB.resolve("templates"));
+    Files.writeString(templatesB.resolve("template-b.hbs"), "Template B");
+
+    // Create a ClassLoader containing both roots
+    URLClassLoader classLoader =
+        new URLClassLoader(
+            new URL[] {moduleA.toUri().toURL(), moduleB.toUri().toURL()},
+            getClass().getClassLoader());
+
+    // Use Spring's DefaultResourceLoader backed by our multi-root classloader
+    SpringTemplateLoader loader = new SpringTemplateLoader(new DefaultResourceLoader(classLoader));
+    loader.setPrefix("classpath:/templates/");
+    loader.setSuffix(".hbs");
+
+    // The loader should find templates in both roots flawlessly without canonical file mismatches
+    assertNotNull(loader.sourceAt("template-a"));
+    assertNotNull(loader.sourceAt("template-b"));
+  }
 }