mraible commented on code in PR #176:
URL: https://github.com/apache/roller/pull/176#discussion_r3891453222
##########
app/src/main/webapp/WEB-INF/jsps/editor/Comments.jsp:
##########
@@ -273,12 +273,12 @@
</s:else>
</div>
- <s:if test="#comment.url != null &&
!#comment.url.equals('')">
+ <s:if test="#comment.safeUrl != null">
Review Comment:
Hiding the URL entirely when it fails validation takes away the thing the
moderator needs to judge the comment (`javascript:...`, a homoglyph domain, an
intranet host). Render the raw value as escaped text when `safeUrl` is null and
only make it a link when it isn't.
##########
app/src/main/java/org/apache/roller/weblogger/pojos/wrapper/WeblogEntryCommentWrapper.java:
##########
@@ -93,7 +93,7 @@ public String getEmail() {
* Value is always html escaped.
*/
public String getUrl() {
- return StringEscapeUtils.escapeHtml4(this.pojo.getUrl());
+ return StringEscapeUtils.escapeHtml4(this.pojo.getSafeUrl());
Review Comment:
`CommentServlet` stores `""` for a blank URL, so this used to return `""`
and the javadoc promises a non-null escaped value; `escapeHtml4(null)` is null,
and with strict mode off a custom template that writes `$comment.url` without
an `isEmpty` guard now prints the literal `$comment.url` into `href` for every
comment without a URL. Return `""` when `getSafeUrl()` is null.
##########
app/src/main/webapp/WEB-INF/velocity/weblog.vm:
##########
@@ -114,30 +113,6 @@ Show RSS, Atom and RSD auto-discovery links as HTML link
elements.
#end
-#**
- * Display a trackback auto-discovery RDF comment for a WeblogEntry, but only
- * if trackbacks are enabled and comments are allowed for the entry.
- **#
-#macro( showTrackbackAutodiscovery $entry )
-#if($config.trackbacksEnabled && $model.weblog.allowComments &&
$entry.commentsStillAllowed)
-<!--
-<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#";
-
xmlns:trackback="http://madskills.com/public/xml/rss/module/trackback/";
- xmlns:dc="http://purl.org/dc/elements/1.1/";>
-<rdf:Description
- rdf:about="$url.entry($entry.anchor)"
- trackback:ping="$url.trackback($entry.anchor)"
- dc:title="$entry.title"
- dc:identifier="$url.entry($entry.anchor)"
- dc:subject="$entry.category.name"
- dc:description="$entry.title"
- dc:creator="$entry.creator.userName"
- dc:date="$entry.pubTime" />
-</rdf:RDF>
--->
-#end
-#end
-
#**
Review Comment:
Every release through 6.1.5 shipped `basic/_day.vm` and
`basicmobile/_day.vm` with `#showTrackbackAutodiscovery($entry)`, and anyone
who clicked "Customize theme" has that copied into their DB templates.
`runtime.strict_mode.enable` is off, so Velocity renders the now-undefined
macro call as literal text under every entry. Keep the macro as a documented
no-op (`#macro( showTrackbackAutodiscovery $entry )#end`) for at least a
release, and the same goes for `$url.trackback(...)` in `URLModel`, which
custom templates reference outside an `#if`.
##########
app/src/main/java/org/apache/roller/weblogger/util/CommentAuthorUrl.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * 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.
+ */
+package org.apache.roller.weblogger.util;
+
+import org.apache.commons.validator.routines.UrlValidator;
+
+/**
+ * Normalizes comment author URLs before they are rendered as links.
+ */
+public final class CommentAuthorUrl {
+
+ private static final UrlValidator VALIDATOR =
+ new UrlValidator(new String[] {"http", "https"});
+
+ private CommentAuthorUrl() {
+ }
+
+ public static String normalize(String value) {
+ if (value == null || value.isBlank()) {
+ return null;
+ }
+ String normalized = value.trim();
+ return VALIDATOR.isValid(normalized) ? normalized : null;
Review Comment:
Default `UrlValidator` (no `ALLOW_LOCAL_URLS`, IANA TLD list, ASCII path
regex) rejects `http://localhost:8080/blog`, `http://intranet-wiki/page`,
`http://my_host.example.com/` and IDN paths, so legacy comments and imported
ones lose their author link on public pages and vanish from the admin page,
with no log line. `CommentServlet` accepts on post with the same validator
today, so at minimum reuse one shared instance/policy for both (this class is
the natural home; have the servlet call `normalize`), and consider
`ALLOW_LOCAL_URLS` for intranet installs.
##########
app/src/main/java/org/apache/roller/weblogger/pojos/WeblogEntryComment.java:
##########
@@ -130,6 +131,13 @@ public String getUrl() {
public void setUrl(String url) {
this.url = url;
}
+
+ /**
+ * URL of the comment writer when it can be safely rendered as a link.
+ */
+ public String getSafeUrl() {
Review Comment:
Nit: this runs the validator (regex + `DomainValidator` lookup) on every
call, and `Comments.jsp` evaluates `#comment.safeUrl` three times per row while
`weblog.vm` evaluates `$comment.url` twice per comment. Compute once per render
(an `s:set` in the JSP, a local in the macro, or memoize in the wrapper).
##########
app/src/test/java/org/apache/roller/weblogger/ui/rendering/IncomingTrackbackRemovalTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * 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.
+ */
+package org.apache.roller.weblogger.ui.rendering;
+
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.roller.weblogger.ui.rendering.model.ConfigModel;
+import org.apache.roller.weblogger.ui.rendering.model.URLModel;
+import org.apache.roller.weblogger.util.BannedwordslistChecker;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class IncomingTrackbackRemovalTest {
+
+ @Test
+ void incomingTrackbackClassesAndHelpersAreRemoved() {
+ assertThrows(ClassNotFoundException.class, () -> Class.forName(
+
"org.apache.roller.weblogger.ui.rendering.servlets.TrackbackServlet"));
+ assertThrows(ClassNotFoundException.class, () -> Class.forName(
+
"org.apache.roller.weblogger.ui.rendering.util.WeblogTrackbackRequest"));
+ assertThrows(ClassNotFoundException.class, () -> Class.forName(
+
"org.apache.roller.weblogger.ui.rendering.plugins.comments.TrackbackLinkbackCommentValidator"));
+ assertThrows(NoSuchMethodException.class,
+ () -> URLModel.class.getMethod("trackback", String.class));
+ assertThrows(NoSuchMethodException.class,
+ () -> ConfigModel.class.getMethod("getTrackbacksEnabled"));
+ assertThrows(NoSuchMethodException.class,
+ () -> BannedwordslistChecker.class.getMethod(
+ "checkTrackback",
+
org.apache.roller.weblogger.pojos.WeblogEntryComment.class));
+ }
+
+ @Test
+ void deploymentAndRuntimeConfigurationDoNotExposeTrackbacks() throws
Exception {
+ assertFileDoesNotContain("src/main/webapp/WEB-INF/web.xml",
"trackback");
Review Comment:
cwd-relative paths again; surefire sets `project.build.directory` for this
module, and no other test in `app/src/test` reads source files relative to the
working directory.
##########
app/src/main/resources/ApplicationResources_de.properties:
##########
@@ -252,19 +250,15 @@ configForm.allowNewUsers=Erlaube das Anlegen neuer
Benutzer?
configForm.allowedExtensions=Zul\u00E4ssige Dateierweiterungen
configForm.commentHtmlAllowed=HTML in Kommentaren erlauben?
configForm.commentPlugins=An-/Abschalten von Plugins zur Kommentarformatierung
-configForm.commentSettings=Kommentar und Trackback Einstellungen
configForm.editorPages=Bearbeitungsseiten
configForm.emailComments=E-Mailbenachrichtung bei Kommentaren?
configForm.enableComments=Kommentare in Weblogs erlauben?
Review Comment:
`configForm.commentSettings` was removed from all seven translated bundles
but is still the display-group key in `runtimeConfigDefs.xml:155`, so the
Global Configuration heading falls back to English for every non-English admin.
Re-add it (minus the Trackback wording) in each bundle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
