From f0c0155d468dc615c939157e95ef220f8e2be6c8 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 14 Nov 2026 22:12:50 -0500 Subject: [PATCH] Check and encode requested tags on every weblog page A custom page that is given tags= together with an entry now checks the tags against the weblog, as it already did without an entry. Tag names are HTML-encoded in the feed link titles of #showAutodiscoveryLinks and in the Gaurav and Fauxcoly tag pages. The page cache key includes the page name and the tags, so such pages get their own cache entries. --- CHANGES.md | 11 ++ .../ui/rendering/servlets/PageServlet.java | 7 +- .../rendering/util/cache/WeblogPageCache.java | 23 ++-- .../main/webapp/WEB-INF/velocity/weblog.vm | 4 +- .../main/webapp/themes/fauxcoly/tags_index.vm | 6 +- .../main/webapp/themes/gaurav/tags_index.vm | 2 +- .../util/cache/WeblogPageCacheKeyTest.java | 80 +++++++++++++ .../AutodiscoveryLinksRenderingTest.java | 112 ++++++++++++++++++ 8 files changed, 230 insertions(+), 15 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCacheKeyTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/AutodiscoveryLinksRenderingTest.java diff --git a/CHANGES.md b/CHANGES.md index ae054e20d7..354c5b4e81 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,16 @@ # Apache Roller — Changes +## 6.1.8 (in development) + +### Bug fixes + +- **Requested tags are checked and encoded on every weblog page.** A custom + page given `tags=` together with an entry now answers 404 for tags the + weblog doesn't have, as it already did without an entry. The tag names are + HTML-encoded in the feed link titles and in the Gaurav and Fauxcoly tag + pages. The page cache keys such pages by page name and tags, so they are no + longer served in place of each other. + ## 6.1.7 ### Improvements diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PageServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PageServlet.java index baf5e0948a..e7c7add962 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PageServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PageServlet.java @@ -384,7 +384,12 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) if (pageRequest.getWeblogCategory() == null) { invalid = true; } - } else if (pageRequest.getTags() != null && !pageRequest.getTags().isEmpty()) { + } + + // tags may come with any of the above, for example as a query + // parameter on a custom page that also names an entry, so they are + // checked whenever they are given + if (!invalid && pageRequest.getTags() != null && !pageRequest.getTags().isEmpty()) { try { // tags specified. make sure they exist. diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCache.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCache.java index dd54fec2ab..9edadf97c8 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCache.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCache.java @@ -173,6 +173,12 @@ public String generateKey(WeblogPageRequest pageRequest) { // may contain spaces or other bad chars String anchor = URLEncoder.encode(pageRequest.getWeblogAnchor(), StandardCharsets.UTF_8); key.append("/entry/").append(anchor); + + // a custom page can also name an entry, and must not share the + // entry's own key + if(pageRequest.getWeblogPageName() != null) { + key.append("/page/").append(pageRequest.getWeblogPageName()); + } } else { if(pageRequest.getWeblogPageName() != null) { @@ -188,14 +194,15 @@ public String generateKey(WeblogPageRequest pageRequest) { String cat = URLEncoder.encode(pageRequest.getWeblogCategoryName(), StandardCharsets.UTF_8); key.append('/').append(cat); } - - if("tags".equals(pageRequest.getContext())) { - if(pageRequest.getTags() != null && !pageRequest.getTags().isEmpty()) { - String[] tags = pageRequest.getTags().toArray(new String[0]); - Arrays.sort(tags); - key.append("/tags/").append(Utilities.stringArrayToString(tags,"+")); - } - } + } + + // tags come from the path in the tags context, or from a query + // parameter on a custom page; either way they change the page + if(pageRequest.getTags() != null && !pageRequest.getTags().isEmpty()) { + String[] tags = pageRequest.getTags().toArray(new String[0]); + Arrays.sort(tags); + String joined = Utilities.stringArrayToString(tags, "+"); + key.append("/tags/").append(URLEncoder.encode(joined, StandardCharsets.UTF_8)); } if(pageRequest.getLocale() != null) { diff --git a/app/src/main/webapp/WEB-INF/velocity/weblog.vm b/app/src/main/webapp/WEB-INF/velocity/weblog.vm index 8ff1065152..b41b05c9e4 100644 --- a/app/src/main/webapp/WEB-INF/velocity/weblog.vm +++ b/app/src/main/webapp/WEB-INF/velocity/weblog.vm @@ -89,8 +89,8 @@ Show RSS, Atom and RSD auto-discovery links as HTML link elements. #if ($model.tags) - - + + #elseif ($model.weblogCategory) diff --git a/app/src/main/webapp/themes/fauxcoly/tags_index.vm b/app/src/main/webapp/themes/fauxcoly/tags_index.vm index 01f535330d..ba138fe95f 100644 --- a/app/src/main/webapp/themes/fauxcoly/tags_index.vm +++ b/app/src/main/webapp/themes/fauxcoly/tags_index.vm @@ -3,7 +3,7 @@ - $model.weblog.name: Entries tagged $model.tags + $model.weblog.name: Entries tagged $utils.escapeHTML($model.tags.toString()) #includeTemplate($model.weblog "standard_head") @@ -28,7 +28,7 @@
-

Entries tagged $model.tags

+

Entries tagged $utils.escapeHTML($model.tags.toString())

#set($pager = $model.getWeblogEntriesPager()) #showWeblogEntriesPager($pager) #if ($model.permalink) @@ -51,7 +51,7 @@

Welcome

- You're viewing entries tagged $model.tags in the weblog $model.weblog.name.

+ You're viewing entries tagged $utils.escapeHTML($model.tags.toString()) in the weblog $model.weblog.name.

From here you can diff --git a/app/src/main/webapp/themes/gaurav/tags_index.vm b/app/src/main/webapp/themes/gaurav/tags_index.vm index 59a8641ae5..1a9848f48c 100755 --- a/app/src/main/webapp/themes/gaurav/tags_index.vm +++ b/app/src/main/webapp/themes/gaurav/tags_index.vm @@ -26,7 +26,7 @@

-

Entries tagged $model.tags

+

Entries tagged $utils.escapeHTML($model.tags.toString())

#set($pager = $model.getWeblogEntriesPager()) #showWeblogEntriesPager($pager) #if ($model.permalink) diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCacheKeyTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCacheKeyTest.java new file mode 100644 index 0000000000..cd1700a279 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/util/cache/WeblogPageCacheKeyTest.java @@ -0,0 +1,80 @@ +/* + * 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.util.cache; + +import java.util.Arrays; + +import org.apache.roller.weblogger.ui.rendering.util.WeblogPageRequest; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** + * Requests that can render different content must get different page cache + * keys. + */ +public class WeblogPageCacheKeyTest { + + private static WeblogPageRequest request(String context, String anchor, + String pageName, String... tags) { + WeblogPageRequest request = new WeblogPageRequest(); + request.setWeblogHandle("blog"); + request.setContext(context); + request.setWeblogAnchor(anchor); + request.setWeblogPageName(pageName); + if (tags.length > 0) { + request.setTags(Arrays.asList(tags)); + } + return request; + } + + private static String key(WeblogPageRequest request) { + return WeblogPageCache.getInstance().generateKey(request); + } + + @Test + public void customPageNamingAnEntryHasItsOwnKey() { + String permalink = key(request("entry", "first-post", null)); + String customPage = key(request("page", "first-post", "archives")); + assertNotEquals(permalink, customPage); + } + + @Test + public void tagsOnACustomPageAreInTheKey() { + String plain = key(request("page", null, "archives")); + String tagged = key(request("page", null, "archives", "java")); + String otherTag = key(request("page", null, "archives", "roller")); + assertNotEquals(plain, tagged); + assertNotEquals(tagged, otherTag); + } + + @Test + public void tagsOnACustomPageNamingAnEntryAreInTheKey() { + String untagged = key(request("page", "first-post", "archives")); + String tagged = key(request("page", "first-post", "archives", "java")); + assertNotEquals(untagged, tagged); + } + + @Test + public void tagOrderDoesNotChangeTheKey() { + assertEquals(key(request("tags", null, null, "java", "roller")), + key(request("tags", null, null, "roller", "java"))); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/AutodiscoveryLinksRenderingTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/AutodiscoveryLinksRenderingTest.java new file mode 100644 index 0000000000..a15535e0cf --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/AutodiscoveryLinksRenderingTest.java @@ -0,0 +1,112 @@ +/* + * 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.Arrays; +import java.util.List; +import java.util.Properties; + +import org.apache.roller.weblogger.ui.rendering.model.UtilitiesModel; +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 #showAutodiscoveryLinks from the real weblog.vm and checks how the + * requested tag names appear in the feed link titles. + */ +public class AutodiscoveryLinksRenderingTest { + + 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", "src/main/webapp/WEB-INF/velocity"); + engine = new VelocityEngine(); + engine.init(props); + } + + public static class StubWeblog { + public boolean getEnableBloggerApi() { return false; } + public String getName() { return "Blog"; } + } + + public static class StubModel { + private final List tags; + StubModel(List tags) { this.tags = tags; } + public boolean getSearchResults() { return false; } + public List getTags() { return tags; } + public Object getWeblogCategory() { return null; } + public StubWeblog getWeblog() { return new StubWeblog(); } + } + + public static class StubFeeds { + public String getAtom() { return "/feed/atom"; } + public String getRss() { return "/feed/rss"; } + public String atomByTags(List tags, boolean excerpts) { return "/feed/atom?tags"; } + public String rssByTags(List tags, boolean excerpts) { return "/feed/rss?tags"; } + } + + public static class StubFeed { + public StubFeeds getEntries() { return new StubFeeds(); } + public StubFeeds getComments() { return new StubFeeds(); } + } + + public static class StubUrl { + public StubFeed getFeed() { return new StubFeed(); } + public String getRsd() { return "/rsd"; } + public String getAbsoluteSite() { return "http://example.test"; } + public String getOpensearch() { return "/opensearch"; } + } + + private static String render(String... tags) { + VelocityContext ctx = new VelocityContext(); + StubModel model = new StubModel(Arrays.asList(tags)); + ctx.put("model", model); + ctx.put("weblog", model.getWeblog()); + ctx.put("url", new StubUrl()); + ctx.put("utils", new UtilitiesModel()); + StringWriter out = new StringWriter(); + engine.evaluate(ctx, out, "test", "#parse('weblog.vm')#showAutodiscoveryLinks($weblog)"); + return out.toString(); + } + + @Test + public void tagNamesAppearInTheFeedTitles() { + String html = render("java", "roller"); + assertTrue(html.contains("title=\"Entries tagged [java, roller] (Atom)\""), html); + assertTrue(html.contains("title=\"Entries tagged [java, roller] (RSS)\""), html); + } + + @Test + public void tagNamesAreHtmlEncodedInTheFeedTitles() { + String html = render("a\"b&d"); + assertTrue(html.contains("title=\"Entries tagged [a"b<c>&d] (Atom)\""), html); + assertFalse(html.contains("a\"b"), html); + } +}