You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by sn...@apache.org on 2026/09/05 20:43:50 UTC
(roller) branch master updated: Normalize frontpage directory parameters before rendering the bundled theme (#169)
This is an automated email from the ASF dual-hosted git repository.
snoopdave pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/roller.git
The following commit(s) were added to refs/heads/master by this push:
new 862e4cb11 Normalize frontpage directory parameters before rendering the bundled theme (#169)
862e4cb11 is described below
commit 862e4cb11cfe7e53b83346443b40b081c887f6ef
Author: David M. Johnson <sn...@users.noreply.github.com>
AuthorDate: Sat Sep 5 16:43:45 2026 -0400
Normalize frontpage directory parameters before rendering the bundled theme (#169)
* 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
* Validate frontpage directory handles before lookup
---
.../ui/rendering/model/UtilitiesModel.java | 9 +
.../main/webapp/themes/frontpage/_blogdirectory.vm | 18 +-
app/src/main/webapp/themes/frontpage/directory.vm | 18 +-
.../roller/weblogger/business/WeblogStatsTest.java | 12 +-
.../velocity/FrontpageDirectoryRenderingTest.java | 233 +++++++++++++++++++++
5 files changed, 276 insertions(+), 14 deletions(-)
diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/model/UtilitiesModel.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/model/UtilitiesModel.java
index a27a4a36a..89f1ddc92 100644
--- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/model/UtilitiesModel.java
+++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/model/UtilitiesModel.java
@@ -20,6 +20,7 @@ package org.apache.roller.weblogger.ui.rendering.model;
import java.text.SimpleDateFormat;
import java.util.Date;
+import java.util.Locale;
import java.util.Map;
import java.util.TimeZone;
import java.util.regex.Matcher;
@@ -202,6 +203,14 @@ public class UtilitiesModel implements Model {
public String left(String str, int length) {
return StringUtils.left(str, length);
}
+
+ /**
+ * Upper-case a value using a locale-independent mapping for templates
+ * that validate protocol or identifier keys.
+ */
+ public String toUpperCase(String str) {
+ return str == null ? null : str.toUpperCase(Locale.ROOT);
+ }
public String escapeHTML(String str) {
return StringEscapeUtils.escapeHtml4(str);
diff --git a/app/src/main/webapp/themes/frontpage/_blogdirectory.vm b/app/src/main/webapp/themes/frontpage/_blogdirectory.vm
index 3f65a22d9..ef77349c9 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)
+ #set($candidateLetter = $utils.toUpperCase($requestedLetter))
+ #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
@@ -73,4 +79,4 @@
</td>
</tr>
</tfoot>
- </table>
\ No newline at end of file
+ </table>
diff --git a/app/src/main/webapp/themes/frontpage/directory.vm b/app/src/main/webapp/themes/frontpage/directory.vm
index 49917397a..8892bdc19 100644
--- a/app/src/main/webapp/themes/frontpage/directory.vm
+++ b/app/src/main/webapp/themes/frontpage/directory.vm
@@ -31,10 +31,20 @@
<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 && $requestedHandle.matches("[A-Za-z0-9_]+"))
+ #set($profileWeblog = $site.getWeblog($requestedHandle))
+ #end
+ #if($profileWeblog)
+ #set($resolvedFirst = $utils.left($profileWeblog.handle,1))
+ #if($resolvedFirst.matches("[A-Za-z]"))
+ <a href="?letter=$utils.toUpperCase($resolvedFirst)">Back to blog directory</a>
+ #else
+ <a href="?">Back to blog directory</a>
+ #end
#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 b18ac1d38..e205f3df5 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 class WeblogStatsTest {
@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
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 000000000..1ea9196fe
--- /dev/null
+++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/FrontpageDirectoryRenderingTest.java
@@ -0,0 +1,233 @@
+/*
+ * 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.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 org.apache.roller.weblogger.ui.rendering.model.UtilitiesModel;
+
+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;
+ private final String weblog;
+ StubModel(String letter) { this(letter, null); }
+ StubModel(String letter, String weblog) {
+ this.letter = letter;
+ this.weblog = weblog;
+ }
+ public String getRequestParameter(String name) {
+ if ("letter".equals(name)) { return letter; }
+ return "weblog".equals(name) ? weblog : 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 StubWeblog getWeblog(String handle) {
+ if ("alice".equals(handle) || "2blog".equals(handle)) {
+ return new StubWeblog(handle);
+ }
+ return null;
+ }
+ public List<Object> getNewWeblogs(int since, int maxResults) {
+ return new ArrayList<>();
+ }
+ }
+
+ public static class StubWeblog {
+ private final String handle;
+ StubWeblog(String handle) { this.handle = handle; }
+ public String getHandle() { return handle; }
+ }
+
+ public static class StubUrl {
+ public String getAbsoluteSite() { return "http://example.test"; }
+ public String getHome() { return "/"; }
+ public String page(String name) { return "/page/" + name; }
+ }
+
+ 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 UtilitiesModel());
+ 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("<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);
+ }
+ }
+
+ private String renderDirectory(String weblogParam) throws Exception {
+ VelocityContext ctx = new VelocityContext();
+ ctx.put("model", new StubModel(null, weblogParam));
+ ctx.put("site", new StubSite());
+ ctx.put("utils", new UtilitiesModel());
+ ctx.put("url", new StubUrl());
+ ctx.put("config", new Object() {
+ public String getSiteName() { return "Example"; }
+ public String getSiteDescription() { return "Description"; }
+ });
+ StringWriter out = new StringWriter();
+ String source = "#macro(includeTemplate $weblog $template)#end\n"
+ + "#parse('directory.vm')";
+ engine.evaluate(ctx, out, "directory-test", source);
+ return out.toString();
+ }
+
+ @Test
+ public void directoryTemplateValidatesTheWeblogParameter() throws Exception {
+ String unknown = renderDirectory("<script>alert(1)</script>");
+ assertFalse(unknown.contains("Back to blog directory"),
+ "invalid handles must not reach the weblog lookup or profile link");
+
+ String alpha = renderDirectory("alice");
+ assertTrue(alpha.contains("href=\"?letter=A\""),
+ "alphabetic resolved handles must link to their normalized directory group:\n"
+ + alpha);
+
+ String numeric = renderDirectory("2blog");
+ assertTrue(numeric.contains("href=\"?\""),
+ "numeric resolved handles must omit the letter filter:\n" + numeric);
+ }
+
+ /**
+ * 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);
+ }
+}