Normalize frontpage directory parameters before rendering the bundled theme

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
     <div class="letterMap">
     <p>
     #set($firstLetterDone = 0)
@@ -22,7 +28,7 @@
     </div>
 
     #if($chosenLetter)
-       <h2 class="pageTitle">Weblogs starting with $chosenLetter</h2>
+       <h2 class="pageTitle">Weblogs starting with $utils.escapeHTML($chosenLetter)</h2>
     #else
        <h2 class="pageTitle">All weblogs</h2>
     #end

app/src/main/webapp/themes/frontpage/directory.vm

@@ -31,10 +31,15 @@
 
             <div id="tabContent">
                 <div id="directory">
-                #if($model.getRequestParameter("weblog"))
-                    #set($handle = $model.getRequestParameter("weblog"))
-                    <a href="?letter=$utils.left($handle,1)">Back to blog directory</a>
-                    #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)
+                    <a href="?letter=$utils.escapeHTML($utils.left($profileWeblog.handle,1))">Back to blog directory</a>
                     #includeTemplate($model.weblog "_blogprofile")
                 #else                 
                    #set($pageLength = $maxResults)

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<String, Long> map = mgr.getWeblogHandleLetterMap();    
-        assertNotNull(map.get("A"));
-        assertNotNull(map.get("B"));
-        assertNotNull(map.get("C"));
+        Map<String, Long> 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

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
+ * <code>letter</code> parameter.
+ *
+ * <p>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<Object> 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<String, Long> getWeblogHandleLetterMap() {
+            Map<String, Long> 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("&", "&amp;").replace("<", "&lt;")
+                    .replace(">", "&gt;").replace("\"", "&quot;");
+        }
+        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>alert(1)</script>", // script payload
+                "\" onmouseover=\"alert(1)", // attribute-breaking payload
+                "A<b>",                     // 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("<script") || html.contains("&lt;script"),
+                    "rejected value [" + value + "] reached the page, raw or encoded:\n" + html);
+            assertFalse(html.contains("onmouseover"),
+                    "rejected value [" + value + "] leaked an event handler:\n" + html);
+        }
+    }
+
+    /**
+     * The sibling directory template resolves a weblog handle from the query
+     * string. It cannot be rendered standalone here because it pulls in other
+     * templates through #includeTemplate, so this is a structural check: the
+     * link must be built from the resolved weblog rather than the raw
+     * parameter, and escaped at output.
+     */
+    @Test
+    public void directoryTemplateValidatesTheWeblogParameter() throws Exception {
+        String vm = new String(Files.readAllBytes(Paths.get(THEME_DIR, "directory.vm")),
+                StandardCharsets.UTF_8);
+        assertFalse(vm.contains("$utils.left($handle,1)"),
+                "the back-link must not be built from the raw weblog parameter:\n" + vm);
+        assertTrue(vm.contains("$site.getWeblog($requestedHandle)"),
+                "the requested handle must be resolved before use:\n" + vm);
+        assertTrue(vm.contains("$utils.escapeHTML($utils.left($profileWeblog.handle,1))"),
+                "the back-link must escape the resolved handle:\n" + vm);
+    }
+
+    /**
+     * Guards the test itself: if the template stopped rendering, or the theme
+     * moved, every assertion above would pass or fail for the wrong reason.
+     */
+    @Test
+    public void templateActuallyRenders() throws Exception {
+        String html = render("A");
+        assertTrue(html.contains("blogdirectory"),
+                "expected the directory table to render:\n" + html);
+        assertTrue(html.contains("letterMap"),
+                "expected the A-Z letter map to render:\n" + html);
+    }
+}