Retire incoming Trackback support and normalize comment links - #176
Retire incoming Trackback support and normalize comment links#176snoopdave wants to merge 1 commit into
Conversation
mraible
left a comment
mraible
left a comment
There was a problem hiding this comment.
Retiring incoming Trackback is overdue and the removal itself is thorough. What I'd like fixed before approving is the compatibility for existing sites (inline), and a few notes outside the diff:
TrackbackLinkbackCommentValidatoris deleted, and it's a documented entry forcomment.validator.classnamesin the user guide.CommentValidationManagerbuilds the list withReflection.newInstances, which throws on the first missing class and leaves the list empty, so an install that lists it loses every comment validator (banned words, excess links, Akismet) silently. Either keep a no-op class for a release or make the manager skip unknown validators with a warning.docs/roller-template-guide.adoc:846still documents$url.trackback(anchor)and line 1861 still documents#showTrackbackAutodiscovery($entry);docs/roller-user-guide.adoc:1282anddocs/examples/configs/tomcat/mod_jk.conf:10also still reference trackbacks.
| #end | ||
| #end | ||
|
|
||
| #** |
There was a problem hiding this 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.
| */ | ||
| public String getUrl() { | ||
| return StringEscapeUtils.escapeHtml4(this.pojo.getUrl()); | ||
| return StringEscapeUtils.escapeHtml4(this.pojo.getSafeUrl()); |
There was a problem hiding this 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.
| </div> | ||
|
|
||
| <s:if test="#comment.url != null && !#comment.url.equals('')"> | ||
| <s:if test="#comment.safeUrl != null"> |
There was a problem hiding this 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.
| return null; | ||
| } | ||
| String normalized = value.trim(); | ||
| return VALIDATOR.isValid(normalized) ? normalized : null; |
There was a problem hiding this 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.
| configForm.commentSettings=Kommentar und Trackback Einstellungen | ||
| configForm.editorPages=Bearbeitungsseiten | ||
| configForm.emailComments=E-Mailbenachrichtung bei Kommentaren? | ||
| configForm.enableComments=Kommentare in Weblogs erlauben? |
There was a problem hiding this 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.
| /** | ||
| * URL of the comment writer when it can be safely rendered as a link. | ||
| */ | ||
| public String getSafeUrl() { |
There was a problem hiding this 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).
|
|
||
| @Test | ||
| void deploymentAndRuntimeConfigurationDoNotExposeTrackbacks() throws Exception { | ||
| assertFileDoesNotContain("src/main/webapp/WEB-INF/web.xml", "trackback"); |
There was a problem hiding this 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.
2 participants
Summary:
Dependency:
Testing: