Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.getMimeType() returns null for extensions not mapped in web.xml/the container, and applyResponseHeaders turns null into an octet-stream attachment. On master setContentType(null) left the type unset and the browser could still display the file; now any unmapped (or uppercase, on a case-sensitive container) extension downloads in the theme preview.

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is also where customized-theme resources land: WeblogCustomTheme.getResource() looks up the media file but never assigns it to resource, so it always returns null, and importTheme stores theme CSS/JS as media files with their original path. resourceType for css/bootstrap.css is text/css, which isn't inline-safe, so the stylesheet goes out as an octet-stream attachment with nosniff and the browser drops it. Fixing WeblogCustomTheme.getResource to return the media file would route these through the theme branch above; alternatively, treat text/css / text/javascript from the servlet-context mime table as theme-authored here as the description already promises.

// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On replace this derives the type from the record's (old) name rather than the uploaded replacement's name (this.uploadedFileName is right there). Replacing photo.jpg with photo.png without renaming stores image/jpeg for PNG bytes and serves them with nosniff; replacing clip.txt with a video keeps text/plain and forces a download. putMedia in MediaCollection has the same mismatch with mf.getName().

manager.updateMediaFile(getActionWeblog(), mediaFile,
new FileInputStream(this.uploadedFile));
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>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.
*
* <p>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<String> INLINE_TYPES = Collections.unmodifiableSet(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth listing what this drops compared to response.setContentType(mediaFile.getContentType()) on master. Existing image/svg+xml media (already stored, already embedded in entries with <img>) now downloads instead of rendering, since browsers never sniff SVG; and image/x-png (legacy IE uploads), image/jpg (some XML-RPC clients), image/avif and image/apng are all passive but absent here. If SVG stays out (defensible, it can carry script), the release note should say so explicitly, and I'd add the other image subtypes.

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<String> 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because active declared types collapse to application/octet-stream before canSave / checkFileType run, an admin's content-type rules in uploads.types.forbid (e.g. image/svg+xml,application/xhtml+xml) no longer match those uploads; the file is stored under the generic type instead of being refused. Run the forbid check against the declared type too, or document that forbid rules are now extension-based.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text/plain with nosniff can't execute anything, so uploaded .txt files (very common as attachments) could stay inline rather than becoming a save-as dialog for an application/octet-stream notes.txt.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utilities.getContentTypeFromFileName is backed by javax.activation's default map, which knows about 22 extensions (no pdf, svg, webp, mp4, mp3, zip, css, js). So "the name decides" only holds for those; for everything else the declared type is stored after all (a report.pdf declared application/zip is stored as zip and downloads). The servlets already use the servlet context's mime table (web.xml has a full one); the policy should consult the same table.

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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously mf.setContentType(null) made checkFileType reject an upload with no type member; storedTypeFor(name, null) now always returns a type, so the upload is accepted as application/octet-stream when the allow list is empty (the default). Probably fine, but it's a loosening the description should mention.

mf.setInputStream(new ByteArrayInputStream(bits));
mf.setLength(bits.length);
String fileLink = mf.getPermalink();
Expand Down
Loading
Loading