Fix bugs found while testing the 6.1.7 integration build

[snoopdave]

Summary

This PR collects general bug fixes found during manual testing of a local 6.1.7 integration branch. That branch combined roller-6.1.x with the open 6.1.7 PRs (#198, #199, #200, #205) and was run on Tomcat 9, JDK 11 and MySQL 8. Most of the bugs are much older than 6.1.7; none were introduced by those PRs. Each fix is a separate commit.

Blogroll, category and ping target dialogs

Each Roller page carries one form salt, shared by all of its forms, and a salt can be used only once. These pages save a dialog with AJAX and then post the page form again. The second post sent the salt the AJAX request had already used, so it was refused and the user got an error page. A retry after an error in the dialog failed the same way.

• Salt for AJAX responses. head.jsp now issues the response's salt in a roller-salt meta tag on every UI page. The new refreshSalt(html) in theme/scripts/roller.js reads it from an AJAX response (parsed with DOMParser, so no scripts run) and copies it into every salt field on the page. It is called first in each AJAX done handler:

  • Bookmarks.jsp: rename blogroll, add blogroll, save bookmark
  • Categories.jsp: save category
  • PingTargets.jsp: save ping target

  The meta tag is needed because some error responses, such as a refused blogroll save, have no forms.

• Dialogs show why a save was refused. They looked only for a duplicate-name message, so any other refusal, such as a malformed ping URL, closed the dialog as if the save had worked. The add and rename blogroll dialogs looked for the bookmark message, while FolderEdit reports folderForm.error.duplicateName, so a refused blogroll name was posted on and ended on an error page. All five dialogs now show the action errors in the response, read with the new actionErrors(html).

• Renaming a blogroll threw a NullPointerException (since 2181cb7, 2021). FolderEdit.save() reads folderId for the folderId response header, but folderId is set only when adding. The rename was saved first, so it worked but reported "System error - check logs". The header now uses the saved folder's id.

• jQuery 3 has no .error(). The five AJAX calls used .error(...), which threw, so their failure handlers never ran. They now use .fail(...).

• "Switch to blogroll" no longer submits on mouseup. Chrome fires mouseup when the list opens, so the page reloaded before a blogroll could be picked. onchange is kept.

• Blogroll page layout. The "Blogroll name" and "Switch to blogroll" rows now use the same label and control columns. The rename buttons sit on the name field's line, using flexbox instead of floats. The folder name is shown with s:property; s:text treated it as a message key and logged a warning on every page view.

AtomPub media collections

These bugs date from 2013 and are in MediaCollection:

• The advertised collection could not be listed. getCollection() added a path separator to the directory name, so the lookup asked for default/, found nothing and threw a NullPointerException. This affected GET /roller-services/app/<blog>/resources/default, the URL in the service document. The name is now looked up as is, and an unknown directory answers 404.
• Media POST without Slug or title threw in replaceNonAlphanumeric(null). The name is now left null, and createFileName() names the file by date.
• Media POST to /resources had no directory name and threw. It now uses the default directory, and an unknown directory answers 404.
• Temporary upload file. postMedia() used the client's file name as the createTempFile() prefix, and createTempFile() refuses a prefix shorter than three characters. It now uses a random prefix, as putMedia() already does.

Planet feed before Planet Config is saved

On a new site, the planet.site.* properties don't exist until Planet Config is saved. PlanetRuntimeConfig.getProperty() then threw, logged a warning and returned null, so /planetrss printed $utils.escapeXML($siteName) as its title and description. A setting that hasn't been saved now returns its default from planetRuntimeConfigDefs.xml, and PlanetFeedServlet passes empty strings in place of nulls.

Pasted images appeared twice

Summernote 0.8.12's pasteByEvent inserts a pasted image file itself but never calls preventDefault(). So when the clipboard holds both HTML and an image file, as after "Copy Image" in a browser, the browser also pasted the HTML <img>, and each paste produced two images. EntryEditor.jsp now has an onPaste callback that cancels the browser's paste whenever Summernote handles an image file, using the same clipboard item Summernote picks.

Decimal values on the configuration page

The maximum upload file and directory sizes are in megabytes with decimals (default 2.00), but they were number inputs with the default step of 1, so browsers refused values such as 0.5. GlobalConfig.jsp now uses step="any".

Testing

• mvn -pl app clean test on JDK 11: 342 tests, 0 failures, 1 skipped.

• New PlanetRuntimeConfigTest (3 tests): a saved setting is returned, a setting that hasn't been saved falls back to its default, and an unknown setting is null. The fallback test fails on the old code.

• New MediaCollectionPathTest (5 tests): a named collection is looked up by its name; an unknown collection or directory is not found; a post to /resources uses the default directory; a post with no name is named by date. All 5 fail on the old code.

• New FolderEditSaveTest (2 tests): renaming saves the folder and sends its id, and adding sends the new id. The rename test fails without the fix (expected <success> but was <input>).

• Manual, on the 6.1.7 integration build (Chrome, Tomcat 9, MySQL 8). These all work:

  • Blogrolls: adding one; a duplicate name followed by a retry; renaming, and renaming to a duplicate name; adding a bookmark straight after a rename.
  • Categories: a duplicate name followed by a retry.
  • Ping targets: an https URL is refused with "The URL is not properly formed.", and a retry with an http URL saves.
  • AtomPub: GET /resources/default lists the media files; media POSTs with no Slug or title, to /resources, and with a short Slug all return 201.
  • Planet feed: with the saved Planet settings removed, the feed shows "Roller Planet" and logs no warning.
  • Paste: pasting a copied image twice gives exactly two <img> tags.
  • Configuration: uploads.file.maxsize can be set to 0.30.

  No "Security Violation" was logged after the fix.

• Not changed here: ping target URLs must be http. JPAPingTargetManagerImpl.isUrlWellFormed refuses https.

• Still to check by hand: picking a blogroll from "Switch to blogroll".

Targets roller-6.1.x for 6.1.7. It will be cherry-picked to master after the 6.1.7 release.

[snoopdave]

Manually tested on the 6.1.7 integration build (Tomcat 9, JDK 11, MySQL 8). Pass.

• Blogroll:
  • Adding a blogroll no longer returns 500.
  • A duplicate name shows the error, and a retry saves.
  • Rename works, and so does adding a bookmark after a rename.
  • "Switch to blogroll" lets you pick a blogroll.
• Categories and ping targets: a duplicate name or invalid URL shows the error, and a retry saves.
• No "Security Violation" errors appear after the fix.
• AtomPub media:
  • Listing /resources/default returns 200, and unknown directories return 404.
  • Uploads with no Slug and no title, uploads to /resources, and uploads with Slug: ab all return 201.
• Planet feed: with no saved Planet settings, /planetrss shows the defaults ("Roller Planet") and logs no warning.
• mvn -pl app clean test on JDK 11: 342 tests, 0 failures.

[snoopdave]

Follow-up testing on the 6.1.7 integration build. Pass.

• 7238d96: an image copied from a web page and pasted into the rich text editor now goes in once, not twice. Checked with a real paste.
• dec26dd: the configuration page accepts decimal values, such as 0.30 MB for the upload limit. The browser had refused anything but whole numbers.
• Regression smoke test: login and logout, entry delete, a logged-out comment, the feeds, search, media folder create, move and delete, and 17 admin and author pages all work. No errors were logged.