Fix bugs found while testing the 6.1.7 integration build

CHANGES.md

@@ -10,6 +10,36 @@
   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.
+- **Renaming a blogroll no longer reports a system error.** The rename was
+  saved, but the page showed "System error - check logs".
+- **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.
+- **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.
+- **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
 
 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.

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,16 +48,19 @@ 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) {
 
         String value = null;
 
         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
      **/

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

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);
 

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<MediaFile> files = dir.getMediaFiles();
 
             SortedSet<MediaFile> sortedSet = new TreeSet<>(new Comparator<Object>() {

app/src/main/webapp/WEB-INF/jsps/admin/GlobalConfig.jsp

@@ -92,7 +92,7 @@
                         <s:text name="%{#pd.key}"/>
                     </label>
                     <div class="col-sm-9 controls">
-                        <input type="number" name='<s:property value="#pd.name" />' size="5"
+                        <input type="number" step="any" name='<s:property value="#pd.name" />' size="5"
                                value='<s:property value="properties[#pd.name].value"/>'
                                id='globalConfig_<s:property value="#pd.nameWithUnderbars" />'
                                class="form-control float" onkeyup="formChanged()"/>

app/src/main/webapp/WEB-INF/jsps/admin/PingTargets.jsp

@@ -277,12 +277,12 @@
 
         }).done(function (data) {
 
-            // kludge: scrape response status from HTML returned by Struts
-            var alertEnd = data.indexOf("ALERT_END");
-            var notUnique = data.indexOf("<s:text name='pingTarget.nameNotUnique' />");
-            if (notUnique > 0 && notUnique < alertEnd) {
+            refreshSalt(data);
+
+            var errors = actionErrors(data);
+            if (errors.length > 0) {
                 feedbackAreaEdit.css("color", "red");
-                feedbackAreaEdit.html('<s:text name="pingTarget.nameNotUnique" />');
+                feedbackAreaEdit.text(errors.join(" "));
 
             } else {
                 feedbackAreaEdit.css("color", "green");
@@ -293,7 +293,7 @@
                 viewChanged();
             }
 
-        }).error(function (data) {
+        }).fail(function (data) {
             feedbackAreaEdit.html('<s:text name="generic.error.check.logs" />');
             feedbackAreaEdit.css("color", "red");
         });

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
                 <s:text name="bookmarksForm.blogrollName"/>
             </label>
             <div class="col-sm-9 controls">
-                <div class="form-control"><s:text name="%{folder.name}"/></div>
+                <div class="form-control"><s:property value="folder.name"/></div>
             </div>
         </div>
 
@@ -72,31 +72,38 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
                 <s:text name="bookmarksForm.blogrollName"/>
             </label>
             <div class="col-sm-9 controls">
-                <input style="width:55%; float:left" type="text" name="folder.name"
-                       value="<s:text name='%{folder.name}'/>" id="bookmarks_folder_name" class="form-control"
-                       onchange="nameChanged()"
-                       onkeyup="nameChanged()"/>
-                <button type="button" id="rename_button"
-                        class="btn btn-success" style="float:left; margin-left:1em;"
-                        onclick="renameFolder(); return false;"
-                        onsubmit="return false;">
-                    <s:text name="generic.rename"/>
-                </button>
-                <button type="button" id="rename_cancel"
-                        class="btn btn-default" style="float:left; margin-left:1em;"
-                        onclick="cancelRenameFolder(); return false;"
-                        onsubmit="return false;">
-                    <s:text name="generic.cancel"/>
-                </button>
+                <div style="display:flex; gap:1em;">
+                    <input type="text" name="folder.name" style="flex:1;"
+                           value="<s:property value='folder.name'/>" id="bookmarks_folder_name" class="form-control"
+                           onchange="nameChanged()"
+                           onkeyup="nameChanged()"/>
+                    <button type="button" id="rename_button" class="btn btn-success"
+                            onclick="renameFolder(); return false;"
+                            onsubmit="return false;">
+                        <s:text name="generic.rename"/>
+                    </button>
+                    <button type="button" id="rename_cancel" class="btn btn-default"
+                            onclick="cancelRenameFolder(); return false;"
+                            onsubmit="return false;">
+                        <s:text name="generic.cancel"/>
+                    </button>
+                </div>
             </div>
         </div>
 
     </s:if>
 
     <%-- allow user to select the bookmark folder to view --%>
 
-    <s:select name="viewFolderId" list="allFolders" listKey="id" listValue="name" emptyOption="true"
-              label="%{getText('bookmarksForm.switchTo')}" onchange="viewChanged()" onmouseup="viewChanged()"/>
+    <div class="form-group ">
+        <label class="col-sm-3 control-label" for="bookmarks_viewFolderId">
+            <s:text name="bookmarksForm.switchTo"/>
+        </label>
+        <div class="col-sm-9 controls">
+            <s:select name="viewFolderId" list="allFolders" listKey="id" listValue="name" emptyOption="true"
+                      theme="simple" cssClass="form-control" onchange="viewChanged()"/>
+        </div>
+    </div>
 
     <table class="rollertable table table-striped">
 
@@ -307,18 +314,18 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
 
         }).done(function (data, status, response) {
 
-            // kludge: scrape response status from HTML returned by Struts
-            var alertEnd = data.indexOf("ALERT_END");
-            var notUnique = data.indexOf('<s:text name="bookmarkForm.error.duplicateName" />');
-            if (notUnique > 0 && notUnique < alertEnd) {
-                alert('<s:text name="bookmarkForm.error.duplicateName" />');
+            refreshSalt(data);
+
+            var errors = actionErrors(data);
+            if (errors.length > 0) {
+                alert(errors.join("\n"));
 
             } else {
                 originalName = newName;
                 nameChanged();
             }
 
-        }).error(function (data) {
+        }).fail(function (data) {
             alert('<s:text name="generic.error.check.logs" />');
         });
     }
@@ -472,12 +479,12 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
 
         }).done(function (data, status, response) {
 
-            // kludge: scrape response status from HTML returned by Struts
-            var alertEnd = data.indexOf("ALERT_END");
-            var notUnique = data.indexOf('<s:text name="bookmarkForm.error.duplicateName" />');
-            if (notUnique > 0 && notUnique < alertEnd) {
+            refreshSalt(data);
+
+            var errors = actionErrors(data);
+            if (errors.length > 0) {
                 feedbackAreaBlogrollEdit.css("color", "red");
-                feedbackAreaBlogrollEdit.html('<s:text name="bookmarkForm.error.duplicateName" />');
+                feedbackAreaBlogrollEdit.text(errors.join(" "));
 
             } else {
                 feedbackAreaBlogrollEdit.css("color", "green");
@@ -494,7 +501,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('<s:text name="generic.error.check.logs" />');
             feedbackAreaBlogrollEdit.css("color", "red");
         });
@@ -819,12 +826,12 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
 
         }).done(function (data) {
 
-            // kludge: scrape response status from HTML returned by Struts
-            var alertEnd = data.indexOf("ALERT_END");
-            var notUnique = data.indexOf('<s:text name="bookmarkForm.error.duplicateName" />');
-            if (notUnique > 0 && notUnique < alertEnd) {
+            refreshSalt(data);
+
+            var errors = actionErrors(data);
+            if (errors.length > 0) {
                 feedbackAreaEdit.css("color", "red");
-                feedbackAreaEdit.html('<s:text name="bookmarkForm.error.duplicateName" />');
+                feedbackAreaEdit.text(errors.join(" "));
 
             } else {
                 feedbackAreaEdit.css("color", "green");
@@ -838,7 +845,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('<s:text name="generic.error.check.logs" />');
             feedbackAreaEdit.css("color", "red");
         });

app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp

@@ -205,24 +205,20 @@
 
         }).done(function (data) {
 
-            // kludge: scrape response status from HTML returned by Struts
-            var alertEnd = data.indexOf("ALERT_END");
-            var notUnique = data.indexOf('<s:text name="categoryForm.error.duplicateName" />');
-            var notValid = data.indexOf('<s:text name="categoryForm.error.invalidName" />');
-            if (notUnique > 0 && notUnique < alertEnd) {
-                feedbackAreaEdit.css("color", "red");
-                feedbackAreaEdit.html('<s:text name="categoryForm.error.duplicateName" />');
-            } else if (notValid > 0 && notValid < alertEnd) {
+            refreshSalt(data);
+
+            var errors = actionErrors(data);
+            if (errors.length > 0) {
                 feedbackAreaEdit.css("color", "red");
-                feedbackAreaEdit.html('<s:text name="categoryForm.error.invalidName" />');
+                feedbackAreaEdit.text(errors.join(" "));
             } else {
                 feedbackAreaEdit.css("color", "green");
                 feedbackAreaEdit.html('<s:text name="generic.success" />');
                 $('#category-edit-modal').modal("hide");
                 location.reload(true);
             }
 
-        }).error(function (data) {
+        }).fail(function (data) {
             feedbackAreaEdit.html('<s:text name="generic.error.check.logs" />');
             feedbackAreaEdit.css("color", "red");
         });