From 6d95b74390cca7608311e2865670b7d161da8451 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 29 Aug 2026 16:40:56 -0400 Subject: [PATCH 1/2] Derive media content types from file content, not the request The type an upload declares is treated as a hint, and the stored type is derived from the file name through one shared MediaTypePolicy. Serving applies the other half of the policy: only a short list of passively-rendered formats is sent inline, everything else is sent as an attachment, and every media response carries nosniff. All paths that accept an upload and all paths that serve uploaded media route through the policy, including the entry editor's replacement-body path and the resource servlets' uploaded-media fallback. Files whose type is outside the inline list, such as CSS and JavaScript held as media, now download rather than render. That is a behaviour change and belongs in the release notes. Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV --- .../servlets/MediaResourceServlet.java | 7 +- .../servlets/PreviewResourceServlet.java | 18 +- .../rendering/servlets/ResourceServlet.java | 18 +- .../ui/struts2/editor/MediaFileAdd.java | 23 +- .../ui/struts2/editor/MediaFileEdit.java | 6 +- .../weblogger/util/MediaTypePolicy.java | 186 +++++++++++++++ .../atomprotocol/MediaCollection.java | 7 +- .../xmlrpc/MetaWeblogAPIHandler.java | 3 +- .../weblogger/util/MediaTypePolicyTest.java | 225 ++++++++++++++++++ 9 files changed, 466 insertions(+), 27 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java index 713e4c2d17..a42faec283 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/MediaResourceServlet.java @@ -31,6 +31,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.util.RollerConstants; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -116,7 +117,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) // set the content type based on whatever is in our web.xml mime defs if (resourceRequest.isThumbnail()) { - response.setContentType("image/png"); + MediaTypePolicy.applyResponseHeaders(response, "image/png", + mediaFile.getName()); try { resourceStream = mediaFile.getThumbnailInputStream(); } catch (Exception e) { @@ -131,7 +133,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } if (resourceStream == null) { - response.setContentType(mediaFile.getContentType()); + MediaTypePolicy.applyResponseHeaders(response, + mediaFile.getContentType(), mediaFile.getName()); resourceStream = mediaFile.getInputStream(); } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java index 7103bea9f6..71e2a09258 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java @@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -129,7 +130,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // if not from theme then see if resource is in weblog's upload dir + boolean fromUploadedMedia = false; if (resourceStream == null) { + fromUploadedMedia = true; try { MediaFileManager mmgr = WebloggerFactory.getWeblogger() .getMediaFileManager(); @@ -160,8 +163,19 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - response.setContentType(this.context.getMimeType(resourceRequest - .getResourcePath())); + String resourceType = this.context.getMimeType( + resourceRequest.getResourcePath()); + if (fromUploadedMedia) { + // Uploaded through the media library, so it is governed by the + // same policy as any other media response. + MediaTypePolicy.applyResponseHeaders(response, resourceType, + resourceRequest.getResourcePath()); + } else { + // A theme resource: authored as part of the theme and served as + // the type the theme intends, but never re-typed by the browser. + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setContentType(resourceType); + } try { // ok, lets serve up the file diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java index 8dbd5dba1e..97d07d2002 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -125,7 +126,9 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // if not from theme then see if resource is in weblog's upload dir + boolean fromUploadedMedia = false; if (resourceStream == null) { + fromUploadedMedia = true; try { MediaFileManager mmgr = WebloggerFactory.getWeblogger() .getMediaFileManager(); @@ -159,8 +162,19 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - response.setContentType(this.context.getMimeType(resourceRequest - .getResourcePath())); + String resourceType = this.context.getMimeType( + resourceRequest.getResourcePath()); + if (fromUploadedMedia) { + // Uploaded through the media library, so it is governed by the + // same policy as any other media response. + MediaTypePolicy.applyResponseHeaders(response, resourceType, + resourceRequest.getResourcePath()); + } else { + // A theme resource: authored as part of the theme and served as + // the type the theme intends, but never re-typed by the browser. + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setContentType(resourceType); + } try { // ok, lets serve up the file diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java index 881dad7549..b424590dab 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java @@ -35,6 +35,7 @@ import org.apache.roller.weblogger.pojos.MediaFileDirectory; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.RollerMessages.RollerMessage; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.util.Utilities; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -173,23 +174,11 @@ public String save() { mediaFile.setLength(this.uploadedFiles[i].length()); mediaFile.setInputStream(new FileInputStream( this.uploadedFiles[i])); - mediaFile - .setContentType(this.uploadedFilesContentType[i]); - - // in some cases Struts2 is not able to guess the content - // type correctly and assigns the default, which is - // octet-stream. So in cases where we see octet-stream - // we double check and see if we can guess the content - // type via the Java MIME type facilities. - mediaFile.setContentType(this.uploadedFilesContentType[i]); - if (mediaFile.getContentType() == null - || mediaFile.getContentType().endsWith("/octet-stream")) { - - String ctype = Utilities.getContentTypeFromFileName(mediaFile.getName()); - if (null != ctype) { - mediaFile.setContentType(ctype); - } - } + // The type the browser put on the part describes what + // the sender meant to send. It is taken as a hint and + // the stored type is worked out from the file name. + mediaFile.setContentType(MediaTypePolicy.storedTypeFor( + mediaFile.getName(), this.uploadedFilesContentType[i])); manager.createMediaFile(getActionWeblog(), mediaFile, errors); WebloggerFactory.getWeblogger().flush(); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java index 76ff2dbb7c..192cb3cbb5 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java @@ -23,6 +23,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.FileIOException; import org.apache.roller.weblogger.business.MediaFileManager; @@ -124,7 +125,10 @@ public String save() { if (uploadedFile != null) { mediaFile.setLength(this.uploadedFile.length()); - mediaFile.setContentType(this.uploadedFileContentType); + // Replacing the body re-decides the type, on the same + // terms as the original upload. + mediaFile.setContentType(MediaTypePolicy.storedTypeFor( + mediaFile.getName(), this.uploadedFileContentType)); manager.updateMediaFile(getActionWeblog(), mediaFile, new FileInputStream(this.uploadedFile)); } else { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java new file mode 100644 index 0000000000..e81fae392e --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java @@ -0,0 +1,186 @@ +/* + * 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.util; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Locale; +import java.util.Set; + +import javax.servlet.http.HttpServletResponse; + +/** + * Decides what type an uploaded file is stored as, and how it is served back. + * + *

A client uploading a file states a type, but the stored type is derived + * from the file name. The declared value is a hint only, consulted where the + * name yields nothing, and it cannot introduce a type the browser would + * execute. + * + *

Serving applies the second half. Only a short list of formats that + * browsers render passively are sent inline; everything else is sent as an + * attachment, and {@code nosniff} accompanies every response so browsers do + * not substitute their own type guess. + */ +public final class MediaTypePolicy { + + private MediaTypePolicy() { + } + + public static final String DEFAULT_TYPE = "application/octet-stream"; + + /** + * Formats browsers render without executing anything the file carries. + * SVG is deliberately absent: it is an XML document that can carry script. + */ + private static final Set INLINE_TYPES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "image/jpeg", "image/pjpeg", "image/png", "image/gif", + "image/bmp", "image/x-ms-bmp", "image/webp", "image/tiff", + "image/x-icon", "image/vnd.microsoft.icon", + "application/pdf"))); + + /** Families served inline whatever the subtype. */ + private static final String[] INLINE_PREFIXES = {"audio/", "video/"}; + + /** + * Types a browser may execute, or that can carry something it will. These + * are never adopted from a client's declaration. + */ + private static final Set ACTIVE_TYPES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "text/html", "application/xhtml+xml", "application/xhtml", + "image/svg+xml", "text/xml", "application/xml", + "text/javascript", "application/javascript", + "application/ecmascript", "text/ecmascript", + "text/vbscript", "application/x-shockwave-flash", + "text/xsl", "application/xslt+xml"))); + + /** + * @param fileName the uploaded file's name + * @param declaredType the type the client said it was, may be null + * @return the type to store: derived from the name where that is + * conclusive, otherwise the declared type if it is not one a + * browser would act on, otherwise the generic binary type + */ + public static String storedTypeFor(String fileName, String declaredType) { + String derived = normalize(deriveFromName(fileName)); + if (isConclusive(derived)) { + return derived; + } + + String declared = normalize(declaredType); + if (isConclusive(declared) && !isActive(declared)) { + return declared; + } + + return DEFAULT_TYPE; + } + + /** @return true when browsers render this type without executing it */ + public static boolean isInlineSafe(String contentType) { + String type = normalize(contentType); + if (type == null) { + return false; + } + if (INLINE_TYPES.contains(type)) { + return true; + } + for (String prefix : INLINE_PREFIXES) { + if (type.startsWith(prefix)) { + return true; + } + } + return false; + } + + /** @return true when a browser may execute this type, or script inside it */ + public static boolean isActive(String contentType) { + String type = normalize(contentType); + if (type == null) { + return false; + } + return ACTIVE_TYPES.contains(type) || type.endsWith("+xml"); + } + + /** + * Sets the type and the headers that govern how the response is treated. + * Anything outside the inline list is marked as an attachment. + */ + public static void applyResponseHeaders(HttpServletResponse response, + String contentType, String fileName) { + response.setHeader("X-Content-Type-Options", "nosniff"); + + String type = normalize(contentType); + if (type == null) { + type = DEFAULT_TYPE; + } + + if (isInlineSafe(type)) { + response.setContentType(type); + return; + } + + // Served as bytes to be saved rather than a document to be rendered. + response.setContentType(DEFAULT_TYPE); + response.setHeader("Content-Disposition", + "attachment; filename=\"" + headerSafe(fileName) + "\""); + } + + private static String deriveFromName(String fileName) { + if (fileName == null || fileName.trim().isEmpty()) { + return null; + } + try { + return Utilities.getContentTypeFromFileName(fileName); + } catch (Exception undetermined) { + return null; + } + } + + /** @return the bare type in lower case, without parameters such as charset */ + private static String normalize(String contentType) { + if (contentType == null) { + return null; + } + String type = contentType.trim(); + int semicolon = type.indexOf(';'); + if (semicolon > -1) { + type = type.substring(0, semicolon).trim(); + } + return type.isEmpty() ? null : type.toLowerCase(Locale.ENGLISH); + } + + private static boolean isConclusive(String type) { + return type != null && !DEFAULT_TYPE.equals(type); + } + + /** + * @return the name with the characters that would end the quoted string or + * start another header removed, since it is placed in one + */ + private static String headerSafe(String fileName) { + if (fileName == null || fileName.trim().isEmpty()) { + return "download"; + } + String safe = fileName.replaceAll("[\\r\\n\"\\\\]", ""); + return safe.trim().isEmpty() ? "download" : safe; + } +} 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 e7e963482e..37c9383157 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 @@ -47,6 +47,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.FileIOException; import org.apache.roller.weblogger.business.MediaFileManager; @@ -136,7 +137,8 @@ public Entry postMedia(AtomRequest areq, Entry entry) throws AtomException { mf.setWeblog(website); mf.setName(fileName); mf.setOriginalPath(justPath); - mf.setContentType(contentType); + mf.setContentType( + MediaTypePolicy.storedTypeFor(fileName, contentType)); mf.setInputStream(fis); mf.setLength(tempFile.length()); @@ -394,7 +396,8 @@ public void putMedia(AtomRequest areq) throws AtomException { // Attempt to load file, to ensure it exists MediaFile mf = fmgr.getMediaFileByPath(website, path); - mf.setContentType(contentType); + mf.setContentType( + MediaTypePolicy.storedTypeFor(mf.getName(), contentType)); mf.setInputStream(fis); mf.setLength(tempFile.length()); diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java index dc8ce3f698..34003c00b5 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java @@ -29,6 +29,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.util.RollerConstants; +import org.apache.roller.weblogger.util.MediaTypePolicy; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.URLStrategy; import org.apache.roller.weblogger.business.WeblogEntryManager; @@ -381,7 +382,7 @@ public Object newMediaObject(String blogid, String userid, String password, mf.setDirectory(root); mf.setWeblog(website); mf.setName(name); - mf.setContentType(type); + mf.setContentType(MediaTypePolicy.storedTypeFor(name, type)); mf.setInputStream(new ByteArrayInputStream(bits)); mf.setLength(bits.length); String fileLink = mf.getPermalink(); diff --git a/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java new file mode 100644 index 0000000000..1e6d095af9 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java @@ -0,0 +1,225 @@ +/* + * 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.util; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import javax.servlet.http.HttpServletResponse; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * What an uploaded file is stored as, and how it comes back. + * + *

The type a client puts on an upload states what the sender meant to send, + * not what the bytes are, and the browser acts on whatever Roller repeats back. + * These cases fix both ends of that: which type is kept, and which types are + * allowed to render as a document rather than download. + */ +public class MediaTypePolicyTest { + + // ----------------------------------------------------------------- // + // Stored type + // ----------------------------------------------------------------- // + + /** The file name decides, so a declaration cannot contradict it. */ + @Test + public void theFileNameDecidesTheStoredType() { + assertEquals("image/jpeg", + MediaTypePolicy.storedTypeFor("holiday.jpg", "text/html"), + "a declared type overrode the file name"); + assertEquals("image/png", + MediaTypePolicy.storedTypeFor("diagram.png", "image/svg+xml")); + assertEquals("image/gif", + MediaTypePolicy.storedTypeFor("loop.GIF", "application/xhtml+xml"), + "the extension must be matched regardless of case"); + } + + /** Where the name says nothing, an executable declaration is still refused. */ + @Test + public void anExecutableDeclarationIsNeverAdopted() { + for (String active : new String[]{ + "text/html", "text/html; charset=utf-8", "application/xhtml+xml", + "image/svg+xml", "application/xml", "text/javascript", + "application/javascript", "text/xsl", "something/custom+xml"}) { + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor("payload.unknownext", active), + "adopted an executable declared type: " + active); + } + } + + /** A harmless declaration is still useful where the name is opaque. */ + @Test + public void aHarmlessDeclarationIsUsedWhenTheNameIsOpaque() { + assertEquals("application/zip", + MediaTypePolicy.storedTypeFor("bundle.unknownext", "application/zip")); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor("bundle.unknownext", null)); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.storedTypeFor(null, null)); + } + + // ----------------------------------------------------------------- // + // Inline policy + // ----------------------------------------------------------------- // + + @Test + public void passiveFormatsRenderInline() { + for (String inline : new String[]{ + "image/jpeg", "image/png", "image/gif", "image/webp", + "application/pdf", "audio/mpeg", "video/mp4", + "image/png; charset=binary"}) { + assertTrue(MediaTypePolicy.isInlineSafe(inline), + "expected to render inline: " + inline); + } + } + + @Test + public void formatsThatCanCarryScriptDoNot() { + for (String blocked : new String[]{ + "image/svg+xml", "text/html", "application/xhtml+xml", + "text/xml", "application/javascript", "text/plain", + "application/zip", null, ""}) { + assertFalse(MediaTypePolicy.isInlineSafe(blocked), + "expected not to render inline: " + blocked); + } + } + + // ----------------------------------------------------------------- // + // Response headers + // ----------------------------------------------------------------- // + + @Test + public void everyResponseDeclaresNosniff() { + for (String type : new String[]{"image/png", "text/html", null}) { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, type, "f.bin"); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + } + } + + @Test + public void anInlineTypeKeepsItsTypeAndIsNotAnAttachment() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "image/png", "diagram.png"); + verify(response).setContentType("image/png"); + verify(response, never()).setHeader(eq("Content-Disposition"), anyString()); + } + + @Test + public void anythingElseIsSentAsAnAttachment() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "text/html", "page.html"); + verify(response).setContentType(MediaTypePolicy.DEFAULT_TYPE); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"page.html\""); + } + + /** The name is placed inside a header, so it cannot be allowed to leave it. */ + @Test + public void theAttachmentNameCannotBreakOutOfTheHeader() { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyResponseHeaders(response, "text/html", + "evil\r\nSet-Cookie: a=b\".html"); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"evilSet-Cookie: a=b.html\""); + } + + // ----------------------------------------------------------------- // + // The callers actually use it + // ----------------------------------------------------------------- // + + private String source(String relativePath) throws Exception { + Path path = Paths.get("src", "main", "java"); + for (String segment : relativePath.split("/")) { + path = path.resolve(segment); + } + assertTrue(Files.isReadable(path), + "cannot read " + path.toAbsolutePath() + " (run from the app module)"); + return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + } + + /** + * The serving path must not set a type of its own, or the headers above + * are decided somewhere this test cannot see. + */ + @Test + public void theServingPathGoesThroughThePolicy() throws Exception { + String servlet = source("org/apache/roller/weblogger/ui/rendering/" + + "servlets/MediaResourceServlet.java"); + assertTrue(servlet.contains("MediaTypePolicy.applyResponseHeaders"), + "MediaResourceServlet must apply the policy to its response"); + assertFalse(servlet.contains("response.setContentType("), + "MediaResourceServlet must not set a content type directly"); + } + + /** + * Uploaded media is also reachable through the two resource servlets, which + * serve theme resources from the same method. Only the uploaded-media + * branch takes the media policy — applying it to theme resources would send + * every stylesheet as a download — but both branches must refuse sniffing. + */ + @Test + public void theResourceServletsCoverTheirUploadedMediaBranch() throws Exception { + for (String name : new String[]{"ResourceServlet", "PreviewResourceServlet"}) { + String servlet = source("org/apache/roller/weblogger/ui/rendering/" + + "servlets/" + name + ".java"); + assertTrue(servlet.contains("MediaTypePolicy.applyResponseHeaders"), + name + " must apply the media policy to uploaded media"); + assertTrue(servlet.contains("fromUploadedMedia"), + name + " must distinguish uploaded media from theme resources"); + assertTrue(servlet.contains("X-Content-Type-Options"), + name + " must refuse sniffing on the theme branch too"); + } + } + + /** Each upload path must derive the stored type rather than take it. */ + @Test + public void everyUploadPathGoesThroughThePolicy() throws Exception { + String[][] callers = { + {"org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java", + "this.uploadedFilesContentType[i]"}, + {"org/apache/roller/weblogger/webservices/atomprotocol/" + + "MediaCollection.java", "setContentType(contentType)"}, + {"org/apache/roller/weblogger/webservices/xmlrpc/" + + "MetaWeblogAPIHandler.java", "setContentType(type)"}, + // Replacing an existing file's body is an upload too. + {"org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java", + "this.uploadedFileContentType"}, + }; + for (String[] caller : callers) { + String src = source(caller[0]); + assertTrue(src.contains("MediaTypePolicy.storedTypeFor"), + caller[0] + " must derive the stored type through the policy"); + assertFalse(src.contains("setContentType(" + caller[1] + ")"), + caller[0] + " still stores the client's declared type directly"); + } + } +} From 23db1ec852a2eb0f2faa3bd527bf7c6debbfe34f Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Wed, 2 Sep 2026 10:20:39 -0400 Subject: [PATCH 2/2] Complete media type handling across upload and serving paths --- .../business/themes/WeblogCustomTheme.java | 50 ++++++++++++- .../servlets/PreviewResourceServlet.java | 4 +- .../rendering/servlets/ResourceServlet.java | 4 +- .../ui/struts2/editor/MediaFileAdd.java | 11 ++- .../ui/struts2/editor/MediaFileEdit.java | 21 ++++-- .../weblogger/util/MediaTypePolicy.java | 53 +++++++++----- .../atomprotocol/MediaCollection.java | 51 ++++++++++--- .../atomprotocol/RollerAtomHandler.java | 4 +- .../xmlrpc/MetaWeblogAPIHandler.java | 9 ++- .../themes/WeblogCustomThemeTest.java | 60 ++++++++++++++++ .../weblogger/util/MediaTypePolicyTest.java | 72 ++++++++++++++++--- .../atomprotocol/MediaCollectionTest.java | 61 ++++++++++++++++ 12 files changed, 349 insertions(+), 51 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java b/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java index 7906990fdf..1416e02f23 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/themes/WeblogCustomTheme.java @@ -22,6 +22,7 @@ import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; +import java.io.InputStream; import java.util.Date; import java.util.List; import org.apache.roller.weblogger.pojos.MediaFile; @@ -159,12 +160,57 @@ public ThemeResource getResource(String path) { try { MediaFileManager mmgr = WebloggerFactory.getWeblogger().getMediaFileManager(); - MediaFile mf = mmgr.getMediaFileByOriginalPath( - this.weblog, path); + MediaFile mediaFile = mmgr.getMediaFileByOriginalPath(this.weblog, path); + if (mediaFile != null) { + resource = new MediaFileThemeResource(mediaFile); + } } catch (WebloggerException ex) { // ignored, resource considered not found } return resource; } + + private static final class MediaFileThemeResource implements ThemeResource { + private final MediaFile mediaFile; + + private MediaFileThemeResource(MediaFile mediaFile) { + this.mediaFile = mediaFile; + } + + @Override + public String getName() { + return mediaFile.getName(); + } + + @Override + public String getPath() { + return mediaFile.getOriginalPath(); + } + + @Override + public long getLastModified() { + return mediaFile.getLastModified(); + } + + @Override + public long getLength() { + return mediaFile.getLength(); + } + + @Override + public InputStream getInputStream() { + return mediaFile.getInputStream(); + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public int compareTo(ThemeResource other) { + return getPath().compareTo(other.getPath()); + } + } } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java index 71e2a09258..3fea57ff2b 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PreviewResourceServlet.java @@ -163,8 +163,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - String resourceType = this.context.getMimeType( - resourceRequest.getResourcePath()); + String resourceType = MediaTypePolicy.typeFromName( + resourceRequest.getResourcePath(), this.context::getMimeType); if (fromUploadedMedia) { // Uploaded through the media library, so it is governed by the // same policy as any other media response. diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java index 97d07d2002..4f793e7036 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServlet.java @@ -162,8 +162,8 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // set the content type based on whatever is in our web.xml mime defs - String resourceType = this.context.getMimeType( - resourceRequest.getResourcePath()); + String resourceType = MediaTypePolicy.typeFromName( + resourceRequest.getResourcePath(), this.context::getMimeType); if (fromUploadedMedia) { // Uploaded through the media library, so it is governed by the // same policy as any other media response. diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java index b424590dab..bee4996e4f 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java @@ -33,6 +33,7 @@ import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.RollerMessages.RollerMessage; import org.apache.roller.weblogger.util.MediaTypePolicy; @@ -177,8 +178,16 @@ public String save() { // The type the browser put on the part describes what // the sender meant to send. It is taken as a hint and // the stored type is worked out from the file name. + String declaredType = MediaTypePolicy.normalizeType( + this.uploadedFilesContentType[i]); + if (!WebloggerFactory.getWeblogger().getFileContentManager().canSave( + getActionWeblog(), fileName, declaredType, + this.uploadedFiles[i].length(), errors)) { + continue; + } mediaFile.setContentType(MediaTypePolicy.storedTypeFor( - mediaFile.getName(), this.uploadedFilesContentType[i])); + mediaFile.getName(), declaredType, + RollerContext.getServletContext()::getMimeType)); manager.createMediaFile(getActionWeblog(), mediaFile, errors); WebloggerFactory.getWeblogger().flush(); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java index 192cb3cbb5..fa089b6c0a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java @@ -30,6 +30,8 @@ import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.ui.core.RollerContext; +import org.apache.roller.weblogger.util.RollerMessages; import org.apache.struts2.convention.annotation.AllowedMethods; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -124,11 +126,20 @@ public String save() { bean.copyTo(mediaFile); if (uploadedFile != null) { - mediaFile.setLength(this.uploadedFile.length()); - // Replacing the body re-decides the type, on the same - // terms as the original upload. - mediaFile.setContentType(MediaTypePolicy.storedTypeFor( - mediaFile.getName(), this.uploadedFileContentType)); + mediaFile.setLength(this.uploadedFile.length()); + String declaredType = MediaTypePolicy.normalizeType( + this.uploadedFileContentType); + RollerMessages errors = new RollerMessages(); + if (!WebloggerFactory.getWeblogger().getFileContentManager().canSave( + getActionWeblog(), this.uploadedFileName, declaredType, + this.uploadedFile.length(), errors)) { + throw new FileIOException(errors.toString()); + } + // Replacing the body re-decides the type, on the same + // terms as the original upload. + mediaFile.setContentType(MediaTypePolicy.storedTypeFor( + this.uploadedFileName, declaredType, + RollerContext.getServletContext()::getMimeType)); manager.updateMediaFile(getActionWeblog(), mediaFile, new FileInputStream(this.uploadedFile)); } else { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java index e81fae392e..2c8a7ebee7 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/MediaTypePolicy.java @@ -52,10 +52,11 @@ private MediaTypePolicy() { */ private static final Set INLINE_TYPES = Collections.unmodifiableSet( new HashSet<>(Arrays.asList( - "image/jpeg", "image/pjpeg", "image/png", "image/gif", + "image/jpeg", "image/jpg", "image/pjpeg", "image/png", + "image/x-png", "image/apng", "image/avif", "image/gif", "image/bmp", "image/x-ms-bmp", "image/webp", "image/tiff", "image/x-icon", "image/vnd.microsoft.icon", - "application/pdf"))); + "application/pdf", "text/plain"))); /** Families served inline whatever the subtype. */ private static final String[] INLINE_PREFIXES = {"audio/", "video/"}; @@ -80,13 +81,14 @@ private MediaTypePolicy() { * conclusive, otherwise the declared type if it is not one a * browser would act on, otherwise the generic binary type */ - public static String storedTypeFor(String fileName, String declaredType) { - String derived = normalize(deriveFromName(fileName)); + public static String storedTypeFor(String fileName, String declaredType, + MimeTypeResolver resolver) { + String derived = typeFromName(fileName, resolver); if (isConclusive(derived)) { return derived; } - String declared = normalize(declaredType); + String declared = normalizeType(declaredType); if (isConclusive(declared) && !isActive(declared)) { return declared; } @@ -94,9 +96,14 @@ public static String storedTypeFor(String fileName, String declaredType) { return DEFAULT_TYPE; } + /** Resolves a file name through the servlet container's MIME mappings. */ + public static String typeFromName(String fileName, MimeTypeResolver resolver) { + return normalizeType(deriveFromName(fileName, resolver)); + } + /** @return true when browsers render this type without executing it */ public static boolean isInlineSafe(String contentType) { - String type = normalize(contentType); + String type = normalizeType(contentType); if (type == null) { return false; } @@ -113,7 +120,7 @@ public static boolean isInlineSafe(String contentType) { /** @return true when a browser may execute this type, or script inside it */ public static boolean isActive(String contentType) { - String type = normalize(contentType); + String type = normalizeType(contentType); if (type == null) { return false; } @@ -128,10 +135,7 @@ public static void applyResponseHeaders(HttpServletResponse response, String contentType, String fileName) { response.setHeader("X-Content-Type-Options", "nosniff"); - String type = normalize(contentType); - if (type == null) { - type = DEFAULT_TYPE; - } + String type = responseTypeFor(contentType); if (isInlineSafe(type)) { response.setContentType(type); @@ -144,19 +148,26 @@ public static void applyResponseHeaders(HttpServletResponse response, "attachment; filename=\"" + headerSafe(fileName) + "\""); } - private static String deriveFromName(String fileName) { - if (fileName == null || fileName.trim().isEmpty()) { + /** @return the response type after applying the inline policy. */ + public static String responseTypeFor(String contentType) { + String type = normalizeType(contentType); + return type != null && isInlineSafe(type) ? type : DEFAULT_TYPE; + } + + private static String deriveFromName(String fileName, MimeTypeResolver resolver) { + if (fileName == null || fileName.trim().isEmpty() || resolver == null) { return null; } - try { - return Utilities.getContentTypeFromFileName(fileName); - } catch (Exception undetermined) { - return null; + String name = fileName.trim(); + int dot = name.lastIndexOf('.'); + if (dot > -1) { + name = name.substring(0, dot) + name.substring(dot).toLowerCase(Locale.ENGLISH); } + return resolver.resolve(name); } /** @return the bare type in lower case, without parameters such as charset */ - private static String normalize(String contentType) { + public static String normalizeType(String contentType) { if (contentType == null) { return null; } @@ -172,6 +183,12 @@ private static boolean isConclusive(String type) { return type != null && !DEFAULT_TYPE.equals(type); } + /** Resolves a name with the MIME mappings supplied by the active servlet container. */ + @FunctionalInterface + public interface MimeTypeResolver { + String resolve(String fileName); + } + /** * @return the name with the characters that would end the quoted string or * start another header removed, since it is placed in 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 37c9383157..b4b100fb87 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 @@ -44,6 +44,7 @@ import java.util.StringTokenizer; import java.util.TreeSet; import java.util.UUID; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -59,6 +60,7 @@ 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.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.Utilities; @@ -70,6 +72,7 @@ public class MediaCollection { private Weblogger roller; private User user; + private HttpServletResponse response; private static final int MAX_ENTRIES = 20; private final String atomURL; @@ -78,8 +81,13 @@ public class MediaCollection { public MediaCollection(User user, String atomURL) { + this(user, atomURL, null); + } + + public MediaCollection(User user, String atomURL, HttpServletResponse response) { this.user = user; this.atomURL = atomURL; + this.response = response; this.roller = WebloggerFactory.getWeblogger(); } @@ -137,12 +145,18 @@ public Entry postMedia(AtomRequest areq, Entry entry) throws AtomException { mf.setWeblog(website); mf.setName(fileName); mf.setOriginalPath(justPath); - mf.setContentType( - MediaTypePolicy.storedTypeFor(fileName, contentType)); mf.setInputStream(fis); mf.setLength(tempFile.length()); RollerMessages errors = new RollerMessages(); + String declaredType = MediaTypePolicy.normalizeType(contentType); + if (!roller.getFileContentManager().canSave(website, fileName, + declaredType, tempFile.length(), errors)) { + throw new AtomException(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor(fileName, + declaredType, + RollerContext.getServletContext()::getMimeType)); fileMgr.createMediaFile(website, mf, errors); if (errors.getErrorCount() > 0) { throw new AtomException(errors.toString()); @@ -222,11 +236,7 @@ public AtomMediaResource getMediaResource(AtomRequest areq) throws AtomException // Parse pathinfo to determine file path String filePath = filePathFromPathInfo(pathInfo); MediaFile mf = fmgr.getMediaFileByOriginalPath(website, filePath); - return new AtomMediaResource( - mf.getName(), - mf.getLength(), - new Date(mf.getLastModified()), - mf.getInputStream()); + return createMediaResource(mf, response); } catch (Exception e) { throw new AtomException( "Unexpected error during file upload", e); @@ -396,8 +406,16 @@ public void putMedia(AtomRequest areq) throws AtomException { // Attempt to load file, to ensure it exists MediaFile mf = fmgr.getMediaFileByPath(website, path); - mf.setContentType( - MediaTypePolicy.storedTypeFor(mf.getName(), contentType)); + String replacementName = pathInfo[pathInfo.length - 1]; + String declaredType = MediaTypePolicy.normalizeType(contentType); + RollerMessages errors = new RollerMessages(); + if (!roller.getFileContentManager().canSave(website, + replacementName, declaredType, tempFile.length(), errors)) { + throw new FileIOException(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor( + replacementName, declaredType, + RollerContext.getServletContext()::getMimeType)); mf.setInputStream(fis); mf.setLength(tempFile.length()); @@ -482,6 +500,21 @@ private String filePathFromPathInfo(String[] pathInfo) { } return path; } + + static AtomMediaResource createMediaResource(MediaFile mediaFile, + HttpServletResponse response) throws IOException { + AtomMediaResource resource = new AtomMediaResource( + mediaFile.getName(), mediaFile.getLength(), + new Date(mediaFile.getLastModified()), mediaFile.getInputStream()); + resource.setContentType(mediaFile.getContentType()); + if (response != null) { + MediaTypePolicy.applyResponseHeaders(response, + mediaFile.getContentType(), mediaFile.getName()); + resource.setContentType( + MediaTypePolicy.responseTypeFor(mediaFile.getContentType())); + } + return resource; + } private Entry createAtomResourceEntry(Weblog website, MediaFile file) { String filePath = file.getPath().endsWith("/") diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java index 55e1c576ef..070741cfdd 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java @@ -96,6 +96,7 @@ public class RollerAtomHandler implements AtomHandler { protected User user = null; protected int maxEntries = 20; protected String atomURL = null; + private final HttpServletResponse response; protected static final boolean THROTTLE; @@ -115,6 +116,7 @@ public class RollerAtomHandler implements AtomHandler { * then user's name, otherwise it will return null. */ public RollerAtomHandler(HttpServletRequest request, HttpServletResponse response) { + this.response = response; roller = WebloggerFactory.getWeblogger(); String userName; @@ -253,7 +255,7 @@ public Entry getEntry(AtomRequest areq) throws AtomException { */ @Override public AtomMediaResource getMediaResource(AtomRequest areq) throws AtomException { - MediaCollection mcol = new MediaCollection(user, atomURL); + MediaCollection mcol = new MediaCollection(user, atomURL, response); return mcol.getMediaResource(areq); } diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java index 34003c00b5..e19ec52133 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/xmlrpc/MetaWeblogAPIHandler.java @@ -44,6 +44,7 @@ import org.apache.roller.weblogger.pojos.WeblogEntry; import org.apache.roller.weblogger.pojos.WeblogEntry.PubStatus; import org.apache.roller.weblogger.pojos.WeblogEntrySearchCriteria; +import org.apache.roller.weblogger.ui.core.RollerContext; import org.apache.roller.weblogger.util.RollerMessages; import org.apache.roller.weblogger.util.Utilities; import org.apache.xmlrpc.XmlRpcException; @@ -382,12 +383,18 @@ public Object newMediaObject(String blogid, String userid, String password, mf.setDirectory(root); mf.setWeblog(website); mf.setName(name); - mf.setContentType(MediaTypePolicy.storedTypeFor(name, type)); mf.setInputStream(new ByteArrayInputStream(bits)); mf.setLength(bits.length); String fileLink = mf.getPermalink(); RollerMessages errors = new RollerMessages(); + String declaredType = MediaTypePolicy.normalizeType(type); + if (!roller.getFileContentManager().canSave(website, name, + declaredType, bits.length, errors)) { + throw new Exception(errors.toString()); + } + mf.setContentType(MediaTypePolicy.storedTypeFor(name, declaredType, + RollerContext.getServletContext()::getMimeType)); fmgr.createMediaFile(website, mf, errors); if (errors.getErrorCount() > 0) { diff --git a/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java b/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java new file mode 100644 index 0000000000..0c48511177 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/business/themes/WeblogCustomThemeTest.java @@ -0,0 +1,60 @@ +/* + * 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. + */ +package org.apache.roller.weblogger.business.themes; + +import java.io.ByteArrayInputStream; + +import org.apache.roller.weblogger.business.MediaFileManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.pojos.ThemeResource; +import org.apache.roller.weblogger.pojos.Weblog; +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +class WeblogCustomThemeTest { + + @Test + void mediaFileBackedResourcesAreReturnedAsThemeResources() throws Exception { + Weblog weblog = mock(Weblog.class); + Weblogger roller = mock(Weblogger.class); + MediaFileManager manager = mock(MediaFileManager.class); + MediaFile mediaFile = mock(MediaFile.class); + ByteArrayInputStream content = new ByteArrayInputStream(new byte[]{1, 2, 3}); + + when(roller.getMediaFileManager()).thenReturn(manager); + when(manager.getMediaFileByOriginalPath(weblog, "css/site.css")) + .thenReturn(mediaFile); + when(mediaFile.getName()).thenReturn("site.css"); + when(mediaFile.getOriginalPath()).thenReturn("css/site.css"); + when(mediaFile.getLength()).thenReturn(3L); + when(mediaFile.getLastModified()).thenReturn(7L); + when(mediaFile.getInputStream()).thenReturn(content); + + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + + ThemeResource resource = new WeblogCustomTheme(weblog) + .getResource("css/site.css"); + + assertNotNull(resource); + assertEquals("site.css", resource.getName()); + assertEquals("css/site.css", resource.getPath()); + assertEquals(3L, resource.getLength()); + assertEquals(7L, resource.getLastModified()); + assertSame(content, resource.getInputStream()); + } + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java index 1e6d095af9..2cd9a850d6 100644 --- a/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/util/MediaTypePolicyTest.java @@ -53,12 +53,15 @@ public class MediaTypePolicyTest { @Test public void theFileNameDecidesTheStoredType() { assertEquals("image/jpeg", - MediaTypePolicy.storedTypeFor("holiday.jpg", "text/html"), + MediaTypePolicy.storedTypeFor("holiday.jpg", "text/html", + MediaTypePolicyTest::mimeTypeFor), "a declared type overrode the file name"); assertEquals("image/png", - MediaTypePolicy.storedTypeFor("diagram.png", "image/svg+xml")); + MediaTypePolicy.storedTypeFor("diagram.png", "image/svg+xml", + MediaTypePolicyTest::mimeTypeFor)); assertEquals("image/gif", - MediaTypePolicy.storedTypeFor("loop.GIF", "application/xhtml+xml"), + MediaTypePolicy.storedTypeFor("loop.GIF", "application/xhtml+xml", + MediaTypePolicyTest::mimeTypeFor), "the extension must be matched regardless of case"); } @@ -70,7 +73,8 @@ public void anExecutableDeclarationIsNeverAdopted() { "image/svg+xml", "application/xml", "text/javascript", "application/javascript", "text/xsl", "something/custom+xml"}) { assertEquals(MediaTypePolicy.DEFAULT_TYPE, - MediaTypePolicy.storedTypeFor("payload.unknownext", active), + MediaTypePolicy.storedTypeFor("payload.unknownext", active, + MediaTypePolicyTest::mimeTypeFor), "adopted an executable declared type: " + active); } } @@ -79,11 +83,14 @@ public void anExecutableDeclarationIsNeverAdopted() { @Test public void aHarmlessDeclarationIsUsedWhenTheNameIsOpaque() { assertEquals("application/zip", - MediaTypePolicy.storedTypeFor("bundle.unknownext", "application/zip")); + MediaTypePolicy.storedTypeFor("bundle.unknownext", "application/zip", + MediaTypePolicyTest::mimeTypeFor)); assertEquals(MediaTypePolicy.DEFAULT_TYPE, - MediaTypePolicy.storedTypeFor("bundle.unknownext", null)); + MediaTypePolicy.storedTypeFor("bundle.unknownext", null, + MediaTypePolicyTest::mimeTypeFor)); assertEquals(MediaTypePolicy.DEFAULT_TYPE, - MediaTypePolicy.storedTypeFor(null, null)); + MediaTypePolicy.storedTypeFor(null, null, + MediaTypePolicyTest::mimeTypeFor)); } // ----------------------------------------------------------------- // @@ -93,8 +100,9 @@ public void aHarmlessDeclarationIsUsedWhenTheNameIsOpaque() { @Test public void passiveFormatsRenderInline() { for (String inline : new String[]{ - "image/jpeg", "image/png", "image/gif", "image/webp", - "application/pdf", "audio/mpeg", "video/mp4", + "image/jpeg", "image/jpg", "image/png", "image/x-png", + "image/apng", "image/avif", "image/gif", "image/webp", + "application/pdf", "text/plain", "audio/mpeg", "video/mp4", "image/png; charset=binary"}) { assertTrue(MediaTypePolicy.isInlineSafe(inline), "expected to render inline: " + inline); @@ -105,7 +113,7 @@ public void passiveFormatsRenderInline() { public void formatsThatCanCarryScriptDoNot() { for (String blocked : new String[]{ "image/svg+xml", "text/html", "application/xhtml+xml", - "text/xml", "application/javascript", "text/plain", + "text/xml", "application/javascript", "application/zip", null, ""}) { assertFalse(MediaTypePolicy.isInlineSafe(blocked), "expected not to render inline: " + blocked); @@ -152,6 +160,35 @@ public void theAttachmentNameCannotBreakOutOfTheHeader() { "attachment; filename=\"evilSet-Cookie: a=b.html\""); } + @Test + public void servletMappingsAreUsedCaseInsensitively() { + assertEquals("application/pdf", MediaTypePolicy.typeFromName( + "report.PDF", MediaTypePolicyTest::mimeTypeFor)); + assertEquals("video/mp4", MediaTypePolicy.typeFromName( + "clip.MP4", MediaTypePolicyTest::mimeTypeFor)); + assertEquals(null, MediaTypePolicy.typeFromName( + "payload.unmapped", MediaTypePolicyTest::mimeTypeFor)); + } + + @Test + public void unknownAndActiveTypesUseTheDownloadType() { + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.responseTypeFor(null)); + assertEquals(MediaTypePolicy.DEFAULT_TYPE, + MediaTypePolicy.responseTypeFor("image/svg+xml")); + assertEquals("text/plain", + MediaTypePolicy.responseTypeFor("text/plain; charset=utf-8")); + } + + private static String mimeTypeFor(String fileName) { + if (fileName.endsWith(".jpg")) return "image/jpeg"; + if (fileName.endsWith(".png")) return "image/png"; + if (fileName.endsWith(".gif")) return "image/gif"; + if (fileName.endsWith(".pdf")) return "application/pdf"; + if (fileName.endsWith(".mp4")) return "video/mp4"; + return null; + } + // ----------------------------------------------------------------- // // The callers actually use it // ----------------------------------------------------------------- // @@ -218,8 +255,23 @@ public void everyUploadPathGoesThroughThePolicy() throws Exception { String src = source(caller[0]); assertTrue(src.contains("MediaTypePolicy.storedTypeFor"), caller[0] + " must derive the stored type through the policy"); + assertTrue(src.contains("getFileContentManager().canSave"), + caller[0] + " must validate the declared type before deriving it"); assertFalse(src.contains("setContentType(" + caller[1] + ")"), caller[0] + " still stores the client's declared type directly"); } } + + @Test + public void replacementRoutesUseTheReplacementName() throws Exception { + String editor = source("org/apache/roller/weblogger/ui/struts2/editor/" + + "MediaFileEdit.java"); + assertTrue(editor.contains("storedTypeFor(\n this.uploadedFileName")); + + String atom = source("org/apache/roller/weblogger/webservices/atomprotocol/" + + "MediaCollection.java"); + assertTrue(atom.contains("storedTypeFor(\n replacementName")); + assertTrue(atom.contains("createMediaResource(mf, response)"), + "Atom media reads must apply the response policy"); + } } 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 new file mode 100644 index 0000000000..c374d2adcb --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollectionTest.java @@ -0,0 +1,61 @@ +/* + * 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. + */ +package org.apache.roller.weblogger.webservices.atomprotocol; + +import java.io.ByteArrayInputStream; + +import javax.servlet.http.HttpServletResponse; + +import com.rometools.propono.atom.server.AtomMediaResource; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.util.MediaTypePolicy; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class MediaCollectionTest { + + @Test + void activeStoredTypeIsReturnedAsADownload() throws Exception { + MediaFile mediaFile = mediaFile("page.html", "text/html"); + HttpServletResponse response = mock(HttpServletResponse.class); + + AtomMediaResource resource = MediaCollection.createMediaResource(mediaFile, response); + + assertEquals(MediaTypePolicy.DEFAULT_TYPE, resource.getContentType()); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"page.html\""); + verify(response).setContentType(MediaTypePolicy.DEFAULT_TYPE); + } + + @Test + void passiveStoredTypeRemainsInline() throws Exception { + MediaFile mediaFile = mediaFile("notes.txt", "text/plain"); + HttpServletResponse response = mock(HttpServletResponse.class); + + AtomMediaResource resource = MediaCollection.createMediaResource(mediaFile, response); + + assertEquals("text/plain", resource.getContentType()); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response).setContentType("text/plain"); + } + + private MediaFile mediaFile(String name, String type) { + MediaFile mediaFile = mock(MediaFile.class); + when(mediaFile.getName()).thenReturn(name); + when(mediaFile.getContentType()).thenReturn(type); + when(mediaFile.getLength()).thenReturn(3L); + when(mediaFile.getLastModified()).thenReturn(7L); + when(mediaFile.getInputStream()) + .thenReturn(new ByteArrayInputStream(new byte[]{1, 2, 3})); + return mediaFile; + } +}