From 6d95b74390cca7608311e2865670b7d161da8451 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 29 Aug 2026 16:40:56 -0400 Subject: [PATCH] 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"); + } + } +}