diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/ajax/ThemeDataServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/ajax/ThemeDataServlet.java index 694c736ddf..bf395a2a0a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/ajax/ThemeDataServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/ajax/ThemeDataServlet.java @@ -17,6 +17,7 @@ */ package org.apache.roller.weblogger.ui.struts2.ajax; +import org.apache.commons.text.StringEscapeUtils; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.business.themes.SharedTheme; @@ -80,17 +81,21 @@ public void doGet( } for (Iterator it = themes.iterator(); it.hasNext();) { SharedTheme theme = it.next(); + // Theme metadata comes from theme.xml, which an operator can edit + // or install; escape it so a quote or newline cannot break out of + // the string and produce malformed JSON. pw.print(" { \"id\" : \""); - pw.print(theme.getId()); + pw.print(StringEscapeUtils.escapeJson(theme.getId())); pw.print("\", "); pw.print("\"name\" : \""); - pw.print(theme.getName()); + pw.print(StringEscapeUtils.escapeJson(theme.getName())); pw.print("\", "); pw.print("\"description\" : \""); - pw.print(theme.getDescription()); + pw.print(StringEscapeUtils.escapeJson(theme.getDescription())); pw.print("\", "); pw.print("\"previewPath\" : \""); - pw.print("/themes" + "/" + theme.getId() + "/" + theme.getPreviewImage().getPath()); + pw.print(StringEscapeUtils.escapeJson( + "/themes" + "/" + theme.getId() + "/" + theme.getPreviewImage().getPath())); pw.print("\" }"); if (it.hasNext()) { pw.println(", "); diff --git a/app/src/main/webapp/WEB-INF/jsps/core/CreateWeblog.jsp b/app/src/main/webapp/WEB-INF/jsps/core/CreateWeblog.jsp index 48c4f08f16..b574975fc5 100644 --- a/app/src/main/webapp/WEB-INF/jsps/core/CreateWeblog.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/core/CreateWeblog.jsp @@ -115,7 +115,7 @@ function previewImage(themeId) { $.ajax({ url: "/roller-ui/authoring/themedata", data: {theme:themeId}, success: function(data) { - $('#themedescription').html(data.description); + $('#themedescription').text(data.description); $('#themeThumbnail').attr('src','' + data.previewPath); } }); diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp index d004596b23..9c9889509f 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp @@ -131,7 +131,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and - + " target="_blank"> @@ -143,13 +143,13 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and - ', - '', - '', - '', - '', - '' )"> + " + data-bookmark-name="" + data-bookmark-url="" + data-bookmark-feed-url="" + data-bookmark-description="" + data-bookmark-image=""> @@ -338,7 +338,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and function confirmDeleteFolder() { $('#boomarks_delete_folder_folderId').val($('#bookmarks_folderId:first').val()); - $('#deleteBlogrollName').html(''); + $('#deleteBlogrollName').text($('#deleteBlogrollName').data('folder-name')); $('#delete-blogroll-modal').modal({show: true}); } @@ -565,7 +565,8 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
- ? + ">?
@@ -700,12 +701,23 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and $('#bookmarkEdit_bean_image:first').val(''); $('#bookmarkEdit_bean_feedUrl:first').val(''); - $('#subtitle_folder_name:first').html(originalName); + $('#subtitle_folder_name:first').text(originalName); $('#addedit-bookmark-modal').modal({show: true}); } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('click', '.bookmark-edit-link', function (event) { + event.preventDefault(); + editBookmark($(this).attr('data-bookmark-id'), + $(this).attr('data-bookmark-name'), + $(this).attr('data-bookmark-url'), + $(this).attr('data-bookmark-feed-url'), + $(this).attr('data-bookmark-description'), + $(this).attr('data-bookmark-image')); + }); + function editBookmark(id, name, url, feedUrl, description, image) { var saveBookmarkButton = $('#save_bookmark:first'); @@ -725,7 +737,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and $('#bookmarkEdit_bean_description:first').val(description); $('#bookmarkEdit_bean_image:first').val(image); - $('#subtitle_folder_name:first').html(originalName); + $('#subtitle_folder_name:first').text(originalName); $('#addedit-bookmark-modal').modal({show: true}); } @@ -770,7 +782,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and elem.removeClass("alert-info"); elem.removeClass("alert-danger"); elem.addClass("alert-success"); - elem.html(message); + elem.text(message); } else { saveBookmarkButton.attr("disabled", true); @@ -789,7 +801,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and elem.removeClass("alert-info"); elem.removeClass("alert-success"); elem.addClass("alert-danger"); - elem.html(message); + elem.text(message); } } diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp index 6e1d649bd2..9082a05b33 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp @@ -55,11 +55,11 @@ - ', - '', - '', - '' )"> + " + data-category-name="" + data-category-desc="" + data-category-image=""> @@ -71,10 +71,10 @@ - ', - '', - )" > + " + data-category-name="" + data-category-in-use=""> @@ -288,27 +288,28 @@ function showCategoryDeleteModal( id, name, inUse ) { $('#categoryRemove_removeId').val(id); $('#categoryEdit_bean_name').val(name); - $('#category-name').html(name); + $('#category-name').text(name); if ( inUse ) { $('#category-in-use').css('display','block'); - $('#category-emtpy').css('display', 'none'); + $('#category-empty').css('display', 'none'); } else { $('#category-in-use').css('display', 'none'); - $('#category-emtpy').css('display', 'block'); + $('#category-empty').css('display', 'block'); } populateCategorySelect(id); $('#delete-category-modal').modal({show: true}); } function populateCategorySelect(removeId) { - const allCategories = []; - - - allCategories.push({ - id: '', - name: '' - }); - + // Category names are author-supplied. They are rendered into data + // attributes below and read back as text here, so no name is ever + // parsed as JavaScript. + const allCategories = $('.category-delete-link').map(function () { + return { + id: $(this).attr('data-category-id'), + name: $(this).attr('data-category-name') + }; + }).get(); const select = $('#categoryRemove_targetCategoryId'); select.empty(); @@ -319,4 +320,20 @@ }); } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('click', '.category-edit-link', function (event) { + event.preventDefault(); + showCategoryEditModal($(this).attr('data-category-id'), + $(this).attr('data-category-name'), + $(this).attr('data-category-desc'), + $(this).attr('data-category-image')); + }); + + $(document).on('click', '.category-delete-link', function (event) { + event.preventDefault(); + showCategoryDeleteModal($(this).attr('data-category-id'), + $(this).attr('data-category-name'), + $(this).attr('data-category-in-use') === 'true'); + }); + diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/Comments.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/Comments.jsp index f13a8d59e6..b76a51da73 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Comments.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Comments.jsp @@ -276,7 +276,7 @@
 :  - + ">
diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/Entries.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/Entries.jsp index 438b1ed006..db793bb43f 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Entries.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Entries.jsp @@ -140,8 +140,9 @@ - ', '' )"> + " + data-post-title=""> @@ -238,9 +239,15 @@ diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/EntryEdit.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/EntryEdit.jsp index 61995356ad..e10ade7fe7 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/EntryEdit.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/EntryEdit.jsp @@ -291,9 +291,10 @@ <%-- delete --%> - ', '' )"> + data-post-id="" + data-post-title=""> @@ -436,10 +437,16 @@ }); function showDeleteModal(postId, postTitle) { - $('#postIdLabel').html(postId); - $('#postTitleLabel').html(postTitle); + $('#postIdLabel').text(postId); + $('#postTitleLabel').text(postTitle); $('#removeId').val(postId); $('#delete-entry-modal').modal({show: true}); } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('click', '.entry-delete-button', function (event) { + event.preventDefault(); + showDeleteModal($(this).attr('data-post-id'), $(this).attr('data-post-title')); + }); + diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileAddSuccess.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileAddSuccess.jsp index c3cbae0f22..a8a9572a9b 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileAddSuccess.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileAddSuccess.jsp @@ -95,8 +95,8 @@
- ')"/> + "/>
@@ -201,6 +201,11 @@ return false; } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('change', '.enclosure-choice', function () { + setEnclosure($(this).attr('data-enclosure-url')); + }); + function setEnclosure(url) { $("#enclosureURL").get(0).value = url; if (isImageChecked()) { diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileImageChooser.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileImageChooser.jsp index dddd4d07ad..c02dcb64ab 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileImageChooser.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileImageChooser.jsp @@ -72,16 +72,16 @@
  • -
    ', - '', - '')"> +
    " + data-mediafile-url="" + data-mediafile-is-image=""> - <s:property value="#mediaFile.name" /> + <s:property value='#mediaFile.name' /> @@ -118,6 +118,13 @@ window.parent.onSelectMediaFile(name, url, isImage); } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('click', '.mediafile-select-target', function () { + onSelectMediaFile($(this).attr('data-mediafile-name'), + $(this).attr('data-mediafile-url'), + $(this).attr('data-mediafile-is-image')); + }); + function highlight(el, flag) { if (flag) { $(el).addClass("highlight"); diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileView.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileView.jsp index 253220461b..46a02002b7 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileView.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/MediaFileView.jsp @@ -19,7 +19,7 @@ - + @@ -169,25 +169,25 @@
  • -
    ', - '' )"> +
    " + data-mediafile-name=""> - <s:property value="#mediaFile.name" />')" --%> /> + <s:property value='#mediaFile.name' /> - <s:property value="#mediaFile.name"/>')" --%> /> + alt="" + />
    @@ -223,22 +223,22 @@
  • -
    ', - '' )"> +
    " + data-mediafile-name=""> - <s:property value="#mediaFile.name"/> + <s:property value='#mediaFile.name'/> - <s:property value="#mediaFile.name"/> + <s:property value='#mediaFile.name'/>
    @@ -348,11 +348,17 @@ - $('#edit-subtitle').html(mediaFileName); + $('#edit-subtitle').text(mediaFileName); $('#mediaFileEditor').attr('src', '' + '&mediaFileId=' + mediaFileId); $('#mediafile_edit_lightbox').modal({show: true}); } + // Values come from data-* attributes and are bound via delegated listeners. + $(document).on('click', '.mediafile-edit-target', function () { + onClickEdit($(this).attr('data-mediafile-id'), + $(this).attr('data-mediafile-name')); + }); + function onEditSuccess() { onEditCancelled(); document.mediaFileViewForm.submit(); diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/TemplateEdit.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/TemplateEdit.jsp index 4b738d3f8a..81b10e6421 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/TemplateEdit.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/TemplateEdit.jsp @@ -177,11 +177,19 @@ + + diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/ThemeEdit.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/ThemeEdit.jsp index dac1ec8855..f71d9de08c 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/ThemeEdit.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/ThemeEdit.jsp @@ -246,7 +246,7 @@ $.ajax({ url: "", data: {theme: themeId}, success: function (data) { - $('#themeDescription').html(data.description); + $('#themeDescription').text(data.description); thumbnail = $('#themeThumbnail'); thumbnail.attr('src', '' + data.previewPath); } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/AuthoringUiSinkAuditTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/AuthoringUiSinkAuditTest.java new file mode 100644 index 0000000000..cf8eac7342 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/AuthoringUiSinkAuditTest.java @@ -0,0 +1,153 @@ +/* + * 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.struts2.editor; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * Structural audit of the authoring UI templates. + * + *

    Values that originate from weblog content are rendered by the editor JSPs. + * This test enforces the two structural rules that keep those values inert: they + * travel in double-quoted data-* attributes rather than inline + * handler literals, and they are written to the DOM through a text API. It is a + * source audit rather than a behavioural test because the guarantee is a + * property of the whole page family, not of any one code path. + */ +public class AuthoringUiSinkAuditTest { + + private static final Path JSP_ROOT = Paths.get( + System.getProperty("project.basedir", System.getProperty("user.dir")), + "src", "main", "webapp", "WEB-INF", "jsps"); + + /** + * An inline event handler attribute whose body opens a single-quoted + * JavaScript string containing a Struts property. Newlines are collapsed + * before matching because these handlers routinely wrap across lines. + */ + private static final Pattern HANDLER_LITERAL = + Pattern.compile("on[a-zA-Z]+\\s*=\\s*\"[^\"]*'\\s* editorJsps() throws IOException { + Path editor = JSP_ROOT.resolve("editor"); + Path core = JSP_ROOT.resolve("core"); + assertTrue(Files.isDirectory(editor), "cannot locate editor JSPs at " + editor); + assertTrue(Files.isDirectory(core), "cannot locate core JSPs at " + core); + try (Stream files = Stream.concat(Files.list(editor), Files.list(core))) { + return files.filter(p -> p.getFileName().toString().endsWith(".jsp")) + .sorted().collect(Collectors.toList()); + } + } + + private List findMatches(Pattern pattern) throws IOException { + List offenders = new ArrayList<>(); + for (Path jsp : editorJsps()) { + String body = new String(Files.readAllBytes(jsp), StandardCharsets.UTF_8); + Matcher matcher = pattern.matcher(body); + while (matcher.find()) { + String snippet = matcher.group().replaceAll("\\s+", " ").trim(); + offenders.add(jsp.getFileName() + ": " + snippet); + } + } + return offenders; + } + + /** + * No weblog-controlled value may sit inside a single-quoted JavaScript + * string literal in an inline event handler. An apostrophe in the value + * closes the literal and the rest of the value is parsed as code. + */ + @Test + public void noStrutsPropertyInsideInlineHandlerLiteral() throws IOException { + List offenders = findMatches(HANDLER_LITERAL); + assertTrue(offenders.isEmpty(), + "authoring values must travel in data-* attributes, not inline " + + "handler string literals; found " + offenders.size() + ":\n " + + String.join("\n ", offenders)); + } + + /** + * The same rule for values assigned into script variables, which are the + * non-handler form of the identical defect. + */ + @Test + public void noStrutsPropertyInsideScriptVariableLiteral() throws IOException { + List offenders = findMatches(SCRIPT_VAR_LITERAL); + assertTrue(offenders.isEmpty(), + "authoring values must reach script through data-* attributes, " + + "not single-quoted var initialisers; found " + offenders.size() + ":\n " + + String.join("\n ", offenders)); + } + + /** + * No dynamic value may be written to the DOM as markup. Moving a value into + * a data attribute does not help if a later html() call re-parses it. + */ + @Test + public void noDynamicHtmlWrites() throws IOException { + List offenders = findMatches(HTML_WRITE); + assertTrue(offenders.isEmpty(), + "dynamic values must be written with a text API (.text(), " + + ".val(), textContent) rather than .html(); found " + + offenders.size() + ":\n " + String.join("\n ", offenders)); + } + + /** + * Guards the audit itself: if the JSP directory moved or the patterns stopped + * matching anything at all, the three tests above would pass vacuously. + */ + @Test + public void auditActuallyInspectsTheEditorTemplates() throws IOException { + List jsps = editorJsps(); + assertFalse(jsps.isEmpty(), "audit found no editor JSPs to inspect"); + assertTrue(jsps.size() >= 10, + "expected the editor template family, found only " + jsps.size()); + } +}