This is an automated email from the ASF dual-hosted git repository. reschke pushed a commit to branch SLING-13288 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 02a24765edcffd3dcc2a2be7b2bbe3a92d65a977 Author: Julian Reschke <[email protected]> AuthorDate: Mon Aug 10 14:12:16 2026 +0100 SLING-13288: logging of 'invalid' vanity paths - prototype --- .../impl/mapping/VanityPathHandler.java | 52 +++++++++++++++++++--- .../impl/mapping/VanityPathMapEntriesTest.java | 30 ++++++++++--- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathHandler.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathHandler.java index bd96990d..c69fa0f0 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathHandler.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathHandler.java @@ -44,6 +44,7 @@ import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.resource.path.Path; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -79,6 +80,9 @@ public class VanityPathHandler { private final Logger log = LoggerFactory.getLogger(VanityPathHandler.class); + // keep track of some dubious vanityPaths for diagnostics (thus size-limited) + private static final int MAX_REPORT_DUBIOUS_VANITY_PATHS = 50; + private final MapConfigurationProvider factory; private byte[] vanityBloomFilter; @@ -193,10 +197,12 @@ public class VanityPathHandler { try (ResourceResolver resolver = factory.getServiceResourceResolver(factory.getServiceUserAuthenticationInfo("mapping"))) { + List<String> dubiousPaths = new ArrayList<>(); + StopWatch sw = StopWatch.createStarted(); log.debug("vanity path initialization - start"); - vanityTargets = loadVanityPaths(resolver); + vanityTargets = loadVanityPaths(resolver, dubiousPaths); // process pending events VanityPathHandler.this.drain.accept("draining vanity path event queue (during cache initialization)"); @@ -228,7 +234,7 @@ public class VanityPathHandler { boolean updateTheCache = isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries(); - return null != loadVanityPath(resource, resolveMapsMap, vanityTargets, updateTheCache, true); + return null != loadVanityPath(resource, resolveMapsMap, vanityTargets, updateTheCache, true, new ArrayList<>()); } private String getMapEntryRedirect(final MapEntry mapEntry) { @@ -410,11 +416,11 @@ public class VanityPathHandler { && (this.factory.isMaxCachedVanityPathEntriesStartup() || this.isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries())) { - loadVanityPath(resource, resolveMapsMap, vanityTargets, true, true); + loadVanityPath(resource, resolveMapsMap, vanityTargets, true, true, new ArrayList<>()); entryMap = resolveMapsMap; } else { final Map<String, List<String>> targetPaths = new HashMap<>(); - loadVanityPath(resource, entryMap, targetPaths, true, false); + loadVanityPath(resource, entryMap, targetPaths, true, false, new ArrayList<>()); } } } @@ -461,11 +467,27 @@ public class VanityPathHandler { return true; } + private String diagnoseDubiousValidVanityPath(String resource, String path) { + String reason = ""; + if (path == null) { + reason = "null path"; + } else if (!path.startsWith("/")) { + reason = "not an absolute path"; + } else if (path.contains("//")) { + reason = "empty path segment"; + } + + return reason.isEmpty() ? "" : String.format("Dubious vanity on '%s', value '%s': %s", resource, path, reason); + } + /** * Load vanity paths - search for all nodes (except under /jcr:system) * having a sling:vanityPath property + * <p> + * Also collect diagnostics about vanity paths that appear to be broken. */ - private Map<String, List<String>> loadVanityPaths(ResourceResolver resolver) { + private Map<String, List<String>> loadVanityPaths( + @NotNull ResourceResolver resolver, @NotNull List<String> dubiousVanityPaths) { final Map<String, List<String>> targetPaths = new ConcurrentHashMap<>(); final String baseQueryString = "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus]" + " FROM [nt:base]" + " WHERE " @@ -496,7 +518,7 @@ public class VanityPathHandler { countInScope += 1; final boolean addToCache = isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries(); - loadVanityPath(resource, resolveMapsMap, targetPaths, addToCache, true); + loadVanityPath(resource, resolveMapsMap, targetPaths, addToCache, true, dubiousVanityPaths); } } @@ -521,6 +543,14 @@ public class VanityPathHandler { } } + // warn about dubious vanity paths + + if (dubiousVanityPaths.size() >= MAX_REPORT_DUBIOUS_VANITY_PATHS) { + log.warn("There are {} dubious vanity paths; excerpt: {}", dubiousVanityPaths.size(), dubiousVanityPaths); + } else if (!dubiousVanityPaths.isEmpty()) { + log.warn("There are {} dubious vanity paths: {}", dubiousVanityPaths.size(), dubiousVanityPaths); + } + this.vanityResourcesOnStartup.set(count); return targetPaths; @@ -543,7 +573,8 @@ public class VanityPathHandler { final Map<String, List<MapEntry>> entryMap, final Map<String, List<String>> targetPaths, boolean addToCache, - boolean updateCounter) { + boolean updateCounter, + final List<String> dubiousVanityPaths) { if (!isValidVanityPath(resource.getPath())) { return null; @@ -561,6 +592,13 @@ public class VanityPathHandler { } for (final String pVanityPath : pVanityPaths) { + + // collect vanity path values that seem to be broken + final String diagnostics = diagnoseDubiousValidVanityPath(resource.getPath(), pVanityPath); + if (!diagnostics.isEmpty() && dubiousVanityPaths.size() > MAX_REPORT_DUBIOUS_VANITY_PATHS) { + dubiousVanityPaths.add(diagnostics); + } + final String[] result = this.getVanityPathDefinition(resource.getPath(), pVanityPath); if (result != null) { // redirect target is the node providing the sling:vanityPath diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java index 05706e03..32f7e2c3 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java @@ -229,11 +229,12 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { method.invoke(mapEntries, ctx, bool); } - private static void loadVanityPaths(MapEntries mapEntries, ResourceResolver resourceResolver) + private static void loadVanityPaths(MapEntries mapEntries, ResourceResolver resourceResolver, List<String> dubious) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = VanityPathHandler.class.getDeclaredMethod("loadVanityPaths", ResourceResolver.class); + Method method = + VanityPathHandler.class.getDeclaredMethod("loadVanityPaths", ResourceResolver.class, List.class); method.setAccessible(true); - method.invoke(mapEntries.vph, resourceResolver); + method.invoke(mapEntries.vph, resourceResolver, dubious); } @Override @@ -267,6 +268,25 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { assertNotNull(vanityMap.get("/" + containerName + "/" + oneMore)); } + @Test + public void test_simple_dubious_vanity_path() { + String vanityPath = ""; + String containerName = "foo"; + String childName = "child"; + String oneMore = "one-more"; + prepareMapEntriesForVanityPath(false, false, containerName, childName, oneMore, vanityPath); + + initializeVanityPaths(); + + Map<String, List<String>> vanityMap = mapEntries.getVanityPathMappings(); + assertNotNull(vanityMap); + assertEquals( + "/" + vanityPath, + vanityMap.get("/" + containerName + "/" + childName).get(0)); + assertEquals(2, vanityMap.size()); + assertNotNull(vanityMap.get("/" + containerName + "/" + oneMore)); + } + // see SLING-12620 @Test public void test_simple_vanity_path_support_with_null_parent() { @@ -1046,7 +1066,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { } }); - loadVanityPaths(mapEntries, resourceResolver); + loadVanityPaths(mapEntries, resourceResolver, new ArrayList<>()); assertEquals(2, getVanityCounter(mapEntries).longValue()); } @@ -1068,7 +1088,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { } }); - loadVanityPaths(mapEntries, resourceResolver); + loadVanityPaths(mapEntries, resourceResolver, new ArrayList<>()); assertEquals(2, getVanityCounter(mapEntries).longValue()); }
