diff --git a/CHANGES.md b/CHANGES.md index 68a5944c8..0d1158e98 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -39,6 +39,12 @@ - **Tag names can no longer contain `"`, `&`, `'`, `<`, `>` or `` ` ``.** These characters are removed when a tag is saved, and from existing tag names when they are shown. +- **Custom-theme files in the media library follow the media rules.** A weblog + with a custom theme still serves its stylesheets and scripts from the media + library as before. Other files there, such as HTML or SVG, are served like + any other media file: as a download unless they are an image, audio, video, + PDF or plain text. The media file editor sets a file's original path only + when `mediafile.originalPathEdit.enabled=true`, as its form already shows. - **Planet is off by default.** `planet.aggregator.enabled` now defaults to `false`. While it is off, the Planet admin pages, `/planetrss` and the Planet background tasks (`RefreshRollerPlanetTask`, `SyncWebsitesTask`) do 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 1416e02f2..9fd2a95ca 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 @@ -170,6 +170,14 @@ public ThemeResource getResource(String path) { return resource; } + /** + * @return true when the resource is a file in the weblog's media library, + * rather than one that ships with a theme + */ + public static boolean isMediaFile(ThemeResource resource) { + return resource instanceof MediaFileThemeResource; + } + private static final class MediaFileThemeResource implements ThemeResource { private final MediaFile mediaFile; 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 3fea57ff2..b484e5c46 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 @@ -33,6 +33,7 @@ import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.business.themes.WeblogCustomTheme; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.Theme; import org.apache.roller.weblogger.pojos.ThemeResource; @@ -95,6 +96,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) long resourceLastMod = 0; InputStream resourceStream = null; + boolean fromThemeMedia = false; // first, see if we have a preview theme to operate from if (!StringUtils.isEmpty(resourceRequest.getThemeName())) { @@ -104,6 +106,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) if (resource != null) { resourceLastMod = resource.getLastModified(); resourceStream = resource.getInputStream(); + fromThemeMedia = WeblogCustomTheme.isMediaFile(resource); } } @@ -117,6 +120,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) if (resource != null) { resourceLastMod = resource.getLastModified(); resourceStream = resource.getInputStream(); + fromThemeMedia = WeblogCustomTheme.isMediaFile(resource); } } } catch (Exception ex) { @@ -170,6 +174,11 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) // same policy as any other media response. MediaTypePolicy.applyResponseHeaders(response, resourceType, resourceRequest.getResourcePath()); + } else if (fromThemeMedia) { + // Found by a custom theme, but held in the media library like any + // other upload. + MediaTypePolicy.applyThemeMediaHeaders(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. 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 4f793e703..e03a34216 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 @@ -34,6 +34,7 @@ import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.business.themes.WeblogCustomTheme; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.ThemeResource; import org.apache.roller.weblogger.pojos.Weblog; @@ -101,6 +102,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) long resourceLastMod = 0; InputStream resourceStream = null; + boolean fromThemeMedia = false; // first see if resource comes from weblog's shared theme try { @@ -111,6 +113,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) if (resource != null) { resourceLastMod = resource.getLastModified(); resourceStream = resource.getInputStream(); + fromThemeMedia = WeblogCustomTheme.isMediaFile(resource); } } } catch (Exception ex) { @@ -169,6 +172,11 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) // same policy as any other media response. MediaTypePolicy.applyResponseHeaders(response, resourceType, resourceRequest.getResourcePath()); + } else if (fromThemeMedia) { + // Found by a custom theme, but held in the media library like any + // other upload. + MediaTypePolicy.applyThemeMediaHeaders(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. diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBean.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBean.java index 0e8ff5351..7f06c0aed 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBean.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBean.java @@ -98,8 +98,9 @@ public void setId(String id) { } /** - * Copies the contents of this bean to a media file object - * + * Copies the contents of this bean to a media file object. The original + * path is not copied: it decides which resource URL serves the file, so + * only actions that are allowed to set it do so. */ public void copyTo(MediaFile dataHolder) throws WebloggerException { @@ -108,7 +109,6 @@ public void copyTo(MediaFile dataHolder) throws WebloggerException { dataHolder.setCopyrightText(this.copyrightText); dataHolder.setTagsAsString(this.tagsAsString); dataHolder.setSharedForGallery(this.isSharedForGallery); - dataHolder.setOriginalPath(this.originalPath); } /** 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 11a787aea..789ba2a6d 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 @@ -124,6 +124,9 @@ public String save() { try { MediaFile mediaFile = manager.getMediaFile(getActionWeblog(), getMediaFileId()); bean.copyTo(mediaFile); + if (getBooleanProp("mediafile.originalPathEdit.enabled")) { + mediaFile.setOriginalPath(bean.getOriginalPath()); + } if (uploadedFile != null) { mediaFile.setLength(this.uploadedFile.length()); 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 2c8a7ebee..214a2f9c0 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 @@ -61,6 +61,16 @@ private MediaTypePolicy() { /** Families served inline whatever the subtype. */ private static final String[] INLINE_PREFIXES = {"audio/", "video/"}; + /** + * Stylesheets and scripts. A custom theme keeps its copies of these in the + * media library, and its pages load them by type. A browser does not render + * either as a document. + */ + private static final Set THEME_ASSET_TYPES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList( + "text/css", "text/javascript", "application/javascript", + "application/x-javascript"))); + /** * Types a browser may execute, or that can carry something it will. These * are never adopted from a client's declaration. @@ -148,6 +158,22 @@ public static void applyResponseHeaders(HttpServletResponse response, "attachment; filename=\"" + headerSafe(fileName) + "\""); } + /** + * Sets the headers for a custom-theme resource held in the media library. + * Stylesheets and scripts keep their type so that the theme's pages can + * load them; anything else is governed like any other media response. + */ + public static void applyThemeMediaHeaders(HttpServletResponse response, + String contentType, String fileName) { + String type = normalizeType(contentType); + if (type != null && THEME_ASSET_TYPES.contains(type)) { + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setContentType(type); + return; + } + applyResponseHeaders(response, contentType, fileName); + } + /** @return the response type after applying the inline policy. */ public static String responseTypeFor(String contentType) { String type = normalizeType(contentType); 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 index 0c4851117..b9b2e16cc 100644 --- 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 @@ -18,8 +18,10 @@ import org.mockito.MockedStatic; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; @@ -55,6 +57,13 @@ void mediaFileBackedResourcesAreReturnedAsThemeResources() throws Exception { assertEquals(3L, resource.getLength()); assertEquals(7L, resource.getLastModified()); assertSame(content, resource.getInputStream()); + assertTrue(WeblogCustomTheme.isMediaFile(resource)); } } + + @Test + void resourcesFromOtherSourcesAreNotMediaFiles() { + assertFalse(WeblogCustomTheme.isMediaFile(mock(ThemeResource.class))); + assertFalse(WeblogCustomTheme.isMediaFile(null)); + } } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServletTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServletTest.java new file mode 100644 index 000000000..5c1030dff --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/rendering/servlets/ResourceServletTest.java @@ -0,0 +1,158 @@ +/* + * 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 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://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. + */ +package org.apache.roller.weblogger.ui.rendering.servlets; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; + +import javax.servlet.ServletConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +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.business.themes.WeblogCustomTheme; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.ui.rendering.util.WeblogResourceRequest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockConstruction; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * A weblog with a custom theme finds its resources in the media library, so a + * resource URL can reach any uploaded file whose original path matches. + */ +class ResourceServletTest { + + private MockedStatic factory; + private MediaFileManager media; + private Weblog weblog; + private ResourceServlet servlet; + + @BeforeEach + void setUp() throws Exception { + media = mock(MediaFileManager.class); + Weblogger roller = mock(Weblogger.class); + when(roller.getMediaFileManager()).thenReturn(media); + factory = mockStatic(WebloggerFactory.class); + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + + weblog = mock(Weblog.class); + when(weblog.getTheme()).thenReturn(new WeblogCustomTheme(weblog)); + + ServletContext context = mock(ServletContext.class); + when(context.getMimeType("page.html")).thenReturn("text/html"); + when(context.getMimeType("/page.html")).thenReturn("text/html"); + when(context.getMimeType("/css/site.css")).thenReturn("text/css"); + ServletConfig config = mock(ServletConfig.class); + when(config.getServletContext()).thenReturn(context); + + servlet = new ResourceServlet(); + servlet.init(config); + } + + @AfterEach + void tearDown() { + factory.close(); + } + + @Test + void uploadedDocumentFoundByCustomThemeIsADownload() throws Exception { + HttpServletResponse response = serve("/page.html", + "

page

"); + + verify(response).setContentType("application/octet-stream"); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"/page.html\""); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response, never()).setContentType("text/html"); + } + + @Test + void customThemeStylesheetKeepsItsType() throws Exception { + HttpServletResponse response = serve("/css/site.css", "body {}"); + + verify(response).setContentType("text/css"); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response, never()).setHeader(eq("Content-Disposition"), anyString()); + } + + private HttpServletResponse serve(String path, String body) throws Exception { + MediaFile file = mock(MediaFile.class); + when(file.getOriginalPath()).thenReturn(path); + when(file.getInputStream()).thenReturn( + new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))); + when(media.getMediaFileByOriginalPath(weblog, path)).thenReturn(file); + + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getDateHeader("If-Modified-Since")).thenReturn(-1L); + HttpServletResponse response = mock(HttpServletResponse.class); + ByteArrayOutputStream written = new ByteArrayOutputStream(); + when(response.getOutputStream()).thenReturn(new CapturingStream(written)); + + try (MockedConstruction parsed = mockConstruction( + WeblogResourceRequest.class, (mocked, ctx) -> { + when(mocked.getWeblog()).thenReturn(weblog); + when(mocked.getResourcePath()).thenReturn(path); + })) { + servlet.doGet(request, response); + } + + assertEquals(body, written.toString(StandardCharsets.UTF_8.name())); + return response; + } + + private static final class CapturingStream extends ServletOutputStream { + private final ByteArrayOutputStream target; + + CapturingStream(ByteArrayOutputStream target) { + this.target = target; + } + + @Override + public void write(int b) { + target.write(b); + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setWriteListener(WriteListener listener) { + } + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBeanTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBeanTest.java new file mode 100644 index 000000000..46ad1a335 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileBeanTest.java @@ -0,0 +1,47 @@ +/* + * 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 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://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. + */ +package org.apache.roller.weblogger.ui.struts2.editor; + +import org.apache.roller.weblogger.pojos.MediaFile; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class MediaFileBeanTest { + + /** + * The original path decides which resource URL serves a file, so a value + * posted with the upload form must not reach the stored file. + */ + @Test + void copyToLeavesTheOriginalPathAlone() throws Exception { + MediaFileBean bean = new MediaFileBean(); + bean.setName("page.html"); + bean.setOriginalPath("/page.html"); + + MediaFile added = new MediaFile(); + bean.copyTo(added); + assertEquals("page.html", added.getName()); + assertNull(added.getOriginalPath()); + + MediaFile imported = new MediaFile(); + imported.setOriginalPath("/css/site.css"); + bean.copyTo(imported); + assertEquals("/css/site.css", imported.getOriginalPath()); + } +} 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 2cd9a850d..e38919887 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 @@ -150,6 +150,44 @@ public void anythingElseIsSentAsAnAttachment() { "attachment; filename=\"page.html\""); } + /** + * A custom theme keeps its stylesheets and scripts in the media library, + * and its pages load them by type, so those two keep their type. + */ + @Test + public void themeMediaStylesheetsAndScriptsKeepTheirType() { + for (String type : new String[]{"text/css", "text/javascript", + "application/javascript"}) { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyThemeMediaHeaders(response, type, "/theme/file"); + verify(response).setContentType(type); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + verify(response, never()).setHeader(eq("Content-Disposition"), anyString()); + } + } + + /** + * Anything else a custom theme finds in the media library is an upload + * like any other, and a document type must not render on Roller's origin. + */ + @Test + public void themeMediaDocumentsAreServedAsDownloads() { + for (String type : new String[]{"text/html", "image/svg+xml", + "application/xhtml+xml", "text/xml"}) { + HttpServletResponse response = mock(HttpServletResponse.class); + MediaTypePolicy.applyThemeMediaHeaders(response, type, "/page.html"); + verify(response).setContentType(MediaTypePolicy.DEFAULT_TYPE); + verify(response).setHeader("Content-Disposition", + "attachment; filename=\"/page.html\""); + verify(response).setHeader("X-Content-Type-Options", "nosniff"); + } + + HttpServletResponse image = mock(HttpServletResponse.class); + MediaTypePolicy.applyThemeMediaHeaders(image, "image/png", "/logo.png"); + verify(image).setContentType("image/png"); + verify(image, never()).setHeader(eq("Content-Disposition"), anyString()); + } + /** The name is placed inside a header, so it cannot be allowed to leave it. */ @Test public void theAttachmentNameCannotBreakOutOfTheHeader() { @@ -219,9 +257,9 @@ public void theServingPathGoesThroughThePolicy() throws Exception { /** * 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. + * serve theme resources from the same method. Uploaded media takes the + * media policy whether the fallback or a custom theme found it; a resource + * that ships with a theme keeps its type. Every branch refuses sniffing. */ @Test public void theResourceServletsCoverTheirUploadedMediaBranch() throws Exception { @@ -232,6 +270,10 @@ public void theResourceServletsCoverTheirUploadedMediaBranch() throws Exception name + " must apply the media policy to uploaded media"); assertTrue(servlet.contains("fromUploadedMedia"), name + " must distinguish uploaded media from theme resources"); + assertTrue(servlet.contains("MediaTypePolicy.applyThemeMediaHeaders"), + name + " must apply the media policy to media a custom theme finds"); + assertTrue(servlet.contains("WeblogCustomTheme.isMediaFile(resource)"), + name + " must check every theme lookup for media files"); assertTrue(servlet.contains("X-Content-Type-Options"), name + " must refuse sniffing on the theme branch too"); }