From e5c7ff2f9612c167a380b039024af10840548d7b Mon Sep 17 00:00:00 2001 From: Matt Raible Date: Sat, 14 Nov 2026 11:53:28 -0700 Subject: [PATCH] Encode meta descriptions once, whatever the trust setting The templates now remove tags, decode entities and encode the result, so the attribute is encoded exactly once in both modes. MetaDescriptionEscapingTest renders both templates with stored and pre-encoded values; it fails on the old templates. (cherry picked from commit f9b195e9b0d74e7839470ee3168ef0745c54647f) --- CHANGES.md | 9 ++ .../main/webapp/WEB-INF/velocity/weblog.vm | 5 +- app/src/main/webapp/themes/gaurav/std_head.vm | 4 +- .../velocity/MetaDescriptionEscapingTest.java | 137 ++++++++++++++++++ 4 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/MetaDescriptionEscapingTest.java diff --git a/CHANGES.md b/CHANGES.md index ae054e20d7..f85731857a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,14 @@ # Apache Roller — Changes +## 6.1.8 (in development) + +### Bug fixes + +- **Meta descriptions are always encoded.** A weblog's "about" text or an + entry's search description containing a quote no longer cuts the + `` tag short, whether or not weblog admins are + trusted. + ## 6.1.7 ### Improvements diff --git a/app/src/main/webapp/WEB-INF/velocity/weblog.vm b/app/src/main/webapp/WEB-INF/velocity/weblog.vm index 8ff1065152..58da142ae1 100644 --- a/app/src/main/webapp/WEB-INF/velocity/weblog.vm +++ b/app/src/main/webapp/WEB-INF/velocity/weblog.vm @@ -128,12 +128,13 @@ blog entry (permalink) page. If the relevant description value has not been con will be created. *# #macro(showMetaDescription) + ## escaped once whatever the trust setting: tags removed, entities decoded, then encoded #if ($model.permalink) #if ($utils.isNotEmpty($model.weblogEntry.searchDescription)) - + #end #elseif ($utils.isNotEmpty($model.weblog.about)) - + #end #end diff --git a/app/src/main/webapp/themes/gaurav/std_head.vm b/app/src/main/webapp/themes/gaurav/std_head.vm index e68bba6e7b..ec5bf81e8e 100755 --- a/app/src/main/webapp/themes/gaurav/std_head.vm +++ b/app/src/main/webapp/themes/gaurav/std_head.vm @@ -1,9 +1,9 @@ #if ($model.permalink == false) - + #else - + #end diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/MetaDescriptionEscapingTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/MetaDescriptionEscapingTest.java new file mode 100644 index 0000000000..38d508d2a0 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/velocity/MetaDescriptionEscapingTest.java @@ -0,0 +1,137 @@ +/* + * 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.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.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The weblog's "about" text and an entry's search description are written into + * a {@code } attribute. They are stored as typed when weblog + * admins are trusted, and HTML-encoded on output when they aren't, so the + * templates must encode them exactly once: a quote must not end the attribute, + * and text the sanitizer already encoded must not be encoded twice. + */ +public class MetaDescriptionEscapingTest { + + /** Minimal stand-ins for the model objects the templates read. */ + public static class StubModel { + private final boolean permalink; + private final StubWeblog weblog; + private final StubEntry entry; + + StubModel(boolean permalink, String text) { + this.permalink = permalink; + this.weblog = new StubWeblog(text); + this.entry = new StubEntry(text); + } + + public boolean isPermalink() { + return permalink; + } + + public StubWeblog getWeblog() { + return weblog; + } + + public StubEntry getWeblogEntry() { + return entry; + } + } + + public static class StubWeblog { + private final String about; + + StubWeblog(String about) { + this.about = about; + } + + public String getAbout() { + return about; + } + } + + public static class StubEntry { + private final String searchDescription; + + StubEntry(String searchDescription) { + this.searchDescription = searchDescription; + } + + public String getSearchDescription() { + return searchDescription; + } + } + + private static String render(String loaderPath, String source, boolean permalink, String text) { + 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", loaderPath); + VelocityEngine engine = new VelocityEngine(); + engine.init(props); + + VelocityContext ctx = new VelocityContext(); + ctx.put("model", new StubModel(permalink, text)); + ctx.put("utils", new UtilitiesModel()); + StringWriter out = new StringWriter(); + engine.evaluate(ctx, out, "test", source); + return out.toString(); + } + + private static String sharedMacro(boolean permalink, String text) { + return render("src/main/webapp/WEB-INF/velocity", + "#parse('weblog.vm')#showMetaDescription()", permalink, text); + } + + private static String gaurav(boolean permalink, String text) { + return render("src/main/webapp/themes/gaurav", "#parse('std_head.vm')", permalink, text); + } + + /** Stored value, and what the attribute must contain. */ + private static final String[][] CASES = { + // stored as typed (trusted admins): a quote must not close the attribute + {"Tips \" autofocus onfocus=\"alert(1)", "Tips " autofocus onfocus="alert(1)"}, + // already encoded by the sanitizer (untrusted admins): encoded once, not twice + {"Tips " autofocus onfocus="alert(1)", "Tips " autofocus onfocus="alert(1)"}, + {"Matt's \"weblog\" & notes", "Matt's "weblog" & notes"}, + {"Bold text", "Bold text"}, + }; + + @Test + public void metaDescriptionIsEncodedOnce() { + for (String[] c : CASES) { + String[] outputs = { + sharedMacro(false, c[0]), sharedMacro(true, c[0]), + gaurav(false, c[0]), gaurav(true, c[0]) + }; + for (String out : outputs) { + assertTrue(out.contains("content=\"" + c[1] + "\">"), c[0] + " gave: " + out); + } + } + } +}