Check and encode requested tags on every weblog page

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

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.

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) {

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.
     <link rel="alternate" type="application/rss+xml"  title="Recent Comments (RSS)"  href="$url.feed.comments.rss" />
 
     #if ($model.tags)
-      <link rel="alternate" type="application/atom+xml" title="Entries tagged $model.tags.toString() (Atom)"  href="$url.feed.entries.atomByTags($model.tags, false)" />
-      <link rel="alternate" type="application/rss+xml"  title="Entries tagged $model.tags.toString() (RSS)"   href="$url.feed.entries.rssByTags($model.tags, false)" />
+      <link rel="alternate" type="application/atom+xml" title="Entries tagged $utils.escapeHTML($model.tags.toString()) (Atom)"  href="$url.feed.entries.atomByTags($model.tags, false)" />
+      <link rel="alternate" type="application/rss+xml"  title="Entries tagged $utils.escapeHTML($model.tags.toString()) (RSS)"   href="$url.feed.entries.rssByTags($model.tags, false)" />
     #elseif ($model.weblogCategory)
       <link rel="alternate" type="application/atom+xml" title="Entries for category $model.weblogCategory.name (Atom)"  href="$url.feed.entries.atom($model.weblogCategory.name, false)" />
       <link rel="alternate" type="application/rss+xml"  title="Entries for category $model.weblogCategory.name (RSS)"   href="$url.feed.entries.rss($model.weblogCategory.name, false)" />

app/src/main/webapp/themes/fauxcoly/tags_index.vm

@@ -3,7 +3,7 @@
 <head>
   <meta charset="utf-8">
   <meta http-equiv="X-UA-Compatible" content="IE=edge">
-  <title>$model.weblog.name: Entries tagged $model.tags</title>
+  <title>$model.weblog.name: Entries tagged $utils.escapeHTML($model.tags.toString())</title>
   #includeTemplate($model.weblog "standard_head")
 </head>
 <body>
@@ -28,7 +28,7 @@
 <div class="yui3-u-2-3">
 <div id="main_content">
 
-<h2>Entries tagged $model.tags</h2>
+<h2>Entries tagged $utils.escapeHTML($model.tags.toString())</h2>
 #set($pager = $model.getWeblogEntriesPager())
 #showWeblogEntriesPager($pager)
 #if ($model.permalink)
@@ -51,7 +51,7 @@
     <h3>Welcome</h3>
 
     <p class="details">
-    You're viewing entries tagged <b>$model.tags</b> in the weblog <b>$model.weblog.name</b>.</p>
+    You're viewing entries tagged <b>$utils.escapeHTML($model.tags.toString())</b> in the weblog <b>$model.weblog.name</b>.</p>
 
     <p class="details">
     From here you can

app/src/main/webapp/themes/gaurav/tags_index.vm

@@ -26,7 +26,7 @@
 	  	<div class="row">
 	  		<!-- BODY MAIN-->
 			<div id="main_content" class="col-lg-8">
-		   	  	<div class="alert alert-info"><p>Entries tagged $model.tags</p></div>
+		   	  	<div class="alert alert-info"><p>Entries tagged $utils.escapeHTML($model.tags.toString())</p></div>
 				#set($pager = $model.getWeblogEntriesPager())
 				#showWeblogEntriesPager($pager)
 				#if ($model.permalink)

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")));
+    }
+}

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<String> tags;
+        StubModel(List<String> tags) { this.tags = tags; }
+        public boolean getSearchResults() { return false; }
+        public List<String> 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<String> tags, boolean excerpts) { return "/feed/atom?tags"; }
+        public String rssByTags(List<String> 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<c>&d");
+        assertTrue(html.contains("title=\"Entries tagged [a&quot;b&lt;c&gt;&amp;d] (Atom)\""), html);
+        assertFalse(html.contains("a\"b<c>"), html);
+    }
+}