From e5089ddd9d9ada63bed576cb7d0a43934ae1867e Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:02:24 -0400 Subject: [PATCH 1/9] Refresh the form salt after AJAX saves on blogroll, category and ping target pages A salt is consumed on use, and these pages post a dialog with AJAX and then post the page form again with the same salt, so the second post was refused. Copy the salt issued with each AJAX response into the page's forms. Also stop the blogroll selector from submitting on mouseup, which reloaded the page as soon as the list was opened. --- CHANGES.md | 10 ++++++++++ .../webapp/WEB-INF/jsps/admin/PingTargets.jsp | 2 ++ .../main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp | 8 +++++++- .../webapp/WEB-INF/jsps/editor/Categories.jsp | 2 ++ app/src/main/webapp/theme/scripts/roller.js | 15 +++++++++++++++ 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index d6988febd2..2aa1fc90a5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,16 @@ log an error and continue. It now stops at startup, so check the log if a custom XML parser is on the classpath. +### Bug fixes + +- **Blogroll, category and ping target dialogs work again after a save.** + Adding or renaming a blogroll, saving a bookmark, or saving a ping target + then refreshing the page failed with an error page. So did retrying after a + "name already in use" message. The page now picks up a new form token after + each save. +- **"Switch to blogroll" lets you pick a blogroll.** The page no longer reloads + as soon as you open the list. + ## 6.1.6 Initial installation now requires a one-time, cryptographically secure setup token printed to the server log. Bootstrap access closes as soon as setup finishes — when the first administrator is created on a new site, or when the database upgrade completes on an existing one. diff --git a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp index 2fad6cc185..50a92fa938 100644 --- a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp @@ -277,6 +277,8 @@ }).done(function (data) { + refreshSalt(data); + // kludge: scrape response status from HTML returned by Struts var alertEnd = data.indexOf("ALERT_END"); var notUnique = data.indexOf(""); 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 9c9889509f..a2238d6482 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp @@ -96,7 +96,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and <%-- allow user to select the bookmark folder to view --%> + label="%{getText('bookmarksForm.switchTo')}" onchange="viewChanged()"/> @@ -307,6 +307,8 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and }).done(function (data, status, response) { + refreshSalt(data); + // kludge: scrape response status from HTML returned by Struts var alertEnd = data.indexOf("ALERT_END"); var notUnique = data.indexOf(''); @@ -472,6 +474,8 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and }).done(function (data, status, response) { + refreshSalt(data); + // kludge: scrape response status from HTML returned by Struts var alertEnd = data.indexOf("ALERT_END"); var notUnique = data.indexOf(''); @@ -819,6 +823,8 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and }).done(function (data) { + refreshSalt(data); + // kludge: scrape response status from HTML returned by Struts var alertEnd = data.indexOf("ALERT_END"); var notUnique = data.indexOf(''); 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 9082a05b33..1bd3d59a4f 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp @@ -205,6 +205,8 @@ }).done(function (data) { + refreshSalt(data); + // kludge: scrape response status from HTML returned by Struts var alertEnd = data.indexOf("ALERT_END"); var notUnique = data.indexOf(''); diff --git a/app/src/main/webapp/theme/scripts/roller.js b/app/src/main/webapp/theme/scripts/roller.js index f703a622d7..c8f4f2d73a 100644 --- a/app/src/main/webapp/theme/scripts/roller.js +++ b/app/src/main/webapp/theme/scripts/roller.js @@ -212,6 +212,21 @@ function validateEmail(email) { var re = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/; return re.test(email); } + +/* + * A salt can be used only once. After a form is posted with AJAX, copy the + * salt issued with the response into the forms on this page, so the next post + * from this page is accepted. + */ +function refreshSalt(html) { + var issued = new DOMParser().parseFromString(html, "text/html") + .querySelector("input[name='salt']"); + if (issued && issued.value) { + document.querySelectorAll("input[name='salt']").forEach(function (field) { + field.value = issued.value; + }); + } +} $(document).ready(function () { jQuery("form.validate-form").validate(); // Added method to check valid email address and add a custom error message From 99bdd5bd039b9dba49d19a94f08ff946e79e34e4 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:08:27 -0400 Subject: [PATCH 2/9] Read the salt from a meta tag and report blogroll errors from the response The page returned for a refused blogroll save has no forms, so the salt is now also issued in a roller-salt meta tag on every UI page. The blogroll dialogs looked for the bookmark duplicate-name message, so a refused blogroll name was treated as a success; they now show the action errors in the response. jQuery 3 has no .error(), so use .fail(). --- .../webapp/WEB-INF/jsps/admin/PingTargets.jsp | 2 +- .../webapp/WEB-INF/jsps/editor/Bookmarks.jsp | 22 ++++++++----------- .../webapp/WEB-INF/jsps/editor/Categories.jsp | 2 +- .../main/webapp/WEB-INF/jsps/tiles/head.jsp | 3 +++ app/src/main/webapp/theme/scripts/roller.js | 16 +++++++++++--- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp index 50a92fa938..8159bf055d 100644 --- a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp @@ -295,7 +295,7 @@ viewChanged(); } - }).error(function (data) { + }).fail(function (data) { feedbackAreaEdit.html(''); feedbackAreaEdit.css("color", "red"); }); 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 a2238d6482..315683fb0d 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp @@ -309,18 +309,16 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and refreshSalt(data); - // kludge: scrape response status from HTML returned by Struts - var alertEnd = data.indexOf("ALERT_END"); - var notUnique = data.indexOf(''); - if (notUnique > 0 && notUnique < alertEnd) { - alert(''); + var errors = actionErrors(data); + if (errors.length > 0) { + alert(errors.join("\n")); } else { originalName = newName; nameChanged(); } - }).error(function (data) { + }).fail(function (data) { alert(''); }); } @@ -476,12 +474,10 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and refreshSalt(data); - // kludge: scrape response status from HTML returned by Struts - var alertEnd = data.indexOf("ALERT_END"); - var notUnique = data.indexOf(''); - if (notUnique > 0 && notUnique < alertEnd) { + var errors = actionErrors(data); + if (errors.length > 0) { feedbackAreaBlogrollEdit.css("color", "red"); - feedbackAreaBlogrollEdit.html(''); + feedbackAreaBlogrollEdit.text(errors.join(" ")); } else { feedbackAreaBlogrollEdit.css("color", "green"); @@ -498,7 +494,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and bookmarksForm.submit(); } - }).error(function (data) { + }).fail(function (data) { feedbackAreaBlogrollEdit.html(''); feedbackAreaBlogrollEdit.css("color", "red"); }); @@ -844,7 +840,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and viewChanged(); } - }).error(function (data) { + }).fail(function (data) { feedbackAreaEdit.html(''); feedbackAreaEdit.css("color", "red"); }); 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 1bd3d59a4f..31c04f2a9c 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp @@ -224,7 +224,7 @@ location.reload(true); } - }).error(function (data) { + }).fail(function (data) { feedbackAreaEdit.html(''); feedbackAreaEdit.css("color", "red"); }); diff --git a/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp b/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp index 42ad1d1d8b..74747be1b4 100644 --- a/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp @@ -5,6 +5,9 @@ You can override it with your own file via WEB-INF/tiles-def.xml <%@ include file="/WEB-INF/jsps/taglibs-struts2.jsp" %> +<%-- salt issued with this response, read by refreshSalt() after an AJAX post --%> +" /> + <%-- jquery-ui webjar is 1.14.2+1 in pom.xml, but its resources are served diff --git a/app/src/main/webapp/theme/scripts/roller.js b/app/src/main/webapp/theme/scripts/roller.js index c8f4f2d73a..357fb5b8c8 100644 --- a/app/src/main/webapp/theme/scripts/roller.js +++ b/app/src/main/webapp/theme/scripts/roller.js @@ -220,13 +220,23 @@ function validateEmail(email) { */ function refreshSalt(html) { var issued = new DOMParser().parseFromString(html, "text/html") - .querySelector("input[name='salt']"); - if (issued && issued.value) { + .querySelector("meta[name='roller-salt']"); + var salt = issued ? issued.getAttribute("content") : ""; + if (salt) { document.querySelectorAll("input[name='salt']").forEach(function (field) { - field.value = issued.value; + field.value = salt; }); } } + +/* Returns the action error messages in a page returned to an AJAX post. */ +function actionErrors(html) { + var errors = new DOMParser().parseFromString(html, "text/html") + .querySelectorAll("#errors li"); + return Array.prototype.map.call(errors, function (error) { + return error.textContent.trim(); + }); +} $(document).ready(function () { jQuery("form.validate-form").validate(); // Added method to check valid email address and add a custom error message From 8754579a94c04fdf4c69e40fe15eb05d905a9d90 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:11:29 -0400 Subject: [PATCH 3/9] Fix the NullPointerException when a blogroll is renamed FolderEdit set folderId only when adding, but always read it for the folderId response header, so every rename threw after the folder was saved and the action returned INPUT with a system error. Use the saved folder's id. --- CHANGES.md | 4 + .../ui/struts2/editor/FolderEdit.java | 2 +- .../ui/struts2/editor/FolderEditSaveTest.java | 114 ++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEditSaveTest.java diff --git a/CHANGES.md b/CHANGES.md index 2aa1fc90a5..404cfca1d5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,10 @@ each save. - **"Switch to blogroll" lets you pick a blogroll.** The page no longer reloads as soon as you open the list. +- **Renaming a blogroll no longer reports a system error.** The rename was + saved, but the page showed "System error - check logs". +- **A blogroll name that is already in use is reported** in the dialog, instead + of the page failing with an error. ## 6.1.6 diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEdit.java index 9df65972f1..e60d694db1 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEdit.java @@ -136,7 +136,7 @@ public String save() { } // HTTP response splitting defense - String sanetizedFolderID = folderId.replace("\n", "").replace("\r", ""); + String sanetizedFolderID = folder.getId().replace("\n", "").replace("\r", ""); httpServletResponse.addHeader("folderId", sanetizedFolderID); diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEditSaveTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEditSaveTest.java new file mode 100644 index 0000000000..892c90ed0a --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/FolderEditSaveTest.java @@ -0,0 +1,114 @@ +/* + * 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.struts2.editor; + +import javax.servlet.http.HttpServletResponse; + +import org.apache.roller.weblogger.business.BookmarkManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.pojos.WeblogBookmarkFolder; +import org.apache.roller.weblogger.util.cache.CacheManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class FolderEditSaveTest { + + private MockedStatic factory; + private MockedStatic cache; + private BookmarkManager bookmarks; + private Weblog weblog; + private HttpServletResponse response; + + @BeforeEach + void setUp() { + factory = mockStatic(WebloggerFactory.class); + cache = mockStatic(CacheManager.class); + bookmarks = mock(BookmarkManager.class); + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getBookmarkManager()).thenReturn(bookmarks); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + weblog = mock(Weblog.class); + response = mock(HttpServletResponse.class); + } + + @AfterEach + void tearDown() { + cache.close(); + factory.close(); + } + + @Test + void renamingAFolderSavesItAndSendsItsId() throws Exception { + WeblogBookmarkFolder folder = new WeblogBookmarkFolder(); + folder.setId("folder-1"); + folder.setName("old"); + folder.setWeblog(weblog); + when(bookmarks.getFolderById(weblog, "folder-1")).thenReturn(folder); + + FolderEdit action = action("folderEdit"); + action.getBean().setId("folder-1"); + action.getBean().setName("new"); + action.myPrepare(); + + assertEquals(FolderEdit.SUCCESS, action.save()); + assertFalse(action.hasActionErrors()); + assertEquals("new", folder.getName()); + verify(bookmarks).saveFolder(folder); + verify(response).addHeader("folderId", "folder-1"); + } + + @Test + void addingAFolderSendsTheNewId() throws Exception { + doAnswer(call -> { + call.getArgument(0).setId("new-folder"); + return null; + }).when(bookmarks).saveFolder(any()); + + FolderEdit action = action("folderAdd"); + action.getBean().setName("added"); + action.myPrepare(); + + assertEquals(FolderEdit.SUCCESS, action.save()); + verify(response).addHeader("folderId", "new-folder"); + } + + private FolderEdit action(String actionName) { + FolderEdit action = spy(new FolderEdit()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString(), anyList()); + action.setActionName(actionName); + action.setActionWeblog(weblog); + action.setServletResponse(response); + return action; + } +} From 793ccfc3dd5e1f0812b9fed6b80dd2c25b3ce92f Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:14:11 -0400 Subject: [PATCH 4/9] Show every action error in the AJAX dialogs The dialogs looked only for a duplicate-name message, so any other refusal (a malformed ping URL, for example) closed the dialog as if the save had worked. Show the action errors in the response instead. --- CHANGES.md | 6 ++++-- .../main/webapp/WEB-INF/jsps/admin/PingTargets.jsp | 8 +++----- .../main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp | 8 +++----- .../main/webapp/WEB-INF/jsps/editor/Categories.jsp | 12 +++--------- 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 404cfca1d5..20c44ce3cd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -21,8 +21,10 @@ as soon as you open the list. - **Renaming a blogroll no longer reports a system error.** The rename was saved, but the page showed "System error - check logs". -- **A blogroll name that is already in use is reported** in the dialog, instead - of the page failing with an error. +- **The blogroll, bookmark, category and ping target dialogs show why a save + was refused.** Before, only a duplicate name was reported. Any other error, + and on the blogroll dialogs even a duplicate name, closed the dialog as if the + save had worked, or ended on an error page. ## 6.1.6 diff --git a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp index 8159bf055d..f0c328ce11 100644 --- a/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp @@ -279,12 +279,10 @@ refreshSalt(data); - // kludge: scrape response status from HTML returned by Struts - var alertEnd = data.indexOf("ALERT_END"); - var notUnique = data.indexOf(""); - if (notUnique > 0 && notUnique < alertEnd) { + var errors = actionErrors(data); + if (errors.length > 0) { feedbackAreaEdit.css("color", "red"); - feedbackAreaEdit.html(''); + feedbackAreaEdit.text(errors.join(" ")); } else { feedbackAreaEdit.css("color", "green"); 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 315683fb0d..c0696cc777 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp @@ -821,12 +821,10 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and refreshSalt(data); - // kludge: scrape response status from HTML returned by Struts - var alertEnd = data.indexOf("ALERT_END"); - var notUnique = data.indexOf(''); - if (notUnique > 0 && notUnique < alertEnd) { + var errors = actionErrors(data); + if (errors.length > 0) { feedbackAreaEdit.css("color", "red"); - feedbackAreaEdit.html(''); + feedbackAreaEdit.text(errors.join(" ")); } else { feedbackAreaEdit.css("color", "green"); 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 31c04f2a9c..324814c153 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp @@ -207,16 +207,10 @@ refreshSalt(data); - // kludge: scrape response status from HTML returned by Struts - var alertEnd = data.indexOf("ALERT_END"); - var notUnique = data.indexOf(''); - var notValid = data.indexOf(''); - if (notUnique > 0 && notUnique < alertEnd) { + var errors = actionErrors(data); + if (errors.length > 0) { feedbackAreaEdit.css("color", "red"); - feedbackAreaEdit.html(''); - } else if (notValid > 0 && notValid < alertEnd) { - feedbackAreaEdit.css("color", "red"); - feedbackAreaEdit.html(''); + feedbackAreaEdit.text(errors.join(" ")); } else { feedbackAreaEdit.css("color", "green"); feedbackAreaEdit.html(''); From 7c32dccd43a1c99772d73890691c9d4de9f93e69 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:20:01 -0400 Subject: [PATCH 5/9] Align the blogroll name and blogroll selector rows Lay out both rows with the same label and control columns, and keep the rename buttons on the name field's line with flexbox instead of floats. Show the folder name with s:property; s:text looked it up as a message key and logged a warning on every page view. --- .../webapp/WEB-INF/jsps/editor/Bookmarks.jsp | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) 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 c0696cc777..3de7bd9ee3 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp @@ -54,7 +54,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
-
+
@@ -72,22 +72,22 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
- - - +
+ + + +
@@ -95,8 +95,15 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and <%-- allow user to select the bookmark folder to view --%> - +
+ +
+ +
+
From bcd7856c89f40af22a5dc09b1278e9cc5d6a2515 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 10:43:35 -0400 Subject: [PATCH 6/9] Fix AtomPub media collection lookups and uploads - A named collection such as /resources/default was looked up as "default/", found nothing, and threw a NullPointerException, so the collection the service document advertises could not be listed. - Media posted with no Slug header and no title threw in replaceNonAlphanumeric(); createFileName() already names such files by date. - Media posted to /resources had no directory name and threw; it now goes to the default directory. An unknown directory answers 404. - The temporary upload file used the client's file name as its prefix, which createTempFile() refuses below three characters. Use a random prefix, as putMedia() does. --- CHANGES.md | 6 + .../atomprotocol/MediaCollection.java | 24 ++- .../atomprotocol/MediaCollectionPathTest.java | 172 ++++++++++++++++++ 3 files changed, 194 insertions(+), 8 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionPathTest.java diff --git a/CHANGES.md b/CHANGES.md index 20c44ce3cd..04ca40de20 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,12 @@ and on the blogroll dialogs even a duplicate name, closed the dialog as if the save had worked, or ended on an error page. +- **AtomPub media collections work.** Listing a media collection by the URL in + the service document (`/resources/default`) failed with a server error. So + did uploading media with no `Slug` header and no title, uploading to + `/resources` itself, or uploading with a very short `Slug`. Unknown media + directories now answer 404. + ## 6.1.6 Initial installation now requires a one-time, cryptographically secure setup token printed to the server log. Bootstrap access closes as soon as setup finishes — when the first administrator is created on a new site, or when the database upgrade completes on an existing one. diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java index b4b100fb87..1081b65440 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java @@ -115,10 +115,14 @@ public Entry postMedia(AtomRequest areq, Entry entry) throws AtomException { } if (pathInfo.length > 1) { // Save to temp file - String fileName = createFileName(website, - (slug != null) ? slug : Utilities.replaceNonAlphanumeric(title,' '), contentType); + String baseName = slug; + if (baseName == null && title != null) { + baseName = Utilities.replaceNonAlphanumeric(title, ' '); + } + // createFileName() uses the date when there is no name + String fileName = createFileName(website, baseName, contentType); try { - tempFile = File.createTempFile(fileName, "tmp"); + tempFile = File.createTempFile(UUID.randomUUID().toString(), "tmp"); FileOutputStream fos = new FileOutputStream(tempFile); Utilities.copyInputToOutput(is, fos); fos.close(); @@ -131,8 +135,12 @@ public Entry postMedia(AtomRequest areq, Entry entry) throws AtomException { justPath = path.substring(lastSlash); } - MediaFileDirectory mdir = - fileMgr.getMediaFileDirectoryByName(website, justPath); + MediaFileDirectory mdir = justPath.isEmpty() + ? fileMgr.getDefaultMediaFileDirectory(website) + : fileMgr.getMediaFileDirectoryByName(website, justPath); + if (mdir == null) { + throw new AtomNotFoundException("Cannot find media directory: " + justPath); + } if (mdir.hasMediaFile(fileName)) { throw new AtomException("Duplicate file name"); @@ -265,9 +273,6 @@ public Feed getCollection(AtomRequest areq) throws AtomException { } catch (Exception ingored) {} } String path = filePathFromPathInfo(pathInfo); - if (!path.isEmpty()) { - path = path + File.separator; - } String handle = pathInfo[0]; String absUrl = WebloggerRuntimeConfig.getAbsoluteContextURL(); @@ -299,6 +304,9 @@ public Feed getCollection(AtomRequest areq) throws AtomException { log.debug("Fetching root resource collection from weblog " + handle); dir = fmgr.getDefaultMediaFileDirectory(website); } + if (dir == null) { + throw new AtomNotFoundException("Cannot find media directory: " + path); + } Set files = dir.getMediaFiles(); SortedSet sortedSet = new TreeSet<>(new Comparator() { diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionPathTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionPathTest.java new file mode 100644 index 0000000000..dd145fdb4c --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionPathTest.java @@ -0,0 +1,172 @@ +/* + * 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.webservices.atomprotocol; + +import java.io.ByteArrayInputStream; +import java.util.Collections; +import java.util.List; + +import com.rometools.propono.atom.server.AtomException; +import com.rometools.propono.atom.server.AtomNotFoundException; +import com.rometools.propono.atom.server.AtomRequest; +import com.rometools.rome.feed.atom.Content; +import com.rometools.rome.feed.atom.Entry; +import com.rometools.rome.feed.atom.Feed; +import org.apache.roller.weblogger.business.FileContentManager; +import org.apache.roller.weblogger.business.MediaFileManager; +import org.apache.roller.weblogger.business.WeblogManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.pojos.User; +import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.pojos.WeblogPermission; +import org.apache.roller.weblogger.util.RollerMessages; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class MediaCollectionPathTest { + + private MockedStatic factory; + private MockedStatic runtimeConfig; + private MediaFileManager files; + private FileContentManager content; + private Weblog weblog; + private MediaFileDirectory defaultDir; + private MediaCollection collection; + + @BeforeEach + void setUp() throws Exception { + factory = mockStatic(WebloggerFactory.class); + runtimeConfig = mockStatic(WebloggerRuntimeConfig.class); + runtimeConfig.when(WebloggerRuntimeConfig::getAbsoluteContextURL) + .thenReturn("https://blog.example"); + + files = mock(MediaFileManager.class); + content = mock(FileContentManager.class); + WeblogManager weblogs = mock(WeblogManager.class); + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getMediaFileManager()).thenReturn(files); + when(weblogger.getFileContentManager()).thenReturn(content); + when(weblogger.getWeblogManager()).thenReturn(weblogs); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + + weblog = mock(Weblog.class); + when(weblog.getHandle()).thenReturn("blog"); + when(weblog.getName()).thenReturn("Blog"); + when(weblog.hasUserPermission(any(), eq(WeblogPermission.POST))).thenReturn(true); + when(weblogs.getWeblogByHandle("blog")).thenReturn(weblog); + + defaultDir = mock(MediaFileDirectory.class); + when(defaultDir.getMediaFiles()).thenReturn(Collections.emptySet()); + when(files.getDefaultMediaFileDirectory(weblog)).thenReturn(defaultDir); + when(files.getMediaFileDirectoryByName(weblog, "default")).thenReturn(defaultDir); + + collection = new MediaCollection(new User(), "https://blog.example/roller-services/app"); + } + + @AfterEach + void tearDown() { + runtimeConfig.close(); + factory.close(); + } + + @Test + void namedCollectionIsLookedUpByItsName() throws Exception { + Feed feed = collection.getCollection(request("/blog/resources/default")); + + assertTrue(feed.getEntries().isEmpty()); + verify(files).getMediaFileDirectoryByName(weblog, "default"); + } + + @Test + void unknownCollectionIsNotFound() { + assertThrows(AtomNotFoundException.class, + () -> collection.getCollection(request("/blog/resources/missing"))); + } + + @Test + void mediaPostedToTheRootCollectionGoesToTheDefaultDirectory() throws Exception { + refuseUploads(); + + AtomException refused = assertThrows(AtomException.class, + () -> collection.postMedia(request("/blog/resources"), mediaEntry(null))); + + assertTrue(refused.getMessage().contains("refused"), refused.getMessage()); + verify(files).getDefaultMediaFileDirectory(weblog); + } + + @Test + void mediaPostedToAnUnknownDirectoryIsNotFound() { + assertThrows(AtomNotFoundException.class, + () -> collection.postMedia(request("/blog/resources/missing"), mediaEntry("a"))); + } + + @Test + void mediaWithoutSlugOrTitleIsNamedByDate() throws Exception { + refuseUploads(); + + assertThrows(AtomException.class, + () -> collection.postMedia(request("/blog/resources/default"), mediaEntry(null))); + + ArgumentCaptor name = ArgumentCaptor.forClass(String.class); + verify(content).canSave(eq(weblog), name.capture(), eq("image/png"), anyLong(), any()); + assertTrue(name.getValue().matches("blog-\\d+\\.png"), name.getValue()); + verify(files, never()).createMediaFile(any(), any(), any()); + } + + private void refuseUploads() throws Exception { + when(content.canSave(any(), anyString(), anyString(), anyLong(), any())) + .thenAnswer(call -> { + call.getArgument(4).addError("refused"); + return false; + }); + } + + private static AtomRequest request(String pathInfo) throws Exception { + AtomRequest request = mock(AtomRequest.class); + when(request.getPathInfo()).thenReturn(pathInfo); + when(request.getInputStream()).thenReturn(new ByteArrayInputStream(new byte[]{1, 2, 3})); + return request; + } + + private static Entry mediaEntry(String title) { + Content body = new Content(); + body.setType("image/png"); + Entry entry = new Entry(); + entry.setTitle(title); + entry.setContents(List.of(body)); + return entry; + } +} From d84d4b620674ee4e986d99f01d61a412fff44f31 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 12:34:41 -0400 Subject: [PATCH 7/9] Use Planet setting defaults before Planet Config is first saved On a new site the planet.site.* properties do not exist until Planet Config is saved. PlanetRuntimeConfig.getProperty() then threw, logged a warning with a stack trace and returned null, and the Planet feed printed $utils.escapeXML($siteName) as its title and description. Return the default from the Planet config definitions for a property that has not been saved, and pass empty strings to the feed template. --- CHANGES.md | 5 +- .../planet/config/PlanetRuntimeConfig.java | 27 ++++++- .../rendering/servlets/PlanetFeedServlet.java | 4 +- .../config/PlanetRuntimeConfigTest.java | 74 +++++++++++++++++++ 4 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/planet/config/PlanetRuntimeConfigTest.java diff --git a/CHANGES.md b/CHANGES.md index 04ca40de20..9ce6868e7b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,12 +25,15 @@ was refused.** Before, only a duplicate name was reported. Any other error, and on the blogroll dialogs even a duplicate name, closed the dialog as if the save had worked, or ended on an error page. - - **AtomPub media collections work.** Listing a media collection by the URL in the service document (`/resources/default`) failed with a server error. So did uploading media with no `Slug` header and no title, uploading to `/resources` itself, or uploading with a very short `Slug`. Unknown media directories now answer 404. +- **The Planet feed has a title before Planet Config is first saved.** On a new + site, `/planetrss` printed `$utils.escapeXML($siteName)` as its title and + description, and logged a warning for each. Unsaved Planet settings now use + their defaults. ## 6.1.6 diff --git a/app/src/main/java/org/apache/roller/planet/config/PlanetRuntimeConfig.java b/app/src/main/java/org/apache/roller/planet/config/PlanetRuntimeConfig.java index 75565135b5..173060113d 100644 --- a/app/src/main/java/org/apache/roller/planet/config/PlanetRuntimeConfig.java +++ b/app/src/main/java/org/apache/roller/planet/config/PlanetRuntimeConfig.java @@ -25,9 +25,12 @@ import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.business.PropertiesManager; import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.runtime.ConfigDef; +import org.apache.roller.weblogger.config.runtime.PropertyDef; import org.apache.roller.weblogger.config.runtime.RuntimeConfigDefs; import org.apache.roller.weblogger.config.runtime.RuntimeConfigDefsParser; import org.apache.roller.weblogger.planet.ui.PlanetConfig; +import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; /** @@ -45,8 +48,10 @@ private PlanetRuntimeConfig() {} /** - * Retrieve a single property from the PropertiesManager ... returns null - * if there is an error + * Retrieve a single property from the PropertiesManager. A property that + * has not been saved yet, as on a new site before Planet Config is saved, + * returns its default from the Planet config definitions. Returns null if + * there is an error. **/ public static String getProperty(String name) { @@ -54,7 +59,8 @@ public static String getProperty(String name) { try { PropertiesManager pmgr = WebloggerFactory.getWeblogger().getPropertiesManager(); - value = pmgr.getProperty(name).getValue(); + RuntimeConfigProperty prop = pmgr.getProperty(name); + value = (prop != null) ? prop.getValue() : getDefaultValue(name); } catch(Exception e) { log.warn("Trouble accessing property: "+name, e); } @@ -65,6 +71,21 @@ public static String getProperty(String name) { } + /** The default value of a Planet property, or null if it has no definition. */ + static String getDefaultValue(String name) { + RuntimeConfigDefs defs = getRuntimeConfigDefs(); + if (defs != null) { + for (ConfigDef configDef : defs.getConfigDefs()) { + PropertyDef def = configDef.getPropertyDef(name); + if (def != null) { + return def.getDefaultValue(); + } + } + } + return null; + } + + /** * Retrieve a property as a boolean ... defaults to false if there is an error **/ diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java index 432a0c33a1..e4e01e2262 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java @@ -148,10 +148,10 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) model.put("lastModified", lastModified); model.put("siteName", - PlanetRuntimeConfig.getProperty("planet.site.name")); + StringUtils.defaultString(PlanetRuntimeConfig.getProperty("planet.site.name"))); model.put("siteDescription", - PlanetRuntimeConfig.getProperty("planet.site.description")); + StringUtils.defaultString(PlanetRuntimeConfig.getProperty("planet.site.description"))); if (StringUtils.isNotEmpty(WebloggerRuntimeConfig diff --git a/app/src/test/java/org/apache/roller/planet/config/PlanetRuntimeConfigTest.java b/app/src/test/java/org/apache/roller/planet/config/PlanetRuntimeConfigTest.java new file mode 100644 index 0000000000..f850f6e3aa --- /dev/null +++ b/app/src/test/java/org/apache/roller/planet/config/PlanetRuntimeConfigTest.java @@ -0,0 +1,74 @@ +/* + * 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.planet.config; + +import org.apache.roller.weblogger.business.PropertiesManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +class PlanetRuntimeConfigTest { + + private MockedStatic factory; + private PropertiesManager properties; + + @BeforeEach + void setUp() { + factory = mockStatic(WebloggerFactory.class); + properties = mock(PropertiesManager.class); + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getPropertiesManager()).thenReturn(properties); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + } + + @AfterEach + void tearDown() { + factory.close(); + } + + @Test + void savedPropertyIsReturned() throws Exception { + when(properties.getProperty("planet.site.name")) + .thenReturn(new RuntimeConfigProperty("planet.site.name", "My Planet")); + + assertEquals("My Planet", PlanetRuntimeConfig.getProperty("planet.site.name")); + } + + @Test + void unsavedPropertyFallsBackToItsDefault() throws Exception { + when(properties.getProperty("planet.site.name")).thenReturn(null); + + assertEquals("Roller Planet", PlanetRuntimeConfig.getProperty("planet.site.name")); + } + + @Test + void unknownPropertyIsNull() throws Exception { + when(properties.getProperty("planet.no.such.property")).thenReturn(null); + + assertNull(PlanetRuntimeConfig.getProperty("planet.no.such.property")); + } +} From 7238d9617b31cb94bc42f68eac6bfb1aab8b1b2c Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 12:48:53 -0400 Subject: [PATCH 8/9] Stop the rich text editor pasting a copied image twice Summernote 0.8.12 inserts a pasted image file itself but does not cancel the paste, so the browser also inserted the clipboard's HTML copy of the image. Cancel the browser's paste when Summernote handles an image file. --- CHANGES.md | 2 ++ .../WEB-INF/jsps/editor/EntryEditor.jsp | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 9ce6868e7b..b3336fc417 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,8 @@ site, `/planetrss` printed `$utils.escapeXML($siteName)` as its title and description, and logged a warning for each. Unsaved Planet settings now use their defaults. +- **An image pasted into the rich text editor appears once.** Pasting an image + copied from a web page inserted it twice. ## 6.1.6 diff --git a/app/src/main/webapp/WEB-INF/jsps/editor/EntryEditor.jsp b/app/src/main/webapp/WEB-INF/jsps/editor/EntryEditor.jsp index e58d61539b..2d38c37b6b 100644 --- a/app/src/main/webapp/WEB-INF/jsps/editor/EntryEditor.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/editor/EntryEditor.jsp @@ -159,7 +159,24 @@ ['misc', ['codeview']], ['insert', ['link']] ], - height: 400 + height: 400, + callbacks: { + onPaste: function (e) { + // Summernote inserts a pasted image file itself, but it + // does not stop the browser from also pasting the image's + // HTML, so an image copied from a web page appeared twice. + // Use the same clipboard item Summernote picks. + var clipboard = (e.originalEvent || e).clipboardData; + if (!clipboard || !clipboard.items || !clipboard.items.length) { + return; + } + var items = clipboard.items; + var item = items.length > 1 ? items[1] : items[0]; + if (item.kind === 'file' && item.type.indexOf('image/') !== -1) { + e.preventDefault(); + } + } + } } ); // Added event listener to confirm once the editor content is changed From dec26dd86aff06683f4d20231c51cdd64a7fa9a5 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 13:27:30 -0400 Subject: [PATCH 9/9] Allow decimal values in the configuration page's float fields The float settings (maximum upload file and directory size, in MB) were rendered as number inputs with the default step of 1, so browsers refused values such as 0.5 or 2.5. Use step="any". --- CHANGES.md | 3 +++ app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index b3336fc417..f202622e70 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -36,6 +36,9 @@ their defaults. - **An image pasted into the rich text editor appears once.** Pasting an image copied from a web page inserted it twice. +- **Decimal settings can be saved on the configuration page.** The maximum + upload file and directory sizes accepted only whole numbers in the browser, + although they are measured in megabytes with decimals (default `2.00`). ## 6.1.6 diff --git a/app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp b/app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp index fc3494608e..cbecee8590 100644 --- a/app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp @@ -92,7 +92,7 @@
-