From 3191df13f412e2bec55dd9cf39ecf8f0bcf4193c Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 10 Oct 2026 17:54:31 -0400 Subject: [PATCH] Fix AtomPub media naming, paging and XML output, and editor menus Found while running a 7.0.0 snapshot on rollerweblogger.org. - Media uploads: derive the file extension from the media type without parameters or a +xml/+json suffix (image/svg+xml gives .svg), and keep a Slug's own extension when it matches the type. - Media collections: put the slash before the offset in the feed id and the next/previous links of named directories. - AtomWriter: leave out characters that XML 1.0 does not allow, in text and attribute values. - Media edit-media URIs: look the file up by directory and name; uploads no longer store the directory name as the file's original path. - Entry editor: upgrade Summernote to 0.9.1, which writes its menu icons as ; 0.8.12 wrote , which jQuery 3.5+ leaves open so it hides the menu labels. Limit the panel chevron in roller.css to collapse toggles, so it no longer lands on the editor's menu items. --- CHANGES.md | 17 +++ app/pom.xml | 6 +- .../business/jpa/JPAMediaFileManagerImpl.java | 3 + .../webservices/atomprotocol/AtomWriter.java | 77 +++++++---- .../atomprotocol/MediaCollection.java | 130 ++++++++++++++---- .../main/webapp/WEB-INF/jsps/tiles/head.jsp | 4 +- .../main/webapp/roller-ui/styles/roller.css | 6 +- .../weblogger/ui/EntryEditorAssetsTest.java | 80 +++++++++++ .../atomprotocol/AtomWriterTest.java | 51 +++++++ .../atomprotocol/MediaCollectionTest.java | 22 +++ .../atomprotocol/RollerAtomProtocolTest.java | 77 +++++++++++ docs/roller-atompub-guide.adoc | 4 +- 12 files changed, 418 insertions(+), 59 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/EntryEditorAssetsTest.java diff --git a/CHANGES.md b/CHANGES.md index 26460a850..1dff5a6f1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,6 +17,23 @@ every AtomPub URI, permission, element and status code, with examples that the test suite checks against the code. +### Bug fixes + +- **AtomPub fixes found while running a 7.0.0 snapshot.** + - Uploaded media files get the usual extension for their type: + `image/svg+xml` gives `.svg`, parameters such as `charset` are ignored, + and a `Slug` that already has a matching extension keeps it. + - The `next` and `previous` links of a named media directory include the + slash before the offset, so paging through it works. + - Characters that XML 1.0 does not allow are left out of AtomPub + responses, so one stored control character no longer makes a whole page + unreadable. + - A file uploaded over AtomPub can be read back through its `edit-media` + URI; it answered 404. +- **The rich text editor's menus show their labels again.** Summernote is + upgraded to 0.9.1, and the panel chevron no longer appears on the + editor's menu items. + ### Behaviour changes worth reading before upgrading - **AtomPub no longer uses ROME Propono.** Roller now has its own AtomPub diff --git a/app/pom.xml b/app/pom.xml index e2ddac3ed..65c5002de 100644 --- a/app/pom.xml +++ b/app/pom.xml @@ -277,10 +277,12 @@ limitations under the License. 1.14.2+1 + - org.webjars.bower + org.webjars.npm summernote - 0.8.12 + 0.9.1 diff --git a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAMediaFileManagerImpl.java b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAMediaFileManagerImpl.java index 85a19cd44..e3c0ddcaa 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAMediaFileManagerImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAMediaFileManagerImpl.java @@ -461,6 +461,9 @@ public MediaFile getMediaFileByPath(Weblog weblog, String path) } else { mdir = getDefaultMediaFileDirectory(weblog); } + if (mdir == null) { + return null; + } if (slash != -1) { fileName = fileName.substring(slash + 1); } diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriter.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriter.java index c309dd61e..0d5e8b238 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriter.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriter.java @@ -62,9 +62,9 @@ public void writeEntry(OutputStream out, AtomEntry entry) throws AtomException { public void writeCategoriesDoc(OutputStream out, AtomCategories cats) throws AtomException { writeDocument(out, APP_NS, "categories", "atom", ATOM_NS, "Error serializing category document", w -> { - w.writeAttribute("fixed", cats.isFixed() ? "yes" : "no"); + w.writeAttribute("fixed", xml(cats.isFixed() ? "yes" : "no")); if (cats.getScheme() != null) { - w.writeAttribute("scheme", cats.getScheme()); + w.writeAttribute("scheme", xml(cats.getScheme())); } for (AtomCategory cat : cats.getCategories()) { writeCategory(w, cat); @@ -151,24 +151,24 @@ private void writeCollection(XMLStreamWriter w, AtomCollection collection) throws XMLStreamException { w.writeStartElement(APP_NS, "collection"); if (collection.getHref() != null) { - w.writeAttribute("href", collection.getHref()); + w.writeAttribute("href", xml(collection.getHref())); } writeAtomText(w, "title", collection.getTitle()); for (String accept : collection.getAccepts()) { w.writeStartElement(APP_NS, "accept"); - w.writeCharacters(accept); + w.writeCharacters(xml(accept)); w.writeEndElement(); } for (AtomCategories cats : collection.getCategories()) { if (cats.getHref() != null) { w.writeEmptyElement(APP_NS, "categories"); - w.writeAttribute("href", cats.getHref()); + w.writeAttribute("href", xml(cats.getHref())); continue; } w.writeStartElement(APP_NS, "categories"); - w.writeAttribute("fixed", cats.isFixed() ? "yes" : "no"); + w.writeAttribute("fixed", xml(cats.isFixed() ? "yes" : "no")); if (cats.getScheme() != null) { - w.writeAttribute("scheme", cats.getScheme()); + w.writeAttribute("scheme", xml(cats.getScheme())); } for (AtomCategory cat : cats.getCategories()) { writeCategory(w, cat); @@ -204,32 +204,32 @@ private void writeEntryBody(XMLStreamWriter w, AtomEntry entry) throws XMLStream } if (entry.getInReplyToRef() != null) { w.writeEmptyElement(THREAD_NS, "in-reply-to"); - w.writeAttribute("ref", entry.getInReplyToRef()); + w.writeAttribute("ref", xml(entry.getInReplyToRef())); if (entry.getInReplyToHref() != null) { - w.writeAttribute("href", entry.getInReplyToHref()); + w.writeAttribute("href", xml(entry.getInReplyToHref())); } } if (entry.getMobileRendition() != null) { w.writeStartElement(ROLLER_NS, "rendition"); - w.writeAttribute("type", "mobile"); - w.writeCharacters(entry.getMobileRendition()); + w.writeAttribute("type", xml("mobile")); + w.writeCharacters(xml(entry.getMobileRendition())); w.writeEndElement(); } for (Map.Entry ext : entry.getExtensions().entrySet()) { w.writeStartElement(ROLLER_NS, ext.getKey()); - w.writeCharacters(ext.getValue()); + w.writeCharacters(xml(ext.getValue())); w.writeEndElement(); } // APP extensions (RFC 5023): app:edited is a child of atom:entry // (section 10.2); app:draft goes inside app:control (section 13.1) if (entry.getEdited() != null) { w.writeStartElement(APP_NS, "edited"); - w.writeCharacters(formatDate(entry.getEdited())); + w.writeCharacters(xml(formatDate(entry.getEdited()))); w.writeEndElement(); } w.writeStartElement(APP_NS, "control"); w.writeStartElement(APP_NS, "draft"); - w.writeCharacters(entry.isDraft() ? "yes" : "no"); + w.writeCharacters(xml(entry.isDraft() ? "yes" : "no")); w.writeEndElement(); w.writeEndElement(); } @@ -238,12 +238,12 @@ private void writeContent(XMLStreamWriter w, String name, AtomContent content) throws XMLStreamException { w.writeStartElement(ATOM_NS, name); if (content.getType() != null) { - w.writeAttribute("type", content.getType()); + w.writeAttribute("type", xml(content.getType())); } if (content.getSrc() != null) { - w.writeAttribute("src", content.getSrc()); + w.writeAttribute("src", xml(content.getSrc())); } else if (content.getValue() != null) { - w.writeCharacters(content.getValue()); + w.writeCharacters(xml(content.getValue())); } w.writeEndElement(); } @@ -251,13 +251,13 @@ private void writeContent(XMLStreamWriter w, String name, AtomContent content) private void writeLink(XMLStreamWriter w, AtomLink link) throws XMLStreamException { w.writeStartElement(ATOM_NS, "link"); if (link.getRel() != null) { - w.writeAttribute("rel", link.getRel()); + w.writeAttribute("rel", xml(link.getRel())); } if (link.getHref() != null) { - w.writeAttribute("href", link.getHref()); + w.writeAttribute("href", xml(link.getHref())); } if (link.getType() != null) { - w.writeAttribute("type", link.getType()); + w.writeAttribute("type", xml(link.getType())); } w.writeEndElement(); } @@ -265,13 +265,13 @@ private void writeLink(XMLStreamWriter w, AtomLink link) throws XMLStreamExcepti private void writeCategory(XMLStreamWriter w, AtomCategory cat) throws XMLStreamException { w.writeStartElement(ATOM_NS, "category"); if (cat.getTerm() != null) { - w.writeAttribute("term", cat.getTerm()); + w.writeAttribute("term", xml(cat.getTerm())); } if (cat.getScheme() != null) { - w.writeAttribute("scheme", cat.getScheme()); + w.writeAttribute("scheme", xml(cat.getScheme())); } if (cat.getLabel() != null) { - w.writeAttribute("label", cat.getLabel()); + w.writeAttribute("label", xml(cat.getLabel())); } w.writeEndElement(); } @@ -282,7 +282,36 @@ private void writeAtomText(XMLStreamWriter w, String name, String text) return; } w.writeStartElement(ATOM_NS, name); - w.writeCharacters(text); + w.writeCharacters(xml(text)); w.writeEndElement(); } + + /** + * Drops the characters that XML 1.0 does not allow in a document (most + * control characters, U+FFFE, U+FFFF and unpaired surrogates). Stored + * text can contain them, and a single one makes the whole response + * unreadable for a client. + */ + static String xml(String text) { + if (text == null) { + return null; + } + StringBuilder out = null; + int i = 0; + while (i < text.length()) { + int cp = text.codePointAt(i); + int len = Character.charCount(cp); + boolean legal = cp == 0x9 || cp == 0xA || cp == 0xD + || (cp >= 0x20 && cp <= 0xD7FF) + || (cp >= 0xE000 && cp <= 0xFFFD) + || (cp >= 0x10000 && cp <= 0x10FFFF); + if (!legal && out == null) { + out = new StringBuilder(text.length()).append(text, 0, i); + } else if (legal && out != null) { + out.appendCodePoint(cp); + } + i += len; + } + return out == null ? text : out.toString(); + } } 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 7626ff701..901eb844a 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 @@ -27,6 +27,8 @@ import java.util.Comparator; import java.util.Date; import java.util.List; +import java.util.Locale; +import java.util.Map; import java.util.Set; import java.util.SortedSet; import java.util.StringTokenizer; @@ -140,7 +142,6 @@ public AtomEntry postMedia(AtomRequest areq, AtomEntry entry) throws AtomExcepti mf.setDirectory(mdir); mf.setWeblog(website); mf.setName(fileName); - mf.setOriginalPath(justPath); mf.setInputStream(fis); mf.setLength(tempFile.length()); @@ -234,10 +235,12 @@ public AtomMediaResource getMediaResource(AtomRequest areq) throws AtomException try { // Parse pathinfo to determine file path String filePath = filePathFromPathInfo(pathInfo); - MediaFile mf = fmgr.getMediaFileByOriginalPath(website, filePath); + // The edit-media URI names the file by directory and name + MediaFile mf = fmgr.getMediaFileByPath(website, filePath); if (mf == null) { throw new AtomNotFoundException("Cannot find media file: " + filePath); } + mf.setContent(roller.getFileContentManager().getFileContent(website, mf.getId())); return createMediaResource(mf, response); } catch (AtomException ae) { throw ae; @@ -281,8 +284,7 @@ public AtomFeed getCollection(AtomRequest areq) throws AtomException { } AtomFeed feed = new AtomFeed(); - feed.setId(atomURL - +"/"+website.getHandle() + "/resources/" + path + start); + feed.setId(collectionURI(website, path, start)); feed.setTitle(website.getName()); List links = new ArrayList<>(); @@ -344,8 +346,7 @@ else if (f1.getLastModified() == f2.getLastModified()) { if (start + count < files.size()) { // add next link int nextOffset = start + max; - String url = atomURL - +"/"+ website.getHandle() + "/resources/" + path + nextOffset; + String url = collectionURI(website, path, nextOffset); AtomLink nextLink = new AtomLink(); nextLink.setRel("next"); nextLink.setHref(url); @@ -354,8 +355,7 @@ else if (f1.getLastModified() == f2.getLastModified()) { if (start > 0) { // add previous link int prevOffset = start > max ? start - max : 0; - String url = atomURL - +"/"+website.getHandle() + "/resources/" + path + prevOffset; + String url = collectionURI(website, path, prevOffset); AtomLink prevLink = new AtomLink(); prevLink.setRel("previous"); prevLink.setHref(url); @@ -610,14 +610,7 @@ private String createFileName(Weblog weblog, String title, String contentType) { throw new IllegalArgumentException("contentType cannot be null"); } - String fileName; - - // Determine the extension based on the contentType. This is a hack. - // The info we need to map from contentType to file extension is in - // JRE/lib/content-type.properties, but Java Activation doesn't provide - // a way to do a reverse mapping or to get at the data. - String[] typeTokens = contentType.split("/"); - String ext = typeTokens[1]; + String ext = fileExtension(contentType); if (title != null && !title.isBlank()) { // We've got a title, so use it to build file name @@ -630,19 +623,100 @@ private String createFileName(Weblog weblog, String title, String contentType) { tmp = (tmp == null) ? s : tmp + "_" + s; count++; } - if (!tmp.endsWith("." + ext)) { - fileName = tmp + "." + ext; - } else { - fileName = tmp; - } - } else { - // No title or text, so instead we'll use the item's date - // in YYYYMMDD format to form the file name - SimpleDateFormat sdf = new SimpleDateFormat(); - sdf.applyPattern("yyyyMMddHHSS"); - fileName = weblog.getHandle()+"-"+sdf.format(new Date())+"."+ext; + return withExtension(tmp, ext); + } + + // No title or text, so instead we'll use the item's date + // in YYYYMMDD format to form the file name + SimpleDateFormat sdf = new SimpleDateFormat(); + sdf.applyPattern("yyyyMMddHHSS"); + return weblog.getHandle() + "-" + sdf.format(new Date()) + "." + ext; + } + + /** + * The URI of one page of a media collection: /{handle}/resources/{offset} + * for the default directory, /{handle}/resources/{dir}/{offset} for a + * named one. getCollection() reads the last segment as the offset. + */ + private String collectionURI(Weblog website, String path, int offset) { + StringBuilder uri = new StringBuilder(atomURL).append('/') + .append(website.getHandle()).append("/resources/"); + if (StringUtils.isNotEmpty(path)) { + uri.append(path).append('/'); } + return uri.append(offset).toString(); + } + + /** Usual file extensions for types whose subtype is not the extension. */ + private static final Map EXTENSIONS = Map.ofEntries( + Map.entry("image/jpeg", "jpg"), + Map.entry("image/pjpeg", "jpg"), + Map.entry("image/svg+xml", "svg"), + Map.entry("application/ld+json", "jsonld"), + Map.entry("image/x-icon", "ico"), + Map.entry("image/vnd.microsoft.icon", "ico"), + Map.entry("image/tiff", "tif"), + Map.entry("text/plain", "txt"), + Map.entry("text/markdown", "md"), + Map.entry("text/javascript", "js"), + Map.entry("application/javascript", "js"), + Map.entry("audio/mpeg", "mp3"), + Map.entry("audio/mp4", "m4a"), + Map.entry("video/quicktime", "mov"), + Map.entry("application/msword", "doc"), + Map.entry("application/vnd.ms-powerpoint", "ppt"), + Map.entry("application/vnd.ms-excel", "xls"), + Map.entry("application/x-shockwave-flash", "swf"), + Map.entry("application/octet-stream", "bin")); + + /** Other extensions that name the same type as the usual one. */ + private static final Map> ALIASES = Map.of( + "jpg", Set.of("jpeg", "jpe"), + "tif", Set.of("tiff"), + "html", Set.of("htm"), + "txt", Set.of("text"), + "mpeg", Set.of("mpg")); - return fileName; + /** + * The file extension for a media type: parameters such as charset and a + * structured-syntax suffix such as {@code +xml} are dropped, and common + * types map to their usual extension ({@code image/svg+xml} gives + * {@code svg}, {@code image/jpeg} gives {@code jpg}). + */ + static String fileExtension(String contentType) { + String type = MediaTypePolicy.normalizeType(contentType); + if (type == null) { + return "bin"; + } + String known = EXTENSIONS.get(type); + if (known != null) { + return known; + } + int slash = type.indexOf('/'); + String subtype = slash > -1 ? type.substring(slash + 1) : type; + int plus = subtype.indexOf('+'); + if (plus > -1) { + subtype = subtype.substring(0, plus); + } + if (subtype.startsWith("x-")) { + subtype = subtype.substring(2); + } + String ext = subtype.replaceAll("[^a-z0-9]", ""); + return ext.isEmpty() ? "bin" : ext; + } + + /** + * Adds the extension to a base name, unless the name already ends with + * it or with another extension for the same type. + */ + static String withExtension(String baseName, String ext) { + int dot = baseName.lastIndexOf('.'); + if (dot > -1) { + String own = baseName.substring(dot + 1).toLowerCase(Locale.ROOT); + if (own.equals(ext) || ALIASES.getOrDefault(ext, Set.of()).contains(own)) { + return baseName; + } + } + return baseName + "." + ext; } } 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 7eba8f701..3a81ee8d1 100644 --- a/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/tiles/head.jsp @@ -41,8 +41,8 @@ You can override it with your own file via WEB-INF/tiles-def.xml - - + + diff --git a/app/src/main/webapp/roller-ui/styles/roller.css b/app/src/main/webapp/roller-ui/styles/roller.css index 452b7004b..e826fd05c 100644 --- a/app/src/main/webapp/roller-ui/styles/roller.css +++ b/app/src/main/webapp/roller-ui/styles/roller.css @@ -90,13 +90,15 @@ table.mm_table_actions td { /* edit entry page */ -.panel-heading a:after { +/* Only the panel collapse toggles, not links in other toolbars that use + .panel-heading, such as the rich text editor's menus */ +.panel-heading a[data-toggle="collapse"]:after { font-family:'Glyphicons Halflings'; content:"\e114"; float: right; color: grey; } -.panel-heading a.collapsed:after { +.panel-heading a[data-toggle="collapse"].collapsed:after { content:"\e080"; } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/EntryEditorAssetsTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/EntryEditorAssetsTest.java new file mode 100644 index 000000000..8ee6b3992 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/EntryEditorAssetsTest.java @@ -0,0 +1,80 @@ +/* + * 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; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Checks the rich text editor's menus: Summernote must write its menu icons + * as {@code }, and Roller's panel chevron must not land on the + * editor toolbar's menu items. + */ +public class EntryEditorAssetsTest { + + private static final Path WEBAPP = Paths.get("src/main/webapp"); + + @Test + public void summernoteClosesItsIconElements() throws IOException { + String head = read(WEBAPP.resolve("WEB-INF/jsps/tiles/head.jsp")); + Matcher script = Pattern.compile("/webjars/(summernote/[^/]+/dist/summernote\\.min\\.js)") + .matcher(head); + assertTrue(script.find(), "head.jsp loads Summernote"); + + String resource = "META-INF/resources/webjars/" + script.group(1); + String js; + try (InputStream in = getClass().getClassLoader().getResourceAsStream(resource)) { + assertNotNull(in, "the Summernote webjar named in head.jsp is on the classpath: " + resource); + js = new String(in.readAllBytes(), StandardCharsets.UTF_8); + } + // Summernote 0.8 built icons as . jQuery 3.5+ keeps that + // as an open , which swallows the menu label after it. + assertFalse(Pattern.compile("class=\"'\\+\\w+\\+'\"/>").matcher(js).find(), + "Summernote must not write icons as self-closing "); + } + + @Test + public void panelChevronIsOnlyOnCollapseToggles() throws IOException { + String css = read(WEBAPP.resolve("roller-ui/styles/roller.css")); + Matcher rule = Pattern.compile("([^{}]*\\.panel-heading\\s+a[^{}]*:after)\\s*\\{").matcher(css); + boolean found = false; + while (rule.find()) { + found = true; + assertTrue(rule.group(1).contains("[data-toggle=\"collapse\"]"), + "rule must be limited to collapse toggles: " + rule.group(1).trim()); + } + assertTrue(found, "roller.css still styles the panel collapse toggles"); + } + + private static String read(Path file) throws IOException { + return new String(Files.readAllBytes(file), StandardCharsets.UTF_8); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriterTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriterTest.java index 9f39933b5..ce200be5b 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriterTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomWriterTest.java @@ -20,6 +20,7 @@ import static org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.APP_NS; import static org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.ATOM_NS; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -27,6 +28,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.Arrays; import java.util.Date; @@ -273,4 +275,53 @@ public void testWriteCategoriesDocument() throws Exception { assertEquals("yes", doc.getDocumentElement().getAttribute("fixed")); assertEquals("Travel", element(doc, ATOM_NS, "category").getAttribute("term")); } + + @Test + public void testCharactersIllegalInXmlAreDropped() throws Exception { + AtomEntry comment = new AtomEntry(); + comment.setId("urn:comment:1"); + comment.setTitle("Comment"); + comment.setUpdated(UPDATED); + comment.setContent(text("text", "before\u0014after")); + AtomPerson author = new AtomPerson(); + author.setName("Pat\u0001"); + comment.getAuthors().add(author); + comment.setExtension("referrer", "https://example.com/?q=\uFFFE"); + AtomEntry entry = new AtomEntry(); + entry.setId("urn:entry:1"); + entry.setTitle("Title\u0000 with \uD800lone surrogate \uD83D\uDE00"); + entry.setUpdated(UPDATED); + entry.getLinks().add(new AtomLink("alternate", "https://example.com/a\u001Fb")); + AtomFeed feed = new AtomFeed(); + feed.setId("urn:feed"); + feed.setTitle("Comments"); + feed.setUpdated(UPDATED); + feed.getEntries().add(comment); + feed.getEntries().add(entry); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new AtomWriter().writeFeed(out, feed); + + Document doc = parse(out.toByteArray()); + String xml = out.toString(StandardCharsets.UTF_8); + assertTrue(xml.contains("beforeafter"), xml); + assertTrue(xml.contains("Title with lone surrogate \uD83D\uDE00"), xml); + assertTrue(xml.contains("https://example.com/ab"), xml); + assertEquals("feed", doc.getDocumentElement().getLocalName()); + } + + @Test + public void testXmlFilterKeepsLegalText() { + String legal = "tab\tnl\ncr\r \u00e9 \uE000 \uFFFD \uD83D\uDE00"; + assertSame(legal, AtomWriter.xml(legal)); + assertNull(AtomWriter.xml(null)); + assertEquals("", AtomWriter.xml("\u0000\u0008\u000B\u000C\uFFFF\uDC00")); + } + + private static AtomContent text(String type, String value) { + AtomContent content = new AtomContent(); + content.setType(type); + content.setValue(value); + return content; + } } diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java index 868bd85d3..2047b1dc3 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java @@ -59,6 +59,28 @@ void passiveStoredTypeRemainsInline() throws Exception { verify(response).setContentType("text/plain"); } + @Test + void fileExtensionComesFromTheMediaType() { + assertEquals("svg", MediaCollection.fileExtension("image/svg+xml")); + assertEquals("txt", MediaCollection.fileExtension("text/plain; charset=UTF-8")); + assertEquals("jpg", MediaCollection.fileExtension("IMAGE/JPEG")); + assertEquals("png", MediaCollection.fileExtension("image/png")); + assertEquals("jsonld", MediaCollection.fileExtension("application/ld+json")); + assertEquals("atom", MediaCollection.fileExtension("application/atom+xml")); + assertEquals("pdf", MediaCollection.fileExtension("application/pdf")); + assertEquals("bin", MediaCollection.fileExtension("application/octet-stream")); + } + + @Test + void aNameKeepsAnExtensionThatMatchesTheType() { + assertEquals("diagram.svg", MediaCollection.withExtension("diagram.svg", "svg")); + assertEquals("photo.jpeg", MediaCollection.withExtension("photo.jpeg", "jpg")); + assertEquals("photo.JPG", MediaCollection.withExtension("photo.JPG", "jpg")); + assertEquals("roller-atompub-model.svg", + MediaCollection.withExtension("roller-atompub-model", "svg")); + assertEquals("notes.md.txt", MediaCollection.withExtension("notes.md", "txt")); + } + private MediaFile mediaFile(String name, String type) { MediaFile mediaFile = mock(MediaFile.class); when(mediaFile.getName()).thenReturn(name); diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomProtocolTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomProtocolTest.java index da0a7f8a3..6a7518023 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomProtocolTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomProtocolTest.java @@ -229,6 +229,83 @@ public void testServiceDocument() throws Exception { assertTrue(out.size() > 0); } + @Test + public void testMediaCollectionNextLinksLeadToTheNextPage() throws Exception { + MediaFileManager mfm = WebloggerFactory.getWeblogger().getMediaFileManager(); + mfm.createMediaFileDirectory( + WebloggerFactory.getWeblogger().getWeblogManager().getWeblogByHandle(HANDLE), + "diagrams"); + TestUtils.endSession(true); + + for (String dir : new String[] {"/resources/diagrams", "/resources"}) { + uploadImages(dir, 21); + + AtomFeed first = new MediaCollection(managedUser(), ATOM_URL) + .getCollection(request("/" + HANDLE + dir, null, null, null)); + assertEquals(20, first.getEntries().size(), dir); + assertEquals(ATOM_URL + "/" + HANDLE + dir + "/0", first.getId(), dir); + String next = linkHref(first, "next"); + assertEquals(ATOM_URL + "/" + HANDLE + dir + "/20", next, dir); + + // follow the next link, as a client would + AtomFeed second = new MediaCollection(managedUser(), ATOM_URL) + .getCollection(request(next.substring(ATOM_URL.length()), null, null, null)); + assertEquals(1, second.getEntries().size(), dir); + assertEquals(ATOM_URL + "/" + HANDLE + dir + "/0", linkHref(second, "previous"), dir); + assertNull(linkHref(second, "next"), dir); + TestUtils.endSession(true); + } + } + + @Test + public void testUploadedMediaCanBeReadThroughItsEditMediaUri() throws Exception { + MediaFileManager mfm = WebloggerFactory.getWeblogger().getMediaFileManager(); + mfm.createMediaFileDirectory( + WebloggerFactory.getWeblogger().getWeblogManager().getWeblogByHandle(HANDLE), + "diagrams"); + TestUtils.endSession(true); + + for (String dir : new String[] {"/resources/diagrams", "/resources"}) { + AtomEntry created = uploadImages(dir, 1).get(0); + String editMedia = created.getLinkHref("edit-media"); + assertNotNull(editMedia, dir); + + AtomMediaResource resource = new MediaCollection(managedUser(), ATOM_URL) + .getMediaResource(request(editMedia.substring(ATOM_URL.length()), null, null, null)); + try (java.io.InputStream in = resource.getInputStream()) { + assertEquals("png-0", new String(in.readAllBytes(), StandardCharsets.UTF_8), dir); + } + TestUtils.endSession(true); + } + } + + private java.util.List uploadImages(String dir, int count) throws Exception { + java.util.List created = new java.util.ArrayList<>(); + ServletContext servletContext = mock(ServletContext.class); + when(servletContext.getMimeType(anyString())).thenReturn("image/png"); + try (MockedStatic rollerContext = + mockStatic(RollerContext.class, CALLS_REAL_METHODS)) { + rollerContext.when(RollerContext::getServletContext).thenReturn(servletContext); + for (int i = 0; i < count; i++) { + AtomEntry mediaIn = new AtomEntry(); + AtomContent content = new AtomContent(); + content.setType("image/png"); + mediaIn.setContent(content); + String slug = "image" + i; + created.add(new MediaCollection(managedUser(), ATOM_URL).postMedia( + request("/" + HANDLE + dir, "image/png", slug, + ("png-" + i).getBytes(StandardCharsets.UTF_8)), mediaIn)); + TestUtils.endSession(true); + } + } + return created; + } + + private static String linkHref(AtomFeed feed, String rel) { + return feed.getLinks().stream().filter(l -> rel.equals(l.getRel())) + .map(AtomLink::getHref).findFirst().orElse(null); + } + @Test public void testMediaUpload() throws Exception { byte[] bytes = "fake-png-bytes".getBytes(StandardCharsets.UTF_8); diff --git a/docs/roller-atompub-guide.adoc b/docs/roller-atompub-guide.adoc index bc7a04284..9414684f3 100644 --- a/docs/roller-atompub-guide.adoc +++ b/docs/roller-atompub-guide.adoc @@ -234,7 +234,9 @@ Slug: sunset ---- * Without `Slug`, Roller names the file from the weblog handle and the date. -The file name extension comes from the media type. +The file name extension comes from the media type, without parameters or a +`+xml`/`+json` suffix (`image/svg+xml` gives `.svg`, `image/jpeg` gives +`.jpg`). A `Slug` that already ends in a matching extension keeps it. * The media type must be one the site allows; the service document lists them in each media collection's `app:accept`. Roller stores the type it derives from the file name and contents, not only the one the client sends.