This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch 5.9.x-javax in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit 823fb688bd9b0bd363843bff7168fe2ef0e13de6 Author: Ben Weidig <[email protected]> AuthorDate: Wed Oct 7 08:19:06 2026 +0200 New classpath asset protection rules --- .../assets/ClasspathAssetRequestHandler.java | 42 +++++ .../org/apache/tapestry5/modules/AssetsModule.java | 97 +++++++++-- .../assets/ClasspathAssetProtectionRuleTest.java | 181 +++++++++++++++++++++ 3 files changed, 304 insertions(+), 16 deletions(-) diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetRequestHandler.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetRequestHandler.java index c93d4e5d1..a8ceadb52 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetRequestHandler.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetRequestHandler.java @@ -25,7 +25,10 @@ import org.apache.tapestry5.services.assets.AssetRequestHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; import java.io.IOException; +import java.net.URISyntaxException; +import java.net.URL; /** * A handler for asset requests for classpath assets (within a specific folder). @@ -93,10 +96,49 @@ public class ClasspathAssetRequestHandler implements AssetRequestHandler } handled = false; } + else if (isDirectory(resource)) + { + if (LOGGER.isWarnEnabled()) + { + LOGGER.warn("Blocked request for classpath asset '" + resourcePath + + "': the path resolves to a directory, not a file."); + } + handled = false; + } else { handled = path.stream(resource); } return handled; } + + /** + * Tells whether the resolved resource is a directory rather than a streamable file. A + * non-existent resource is not a directory (it is handled as a normal "not found" downstream). + */ + private static boolean isDirectory(Resource resource) + { + final URL url = resource.toURL(); + + if (url == null) + { + return false; + } + + if ("file".equalsIgnoreCase(url.getProtocol())) + { + try + { + return new File(url.toURI()).isDirectory(); + } + catch (URISyntaxException ex) + { + return new File(url.getPath()).isDirectory(); + } + } + + // Inside archives (jar:, etc.) a directory entry, when the classloader exposes one at all, + // is conventionally represented with a trailing slash. + return url.getPath().endsWith("/"); + } } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/modules/AssetsModule.java b/tapestry-core/src/main/java/org/apache/tapestry5/modules/AssetsModule.java index e634285c8..264c96558 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/modules/AssetsModule.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/modules/AssetsModule.java @@ -12,6 +12,8 @@ package org.apache.tapestry5.modules; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -76,6 +78,7 @@ import org.apache.tapestry5.services.ClasspathAssetAliasManager; import org.apache.tapestry5.services.ClasspathAssetProtectionRule; import org.apache.tapestry5.services.ClasspathProvider; import org.apache.tapestry5.services.ComponentClassResolver; +import org.apache.tapestry5.services.ContextAssetProtectionRule; import org.apache.tapestry5.services.ContextProvider; import org.apache.tapestry5.services.Core; import org.apache.tapestry5.services.assets.AssetChecksumGenerator; @@ -419,23 +422,79 @@ public class AssetsModule { return chainBuilder.build(ClasspathAssetProtectionRule.class, rules); } - + + /** + * Sensitive resource types, blocked when the resource's file name ends with the (lower-case) suffix. + */ + private static final Map<String, String> BLOCKED_FILE_SUFFIXES = blockedFileSuffixes(); + + private static Map<String, String> blockedFileSuffixes() + { + Map<String, String> map = new LinkedHashMap<>(22); // = 16 elements / 0.75 default load factor + map.put("ClassFile", ".class"); + map.put("PropertiesFile", ".properties"); + map.put("XMLFile", ".xml"); + map.put("TemplateFile", ".tml"); + map.put("MFFile", ".mf"); + map.put("PEMFile", ".pem"); + map.put("JKSFile", ".jks"); + map.put("P12File", ".p12"); + map.put("P8File", ".p8"); + map.put("PKCS8File", ".pkcs8"); + map.put("ASCFile", ".asc"); + map.put("GPGFile", ".gpg"); + map.put("PGPFile", ".pgp"); + map.put("KeystoreFile", ".keystore"); + map.put("KeyFile", ".key"); + map.put("PFXFile", ".pfx"); + return Collections.unmodifiableMap(map); + } + + /** + * Sensitive resources, blocked by the complete (lower-case) resource's file name. + */ + private static final Map<String, String> BLOCKED_FILE_NAMES = blockedFileNames(); + + private static Map<String, String> blockedFileNames() + { + Map<String, String> map = new LinkedHashMap<>(); + map.put("DockerFile", "dockerfile"); + map.put("IdRSAFile", "id_rsa"); + map.put("IdEd25519File", "id_ed25519"); + map.put("IdEcdsaFile", "id_ecdsa"); + return Collections.unmodifiableMap(map); + } public static void contributeClasspathAssetProtectionRule( - OrderedConfiguration<ClasspathAssetProtectionRule> configuration) + OrderedConfiguration<ClasspathAssetProtectionRule> configuration, + @Symbol(TapestryHttpSymbolConstants.PRODUCTION_MODE) + boolean productionMode) { - ClasspathAssetProtectionRule classFileRule = (s) -> s.toLowerCase().endsWith(".class"); - configuration.add("ClassFile", classFileRule); - ClasspathAssetProtectionRule propertiesFileRule = (s) -> s.toLowerCase().endsWith(".properties"); - configuration.add("PropertiesFile", propertiesFileRule); - ClasspathAssetProtectionRule xmlFileRule = (s) -> s.toLowerCase().endsWith(".xml"); - configuration.add("XMLFile", xmlFileRule); - ClasspathAssetProtectionRule tmlFileRule = (s) -> s.toLowerCase().endsWith(".tml"); - configuration.add("TemplateFile", tmlFileRule); - ClasspathAssetProtectionRule folderRule = (s) -> isFolderToBlock(s); - configuration.add("Folder", folderRule); + BLOCKED_FILE_SUFFIXES.forEach((id, suffix) -> + configuration.add(id, (s) -> lowerCaseLastSegment(s).endsWith(suffix))); + + // Block all filenames (and last-segment folders) starting with a dot. + configuration.add("Dotfile", (s) -> lowerCaseLastSegment(s).startsWith(".")); + + BLOCKED_FILE_NAMES.forEach((id, blocked) -> + configuration.add(id, (s) -> lowerCaseLastSegment(s).equals(blocked))); + + // Requests that resolve to a directory (rather than a file) are refused directly by + // ClasspathAssetRequestHandler, which can inspect the actual resolved Resource. + // A path string alone cannot distinguish a folder from an extension-less file here. + + // TAP5-2835: Don't serve source maps in production + if (productionMode) + { + ClasspathAssetProtectionRule mapFileRule = (s) -> lowerCaseLastSegment(s).endsWith(".map"); + configuration.add("MapFile", mapFileRule); + } } - - final private static boolean isFolderToBlock(String path) + + /** + * Returns the last path segment (the file name), lower-cased, with any backslashes normalized + * to forward slashes first. + */ + private static String lowerCaseLastSegment(String path) { path = path.replace('\\', '/'); final int lastIndex = path.lastIndexOf('/'); @@ -443,7 +502,13 @@ public class AssetsModule { path = path.substring(lastIndex); } - return !path.contains("."); + return path.toLowerCase(); + } + + @Primary + public static ContextAssetProtectionRule buildContextAssetProtectionRule( + List<ContextAssetProtectionRule> rules, ChainBuilder chainBuilder) + { + return chainBuilder.build(ContextAssetProtectionRule.class, rules); } - } diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetProtectionRuleTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetProtectionRuleTest.java new file mode 100644 index 000000000..9c8e25ed4 --- /dev/null +++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/assets/ClasspathAssetProtectionRuleTest.java @@ -0,0 +1,181 @@ +// Copyright 2026 The Apache Software Foundation +// +// 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.apache.tapestry5.internal.services.assets; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.tapestry5.commons.OrderedConfiguration; +import org.apache.tapestry5.modules.AssetsModule; +import org.apache.tapestry5.services.ClasspathAssetProtectionRule; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Exercises every rule contributed by + * {@link AssetsModule#contributeClasspathAssetProtectionRule}. + * Rather than re-implementing the rules, it captures the real contributions and composes them + * exactly as the service does ("blocked if any rule says so"), so the assertions can never + * drift from the production logic. + */ +class ClasspathAssetProtectionRuleTest +{ + /** + * Builds the real, composed protection rule for the given mode. Also asserts that no two + * contributions share an id (building throws otherwise). Package-visible so the handler test + * can reuse the real rules instead of a mirror. + */ + static ClasspathAssetProtectionRule realRule(boolean productionMode) + { + final Map<String, ClasspathAssetProtectionRule> rules = new LinkedHashMap<>(); + + OrderedConfiguration<ClasspathAssetProtectionRule> configuration = + new OrderedConfiguration<ClasspathAssetProtectionRule>() + { + @Override + public void add(String id, ClasspathAssetProtectionRule object, String... constraints) + { + if (rules.put(id, object) != null) + { + throw new IllegalStateException("Duplicate contribution id: " + id); + } + } + + @Override + public void override(String id, ClasspathAssetProtectionRule object, String... constraints) + { + throw new UnsupportedOperationException(); + } + + @Override + public void addInstance(String id, Class<? extends ClasspathAssetProtectionRule> clazz, String... constraints) + { + throw new UnsupportedOperationException(); + } + + @Override + public void overrideInstance(String id, Class<? extends ClasspathAssetProtectionRule> clazz, String... constraints) + { + throw new UnsupportedOperationException(); + } + }; + + AssetsModule.contributeClasspathAssetProtectionRule(configuration, productionMode); + + return path -> rules.values().stream().anyMatch(rule -> rule.block(path)); + } + + private static final ClasspathAssetProtectionRule PRODUCTION = realRule(true); + + private static final ClasspathAssetProtectionRule DEVELOPMENT = realRule(false); + + /** + * Covers all extension rules, the dot-file rule, every exact file-name rule (case-insensitively), + * and path-separator independence. + */ + @ParameterizedTest + @ValueSource(strings = { + // --- extension rules (BLOCKED_FILE_SUFFIXES) --- + "com/example/Service.class", + "com/example/messages.properties", + "com/example/config.xml", + "com/example/page.tml", + "META-INF/MANIFEST.MF", // .mf, matched case-insensitively + "certs/server.pem", + "certs/store.jks", + "certs/cert.p12", + "certs/key.p8", + "certs/key.pkcs8", + "certs/signature.asc", + "certs/secret.gpg", + "certs/secret.pgp", + "certs/app.keystore", + "certs/server.key", + "certs/cert.pfx", + // --- dot-file rule (must match on the file name, not just the root) --- + "config/.env", + "a/b/.gitignore", + ".npmrc", + // --- exact file-name rules (BLOCKED_FILE_NAMES), case-insensitive --- + "build/Dockerfile", + "build/DOCKERFILE", + "ssh/id_rsa", + "ssh/id_ed25519", + "ssh/id_ecdsa", + "ID_RSA", + // --- path-separator independence (backslashes) --- + "config\\server.pem", + "build\\Dockerfile", + }) + void blockedInEveryMode(String path) + { + assertTrue(PRODUCTION.block(path), "should be blocked (production mode): " + path); + assertTrue(DEVELOPMENT.block(path), "should be blocked (development mode): " + path); + } + + /** + * Guard against over-blocking: ordinary web assets, and near-misses of the sensitive rules, + * must still be served. Notably {@code id_rsa.pub} (a public key) is not blocked, because the + * name rules match exactly rather than by prefix. + * Extension-less names are no longer blocked by any rule (whether such a path is a directory + * is decided by the request handler at stream time, not here). + */ + @ParameterizedTest + @ValueSource(strings = { + "assets/app.js", + "assets/app.min.js", + "styles/site.css", + "img/logo.png", + "fonts/font.woff2", + "data/config.json", + "docs/readme.txt", + "keys/id_rsa.pub", // public key: exact-match rule must NOT block it + "build/Dockerfile.template", // not an exact "dockerfile" match + "docs/LICENSE", // extension-less file: no longer blocked by a rule + "META-INF/services", // extension-less name (folder-ness decided at stream time) + "bin/run", // extension-less file + }) + void allowedInEveryMode(String path) + { + assertFalse(PRODUCTION.block(path), "should be served (production mode): " + path); + assertFalse(DEVELOPMENT.block(path), "should be served (development mode): " + path); + } + + /** + * Source maps are the one mode-dependent rule: blocked in production, served in development. + */ + @Test + void sourceMapsBlockedOnlyInProduction() + { + assertTrue(PRODUCTION.block("assets/app.js.map"), "source map must be blocked in production"); + assertFalse(DEVELOPMENT.block("assets/app.js.map"), "source map should be served in development"); + } + + /** + * Building the chain throws on a duplicate contribution id, so a successful build is the + * assertion that all ids are distinct. + */ + @Test + void contributionIdsAreUnique() + { + assertNotNull(realRule(true)); + assertNotNull(realRule(false)); + } +}
