From 57970f425a42ca648fa1a90d887bc56b66a489e2 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 29 Aug 2026 09:28:24 -0400 Subject: [PATCH] Normalize frontpage directory parameters before rendering the bundled theme The bundled frontpage directory filters its listing by a letter parameter. It now accepts that parameter only when it matches one of the directory's own A-Z keys, falling back to the full listing otherwise (as a missing parameter already did), and HTML-escapes it in the heading. The sibling directory template resolves its weblog handle before use and builds the back-link from the resolved weblog rather than the request parameter. Adds FrontpageDirectoryRenderingTest and tightens the weblog letter-map test to assert the complete A-Z key set the template now depends on. Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV --- .../webapp/themes/frontpage/_blogdirectory.vm | 16 +- .../main/webapp/themes/frontpage/directory.vm | 13 +- .../weblogger/business/WeblogStatsTest.java | 12 +- .../FrontpageDirectoryRenderingTest.java | 209 ++++++++++++++++++ 4 files changed, 237 insertions(+), 13 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/FrontpageDirectoryRenderingTest.java diff --git a/app/src/main/webapp/themes/frontpage/_blogdirectory.vm b/app/src/main/webapp/themes/frontpage/_blogdirectory.vm index 3f65a22d9e..3ebb9274ad 100644 --- a/app/src/main/webapp/themes/frontpage/_blogdirectory.vm +++ b/app/src/main/webapp/themes/frontpage/_blogdirectory.vm @@ -1,8 +1,14 @@ -#if($model.getRequestParameter("letter")) - #set($chosenLetter = $model.getRequestParameter("letter")) - #end +#set($weblogLetterMap = $site.getWeblogHandleLetterMap()) - #set($weblogLetterMap = $site.getWeblogHandleLetterMap()) + ## Accept only a known A-Z key; otherwise render the full listing, exactly + ## as a missing parameter does. + #set($requestedLetter = $model.getRequestParameter("letter")) + #if($requestedLetter && $requestedLetter.length() == 1) + #set($candidateLetter = $requestedLetter.toUpperCase()) + #if($weblogLetterMap.containsKey($candidateLetter)) + #set($chosenLetter = $candidateLetter) + #end + #end

#set($firstLetterDone = 0) @@ -22,7 +28,7 @@

#if($chosenLetter) -

Weblogs starting with $chosenLetter

+

Weblogs starting with $utils.escapeHTML($chosenLetter)

#else

All weblogs

#end diff --git a/app/src/main/webapp/themes/frontpage/directory.vm b/app/src/main/webapp/themes/frontpage/directory.vm index 49917397aa..08f96cc988 100644 --- a/app/src/main/webapp/themes/frontpage/directory.vm +++ b/app/src/main/webapp/themes/frontpage/directory.vm @@ -31,10 +31,15 @@
- #if($model.getRequestParameter("weblog")) - #set($handle = $model.getRequestParameter("weblog")) - Back to blog directory - #set($profileWeblog = $site.getWeblog($handle)) + ## Render the profile only for a weblog that exists, and build + ## the back-link from the resolved weblog's own handle. + #set($profileWeblog = false) + #set($requestedHandle = $model.getRequestParameter("weblog")) + #if($requestedHandle) + #set($profileWeblog = $site.getWeblog($requestedHandle)) + #end + #if($profileWeblog) + Back to blog directory #includeTemplate($model.weblog "_blogprofile") #else #set($pageLength = $maxResults) diff --git a/app/src/test/java/org/apache/roller/weblogger/business/WeblogStatsTest.java b/app/src/test/java/org/apache/roller/weblogger/business/WeblogStatsTest.java index b18ac1d38c..e205f3df59 100644 --- a/app/src/test/java/org/apache/roller/weblogger/business/WeblogStatsTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/business/WeblogStatsTest.java @@ -122,10 +122,14 @@ public void testGetUserNameLetterMap() throws Exception { @Test public void testGetWeblogLetterMap() throws Exception { WeblogManager mgr = WebloggerFactory.getWeblogger().getWeblogManager(); - Map map = mgr.getWeblogHandleLetterMap(); - assertNotNull(map.get("A")); - assertNotNull(map.get("B")); - assertNotNull(map.get("C")); + Map map = mgr.getWeblogHandleLetterMap(); + // The frontpage blog directory validates its letter parameter against + // these keys, so the contract is the exact A-Z set rather than a + // sample: a missing key would silently reject a legitimate letter. + assertEquals(26, map.size(), "expected the complete A-Z key set"); + for (char c = 'A'; c <= 'Z'; c++) { + assertNotNull(map.get(String.valueOf(c)), "missing key " + c); + } } @AfterEach diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/FrontpageDirectoryRenderingTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/FrontpageDirectoryRenderingTest.java new file mode 100644 index 0000000000..2c3103c3d0 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/FrontpageDirectoryRenderingTest.java @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * 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. For additional + * information regarding copyright in this work, please see the NOTICE + * file in the top level directory of this distribution. + */ +package org.apache.roller.weblogger.ui.rendering.velocity; + +import java.io.StringWriter; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; + +import org.apache.velocity.VelocityContext; +import org.apache.velocity.app.VelocityEngine; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Renders the bundled frontpage blog-directory template against the real + * Velocity engine and asserts how it treats the caller-supplied + * letter parameter. + * + *

The template is reached anonymously, so the parameter is untrusted. The + * contract is that only a value which normalizes to one of the directory's own + * A-Z keys is used, and that anything else falls back to the complete directory + * without the rejected value appearing in the response in any form — raw, + * HTML-encoded, or URL-encoded. + */ +public class FrontpageDirectoryRenderingTest { + + private static final String THEME_DIR = "src/main/webapp/themes/frontpage"; + private static final String TEMPLATE = "_blogdirectory.vm"; + + private static VelocityEngine engine; + + @BeforeAll + public static void setUpEngine() { + Properties props = new Properties(); + props.setProperty("resource.loaders", "file"); + props.setProperty("resource.loader.file.class", + "org.apache.velocity.runtime.resource.loader.FileResourceLoader"); + props.setProperty("resource.loader.file.path", THEME_DIR); + engine = new VelocityEngine(); + engine.init(props); + } + + /** Minimal stand-ins for the model objects the template reads. */ + public static class StubModel { + private final String letter; + StubModel(String letter) { this.letter = letter; } + public String getRequestParameter(String name) { + return "letter".equals(name) ? letter : null; + } + } + + public static class StubPager { + public List getItems() { return new ArrayList<>(); } + public String prevLink() { return null; } + public String nextLink() { return null; } + public String prevName() { return null; } + public String nextName() { return null; } + } + + public static class StubSite { + public Map getWeblogHandleLetterMap() { + Map map = new LinkedHashMap<>(); + for (char c = 'A'; c <= 'Z'; c++) { + map.put(String.valueOf(c), 1L); + } + return map; + } + public StubPager getWeblogsByLetterPager(String letter, int offset, int length) { + return new StubPager(); + } + } + + public static class StubUtils { + public String escapeHTML(String str) { + return str == null ? null : str.replace("&", "&").replace("<", "<") + .replace(">", ">").replace("\"", """); + } + public String left(String str, int len) { + if (str == null) { return null; } + return str.length() <= len ? str : str.substring(0, len); + } + } + + public static class StubUrl { + public String getAbsoluteSite() { return "http://example.test"; } + } + + private String render(String letterParam) throws Exception { + VelocityContext ctx = new VelocityContext(); + ctx.put("model", new StubModel(letterParam)); + ctx.put("site", new StubSite()); + ctx.put("utils", new StubUtils()); + ctx.put("url", new StubUrl()); + ctx.put("pageLength", 30); + StringWriter out = new StringWriter(); + engine.mergeTemplate(TEMPLATE, "UTF-8", ctx, out); + return out.toString(); + } + + @Test + public void missingLetterRendersCompleteDirectory() throws Exception { + String html = render(null); + assertTrue(html.contains("All weblogs"), + "a missing letter must render the complete directory:\n" + html); + assertFalse(html.contains("Weblogs starting with"), + "a missing letter must not render a filtered heading"); + } + + @Test + public void validUppercaseLetterIsAccepted() throws Exception { + String html = render("A"); + assertTrue(html.contains("Weblogs starting with A"), + "a valid key must be accepted:\n" + html); + } + + @Test + public void lowercaseLetterNormalizesToTheSameGroup() throws Exception { + assertTrue(render("a").contains("Weblogs starting with A"), + "lowercase input must normalize to the uppercase key"); + } + + /** + * Every value that is not a single A-Z key must be discarded outright and + * must not be echoed, raw or encoded. + */ + @Test + public void invalidValuesFallBackAndAreNotEchoed() throws Exception { + String[] rejected = { + "AB", // multi-character + "1", // numeric + "!", // punctuation + "é", // non-ASCII + "", // script payload + "\" onmouseover=\"alert(1)", // attribute-breaking payload + "A", // valid prefix, invalid remainder + }; + for (String value : rejected) { + String html = render(value); + assertTrue(html.contains("All weblogs"), + "rejected value [" + value + "] must fall back to the complete " + + "directory:\n" + html); + // Assert against the heading directly. A bare contains(value) would + // match incidentally: single characters such as "1" occur naturally + // in the rendered letter counts. + assertFalse(html.contains("Weblogs starting with"), + "rejected value [" + value + "] produced a filtered heading:\n" + html); + assertFalse(html.contains("