From 83baa8618336f995b46a812d888dda2790e92dcb Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Tue, 29 Sep 2026 17:03:42 -0400 Subject: [PATCH 1/5] Serve AtomPub through a Roller servlet Map /roller-services/app/* to a small RollerAtomServlet that extends Propono's AtomServlet: - Answer 404 while webservices.enableAtomPub is off, matching the XML-RPC endpoint. Previously only the service document checked the setting. - Read Atom entry bodies (POST of Atom content, PUT to an entry URI) with Roller's standard XML parser configuration, cap them at 10 MB, and hand Propono a buffered copy. Unparseable entries get 400; oversized ones 413. RollerAtomHandler's entry-URI test moves into a static helper so the servlet and handler use the same rule. --- .../atomprotocol/RollerAtomHandler.java | 13 +- .../atomprotocol/RollerAtomServlet.java | 193 ++++++++++++++ app/src/main/webapp/WEB-INF/web.xml | 2 +- .../atomprotocol/RollerAtomServletTest.java | 240 ++++++++++++++++++ 4 files changed, 446 insertions(+), 2 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java 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 d1bdbb202..52c8336c8 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 @@ -327,7 +327,18 @@ public boolean isAtomServiceURI(AtomRequest areq) { */ @Override public boolean isEntryURI(AtomRequest areq) { - String[] pathInfo = StringUtils.split(areq.getPathInfo(),"/"); + return isEntryPath(areq.getPathInfo()); + } + + /** + * True if the path info names an entry. Shared with RollerAtomServlet so + * both agree on which requests carry an entry body. + */ + static boolean isEntryPath(String path) { + String[] pathInfo = StringUtils.split(path, "/"); + if (pathInfo == null) { + return false; + } if (pathInfo.length > 2 && pathInfo[1].equals("entry")) { return true; } diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java new file mode 100644 index 000000000..b01cb7603 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java @@ -0,0 +1,193 @@ +/* + * 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.webservices.atomprotocol; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import javax.servlet.ReadListener; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; + +import com.rometools.propono.atom.server.AtomServlet; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.util.SafeSAXBuilder; +import org.jdom2.JDOMException; + +/** + * Roller's AtomPub endpoint. It answers only while + * webservices.enableAtomPub is on, and it reads each Atom entry + * body with Roller's shared XML parser settings before the Propono servlet + * handles the request. + */ +public class RollerAtomServlet extends AtomServlet { + + private static final long serialVersionUID = 1L; + + private static final Log LOG = LogFactory.getLog(RollerAtomServlet.class); + + /** Largest Atom entry body accepted, in bytes. Media uploads are not affected. */ + static final int MAX_ENTRY_BYTES = 10 * 1024 * 1024; + + private static final String ATOM_CONTENT_TYPE = "application/atom+xml"; + + @Override + protected void service(HttpServletRequest req, HttpServletResponse res) + throws ServletException, IOException { + + if (!WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) { + LOG.debug("AtomPub service is disabled; rejecting request"); + sendText(res, HttpServletResponse.SC_NOT_FOUND, "AtomPub service is disabled"); + return; + } + + if (!carriesEntry(req)) { + forward(req, res); + return; + } + + byte[] body = readBody(req.getInputStream()); + if (body == null) { + sendText(res, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, "Entry is too large"); + return; + } + try { + // Propono reads the entry as UTF-8 text, so check the same text. + new SafeSAXBuilder().build(new InputStreamReader( + new ByteArrayInputStream(body), StandardCharsets.UTF_8)); + } catch (JDOMException e) { + LOG.debug("Rejecting Atom entry that could not be parsed", e); + sendText(res, HttpServletResponse.SC_BAD_REQUEST, "Invalid Atom entry"); + return; + } + forward(new BufferedBodyRequest(req, body), res); + } + + /** Hands the request to the Propono servlet. */ + protected void forward(HttpServletRequest req, HttpServletResponse res) + throws ServletException, IOException { + super.service(req, res); + } + + /** + * True when Propono would parse the request body as an Atom entry: a POST + * of Atom content, or a PUT to an entry URI. + */ + static boolean carriesEntry(HttpServletRequest req) { + String method = req.getMethod(); + if ("POST".equalsIgnoreCase(method)) { + String contentType = req.getContentType(); + return contentType != null && contentType.startsWith(ATOM_CONTENT_TYPE); + } + if ("PUT".equalsIgnoreCase(method)) { + return RollerAtomHandler.isEntryPath(req.getPathInfo()); + } + return false; + } + + /** Reads the whole body, or returns null when it exceeds the limit. */ + private static byte[] readBody(InputStream in) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int total = 0; + int read; + while ((read = in.read(buffer)) != -1) { + total += read; + if (total > MAX_ENTRY_BYTES) { + return null; + } + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + + private static void sendText(HttpServletResponse res, int status, String message) + throws IOException { + res.setStatus(status); + res.setContentType("text/plain;charset=UTF-8"); + res.getWriter().write(message); + } + + /** A request whose body has already been read into memory. */ + static final class BufferedBodyRequest extends HttpServletRequestWrapper { + + private final byte[] body; + + BufferedBodyRequest(HttpServletRequest request, byte[] body) { + super(request); + this.body = body; + } + + @Override + public ServletInputStream getInputStream() { + final ByteArrayInputStream in = new ByteArrayInputStream(body); + return new ServletInputStream() { + @Override + public int read() { + return in.read(); + } + + @Override + public int read(byte[] b, int off, int len) { + return in.read(b, off, len); + } + + @Override + public boolean isFinished() { + return in.available() == 0; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener listener) { + throw new UnsupportedOperationException(); + } + }; + } + + @Override + public BufferedReader getReader() { + return new BufferedReader(new InputStreamReader( + new ByteArrayInputStream(body), StandardCharsets.UTF_8)); + } + + @Override + public int getContentLength() { + return body.length; + } + + @Override + public long getContentLengthLong() { + return body.length; + } + } +} diff --git a/app/src/main/webapp/WEB-INF/web.xml b/app/src/main/webapp/WEB-INF/web.xml index 8b1b550e3..70f239932 100644 --- a/app/src/main/webapp/WEB-INF/web.xml +++ b/app/src/main/webapp/WEB-INF/web.xml @@ -286,7 +286,7 @@ AtomServlet - com.rometools.propono.atom.server.AtomServlet + org.apache.roller.weblogger.webservices.atomprotocol.RollerAtomServlet diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java new file mode 100644 index 000000000..9fe5954a8 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java @@ -0,0 +1,240 @@ +/* + * 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.webservices.atomprotocol; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +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.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class RollerAtomServletTest { + + private static final String ENTRY = + "\n" + + "" + + "Hello" + + "urn:uuid:00000000-0000-0000-0000-000000000001" + + "2026-01-01T00:00:00Z" + + "Body" + + ""; + + private static final String ENTRY_WITH_DOCTYPE = + "\n" + + "\n" + + "Hello"; + + private MockedStatic config; + private RecordingServlet servlet; + private HttpServletResponse response; + private StringWriter responseBody; + + @BeforeEach + void setUp() throws IOException { + config = mockStatic(WebloggerRuntimeConfig.class); + config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) + .thenReturn(true); + servlet = new RecordingServlet(); + response = mock(HttpServletResponse.class); + responseBody = new StringWriter(); + when(response.getWriter()).thenReturn(new PrintWriter(responseBody)); + } + + @AfterEach + void tearDown() { + config.close(); + } + + @Test + void disabledServiceAnswersNotFoundWithoutReadingTheBody() throws Exception { + config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) + .thenReturn(false); + HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); + + servlet.service(request, response); + + verify(response).setStatus(HttpServletResponse.SC_NOT_FOUND); + verify(request, never()).getInputStream(); + assertNull(servlet.forwarded); + } + + @Test + void disabledServiceAlsoRefusesReads() throws Exception { + config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) + .thenReturn(false); + + servlet.service(request("GET", "/blog/entries", null, ""), response); + + verify(response).setStatus(HttpServletResponse.SC_NOT_FOUND); + assertNull(servlet.forwarded); + } + + @Test + void readsPassThroughUnchanged() throws Exception { + HttpServletRequest request = request("GET", "/blog/entries", null, ""); + + servlet.service(request, response); + + assertSame(request, servlet.forwarded); + } + + @Test + void wellFormedEntryIsForwardedWithTheSameBody() throws Exception { + servlet.service(request("POST", "/blog/entries", "application/atom+xml;type=entry", ENTRY), response); + + assertNotNull(servlet.forwarded); + assertArrayEquals(ENTRY.getBytes(StandardCharsets.UTF_8), servlet.forwardedBody); + } + + @Test + void postedEntryWithDoctypeIsRefused() throws Exception { + servlet.service(request("POST", "/blog/entries", "application/atom+xml", ENTRY_WITH_DOCTYPE), response); + + verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertNull(servlet.forwarded); + } + + @Test + void entryUpdateWithDoctypeIsRefusedWhateverItsContentType() throws Exception { + servlet.service(request("PUT", "/blog/entry/abc", "text/plain", ENTRY_WITH_DOCTYPE), response); + + verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertNull(servlet.forwarded); + } + + @Test + void mediaUploadIsForwardedWithoutParsing() throws Exception { + HttpServletRequest request = request("POST", "/blog/resources", "image/png", "not xml"); + + servlet.service(request, response); + + assertSame(request, servlet.forwarded); + verify(request, never()).getInputStream(); + } + + @Test + void mediaUpdateIsForwardedWithoutParsing() throws Exception { + HttpServletRequest request = request("PUT", "/blog/resource/photo.png", "image/png", "not xml"); + + servlet.service(request, response); + + assertSame(request, servlet.forwarded); + verify(request, never()).getInputStream(); + } + + @Test + void oversizedEntryIsRefused() throws Exception { + byte[] big = new byte[RollerAtomServlet.MAX_ENTRY_BYTES + 1]; + Arrays.fill(big, (byte) ' '); + HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ""); + when(request.getInputStream()).thenReturn(stream(big)); + + servlet.service(request, response); + + verify(response).setStatus(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + assertNull(servlet.forwarded); + } + + @Test + void entryPathsMatchTheHandler() { + assertTrue(RollerAtomHandler.isEntryPath("/blog/entry/abc")); + assertTrue(RollerAtomHandler.isEntryPath("/blog/resource/photo.png.media-link")); + assertFalse(RollerAtomHandler.isEntryPath("/blog/resource/photo.png")); + assertFalse(RollerAtomHandler.isEntryPath("/blog/entries")); + assertFalse(RollerAtomHandler.isEntryPath(null)); + } + + private static HttpServletRequest request(String method, String pathInfo, + String contentType, String body) throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getMethod()).thenReturn(method); + when(request.getPathInfo()).thenReturn(pathInfo); + when(request.getContentType()).thenReturn(contentType); + when(request.getInputStream()).thenReturn(stream(body.getBytes(StandardCharsets.UTF_8))); + return request; + } + + private static ServletInputStream stream(byte[] bytes) { + ByteArrayInputStream in = new ByteArrayInputStream(bytes); + return new ServletInputStream() { + @Override + public int read() { + return in.read(); + } + + @Override + public int read(byte[] b, int off, int len) { + return in.read(b, off, len); + } + + @Override + public boolean isFinished() { + return in.available() == 0; + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setReadListener(ReadListener listener) { + throw new UnsupportedOperationException(); + } + }; + } + + /** Records what would have been handed to the Propono servlet. */ + private static final class RecordingServlet extends RollerAtomServlet { + private static final long serialVersionUID = 1L; + HttpServletRequest forwarded; + byte[] forwardedBody; + + @Override + protected void forward(HttpServletRequest req, HttpServletResponse res) throws IOException { + forwarded = req; + if (req instanceof BufferedBodyRequest) { + forwardedBody = req.getInputStream().readAllBytes(); + } + } + } +} From 7487cc67aae13b24e5982ac6dc66c2245ee0d114 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 08:22:39 -0400 Subject: [PATCH 2/5] Authenticate AtomPub entry requests before reading the body RollerAtomServlet now creates the AtomPub handler first and answers 401 when no user is authenticated, before buffering or parsing the entry. The handler is passed to Propono through a request attribute, which RollerAtomHandlerFactory returns, so authentication runs once per request. Add a 6.1.7 CHANGES.md section for the AtomPub behaviour changes. --- CHANGES.md | 9 ++++ .../RollerAtomHandlerFactory.java | 10 +++- .../atomprotocol/RollerAtomServlet.java | 22 +++++++++ .../atomprotocol/RollerAtomServletTest.java | 46 +++++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2f4ad53f8..df5e3f2b1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,14 @@ # Apache Roller β€” Changes +## 6.1.7 + +### Behaviour changes worth reading before upgrading + +- **AtomPub honours `webservices.enableAtomPub` on every request.** While the + setting is off, every AtomPub URL answers 404, not only the service document. +- **AtomPub entry bodies are limited to 10 MB.** A larger entry is refused with + 413. Media uploads are not affected. + ## 6.1.6 Initial installation now requires a one-time, cryptographically secure setup token printed to the server log. Bootstrap access closes as soon as setup finishes β€” when the first administrator is created on a new site, or when the database upgrade completes on an existing one. diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandlerFactory.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandlerFactory.java index 4b3e1f361..29d5bb06b 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandlerFactory.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandlerFactory.java @@ -30,12 +30,18 @@ public class RollerAtomHandlerFactory extends AtomHandlerFactory { /** - * Create new AtomHandler. + * Return the handler that {@link RollerAtomServlet} already authenticated + * for this request, or create a new AtomHandler. */ @Override public AtomHandler newAtomHandler( HttpServletRequest req, HttpServletResponse res) { + Object handler = req.getAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE); + if (handler instanceof AtomHandler) { + req.removeAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE); + return (AtomHandler) handler; + } return new RollerAtomHandler(req, res); - } + } } diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java index b01cb7603..25bf629d9 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java @@ -32,6 +32,7 @@ import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; +import com.rometools.propono.atom.server.AtomHandler; import com.rometools.propono.atom.server.AtomServlet; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -56,6 +57,13 @@ public class RollerAtomServlet extends AtomServlet { private static final String ATOM_CONTENT_TYPE = "application/atom+xml"; + /** + * Request attribute that carries the handler authenticated by this servlet + * to {@link RollerAtomHandlerFactory}, so Propono does not authenticate the + * request a second time. + */ + static final String HANDLER_ATTRIBUTE = RollerAtomServlet.class.getName() + ".handler"; + @Override protected void service(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException { @@ -71,6 +79,15 @@ protected void service(HttpServletRequest req, HttpServletResponse res) return; } + // Authenticate before reading the body, as Propono does. + AtomHandler handler = createHandler(req, res); + if (handler.getAuthenticatedUsername() == null) { + res.setHeader("WWW-Authenticate", "BASIC realm=\"AtomPub\""); + res.sendError(HttpServletResponse.SC_UNAUTHORIZED); + return; + } + req.setAttribute(HANDLER_ATTRIBUTE, handler); + byte[] body = readBody(req.getInputStream()); if (body == null) { sendText(res, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, "Entry is too large"); @@ -88,6 +105,11 @@ protected void service(HttpServletRequest req, HttpServletResponse res) forward(new BufferedBodyRequest(req, body), res); } + /** Creates the handler that authenticates the request. */ + protected AtomHandler createHandler(HttpServletRequest req, HttpServletResponse res) { + return new RollerAtomHandler(req, res); + } + /** Hands the request to the Propono servlet. */ protected void forward(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException { diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java index 9fe5954a8..0ef75db63 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import com.rometools.propono.atom.server.AtomHandler; import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -174,6 +175,42 @@ void oversizedEntryIsRefused() throws Exception { assertNull(servlet.forwarded); } + @Test + void unauthenticatedEntryPostIsRefusedWithoutReadingTheBody() throws Exception { + servlet.userName = null; + HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); + + servlet.service(request, response); + + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED); + verify(request, never()).getInputStream(); + assertNull(servlet.forwarded); + } + + @Test + void unauthenticatedEntryUpdateIsRefusedWithoutReadingTheBody() throws Exception { + servlet.userName = null; + HttpServletRequest request = request("PUT", "/blog/entry/abc", "application/atom+xml", ENTRY); + + servlet.service(request, response); + + verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED); + verify(request, never()).getInputStream(); + assertNull(servlet.forwarded); + } + + @Test + void authenticatedHandlerIsReusedByTheFactory() throws Exception { + HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); + + servlet.service(request, response); + + verify(request).setAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE, servlet.handler); + when(request.getAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE)).thenReturn(servlet.handler); + assertSame(servlet.handler, new RollerAtomHandlerFactory().newAtomHandler(request, response)); + verify(request).removeAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE); + } + @Test void entryPathsMatchTheHandler() { assertTrue(RollerAtomHandler.isEntryPath("/blog/entry/abc")); @@ -228,6 +265,15 @@ private static final class RecordingServlet extends RollerAtomServlet { private static final long serialVersionUID = 1L; HttpServletRequest forwarded; byte[] forwardedBody; + String userName = "alice"; + AtomHandler handler; + + @Override + protected AtomHandler createHandler(HttpServletRequest req, HttpServletResponse res) { + handler = mock(AtomHandler.class); + when(handler.getAuthenticatedUsername()).thenReturn(userName); + return handler; + } @Override protected void forward(HttpServletRequest req, HttpServletResponse res) throws IOException { From 59d886db8384c5bb1198fbb6925cd45c7319279d Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 08:26:25 -0400 Subject: [PATCH 3/5] Potential fix for pull request finding 'CodeQL / Resolving XML external entity in user-controlled data' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../webservices/atomprotocol/RollerAtomServlet.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java index 25bf629d9..49ccfe369 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java @@ -95,7 +95,11 @@ protected void service(HttpServletRequest req, HttpServletResponse res) } try { // Propono reads the entry as UTF-8 text, so check the same text. - new SafeSAXBuilder().build(new InputStreamReader( + SafeSAXBuilder saxBuilder = new SafeSAXBuilder(); + saxBuilder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + saxBuilder.setFeature("http://xml.org/sax/features/external-general-entities", false); + saxBuilder.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + saxBuilder.build(new InputStreamReader( new ByteArrayInputStream(body), StandardCharsets.UTF_8)); } catch (JDOMException e) { LOG.debug("Rejecting Atom entry that could not be parsed", e); From bfbe58c8991082bb2a0571cddd7b05e7ddefdd90 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 08:29:26 -0400 Subject: [PATCH 4/5] Trim redundant RollerAtomServletTest cases Fold the disabled-service read check into the write check, and drop the media PUT, unauthenticated PUT and entry-path cases, which repeat paths the remaining tests already cover. --- .../atomprotocol/RollerAtomServletTest.java | 47 ++----------------- 1 file changed, 5 insertions(+), 42 deletions(-) diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java index 0ef75db63..92b520b06 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java @@ -37,11 +37,9 @@ import org.mockito.MockedStatic; import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; 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.never; @@ -86,7 +84,7 @@ void tearDown() { } @Test - void disabledServiceAnswersNotFoundWithoutReadingTheBody() throws Exception { + void disabledServiceAnswersNotFoundForWritesAndReads() throws Exception { config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) .thenReturn(false); HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); @@ -96,16 +94,12 @@ void disabledServiceAnswersNotFoundWithoutReadingTheBody() throws Exception { verify(response).setStatus(HttpServletResponse.SC_NOT_FOUND); verify(request, never()).getInputStream(); assertNull(servlet.forwarded); - } - @Test - void disabledServiceAlsoRefusesReads() throws Exception { - config.when(() -> WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) - .thenReturn(false); + HttpServletResponse readResponse = mock(HttpServletResponse.class); + when(readResponse.getWriter()).thenReturn(new PrintWriter(new StringWriter())); + servlet.service(request("GET", "/blog/entries", null, ""), readResponse); - servlet.service(request("GET", "/blog/entries", null, ""), response); - - verify(response).setStatus(HttpServletResponse.SC_NOT_FOUND); + verify(readResponse).setStatus(HttpServletResponse.SC_NOT_FOUND); assertNull(servlet.forwarded); } @@ -152,16 +146,6 @@ void mediaUploadIsForwardedWithoutParsing() throws Exception { verify(request, never()).getInputStream(); } - @Test - void mediaUpdateIsForwardedWithoutParsing() throws Exception { - HttpServletRequest request = request("PUT", "/blog/resource/photo.png", "image/png", "not xml"); - - servlet.service(request, response); - - assertSame(request, servlet.forwarded); - verify(request, never()).getInputStream(); - } - @Test void oversizedEntryIsRefused() throws Exception { byte[] big = new byte[RollerAtomServlet.MAX_ENTRY_BYTES + 1]; @@ -187,18 +171,6 @@ void unauthenticatedEntryPostIsRefusedWithoutReadingTheBody() throws Exception { assertNull(servlet.forwarded); } - @Test - void unauthenticatedEntryUpdateIsRefusedWithoutReadingTheBody() throws Exception { - servlet.userName = null; - HttpServletRequest request = request("PUT", "/blog/entry/abc", "application/atom+xml", ENTRY); - - servlet.service(request, response); - - verify(response).sendError(HttpServletResponse.SC_UNAUTHORIZED); - verify(request, never()).getInputStream(); - assertNull(servlet.forwarded); - } - @Test void authenticatedHandlerIsReusedByTheFactory() throws Exception { HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); @@ -211,15 +183,6 @@ void authenticatedHandlerIsReusedByTheFactory() throws Exception { verify(request).removeAttribute(RollerAtomServlet.HANDLER_ATTRIBUTE); } - @Test - void entryPathsMatchTheHandler() { - assertTrue(RollerAtomHandler.isEntryPath("/blog/entry/abc")); - assertTrue(RollerAtomHandler.isEntryPath("/blog/resource/photo.png.media-link")); - assertFalse(RollerAtomHandler.isEntryPath("/blog/resource/photo.png")); - assertFalse(RollerAtomHandler.isEntryPath("/blog/entries")); - assertFalse(RollerAtomHandler.isEntryPath(null)); - } - private static HttpServletRequest request(String method, String pathInfo, String contentType, String body) throws IOException { HttpServletRequest request = mock(HttpServletRequest.class); From e2a72689b8a878bc9adb3135460dcf7c38e152c3 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Wed, 7 Oct 2026 16:52:34 -0400 Subject: [PATCH 5/5] Refine AtomPub entry handling and size settings --- CHANGES.md | 6 +- .../ui/struts2/admin/GlobalConfig.java | 7 +- .../atomprotocol/RollerAtomServlet.java | 72 +++++---- .../resources/ApplicationResources.properties | 2 + .../weblogger/config/runtimeConfigDefs.xml | 5 + .../admin/GlobalConfigAtomPubLimitTest.java | 113 ++++++++++++++ .../atomprotocol/RollerAtomServletTest.java | 140 +++++++++++++++++- docs/roller-user-guide.adoc | 9 ++ 8 files changed, 319 insertions(+), 35 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfigAtomPubLimitTest.java diff --git a/CHANGES.md b/CHANGES.md index 0ea1d8b9d..6ea77b34e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,8 +23,10 @@ - **AtomPub honours `webservices.enableAtomPub` on every request.** While the setting is off, every AtomPub URL answers 404, not only the service document. -- **AtomPub entry bodies are limited to 10 MB.** A larger entry is refused with - 413. Media uploads are not affected. +- **AtomPub entry bodies default to a 1 MiB limit.** A larger entry is refused + with 413. Set `webservices.atomPubMaxEntrySize` in Server Settings to change + the limit in bytes (default 1048576). Media uploads use the existing file + upload limits. - **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/ui/struts2/admin/GlobalConfig.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java index 4cd235084..c93e1a39e 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java @@ -195,7 +195,12 @@ public String save() { } else if ( incomingProp != null && propertyDef.getType().equals("integer") ) { try { - Integer.parseInt(incomingProp); + int value = Integer.parseInt(incomingProp); + if ("webservices.atomPubMaxEntrySize".equals(propName) + && (value <= 0 || value == Integer.MAX_VALUE)) { + addError("ConfigForm.invalidAtomPubMaxEntrySize"); + continue; + } updProp.setValue(incomingProp); log.debug("Set integer " + propName + " = " + incomingProp); } catch ( NumberFormatException nfe ) { diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java index 49ccfe369..2aaccec47 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java @@ -20,11 +20,10 @@ import java.io.BufferedReader; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import javax.xml.parsers.ParserConfigurationException; import javax.servlet.ReadListener; import javax.servlet.ServletException; import javax.servlet.ServletInputStream; @@ -37,8 +36,12 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; -import org.apache.roller.weblogger.util.SafeSAXBuilder; -import org.jdom2.JDOMException; +import org.apache.roller.weblogger.util.SecureXmlParsers; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.DefaultHandler; /** * Roller's AtomPub endpoint. It answers only while @@ -52,8 +55,10 @@ public class RollerAtomServlet extends AtomServlet { private static final Log LOG = LogFactory.getLog(RollerAtomServlet.class); - /** Largest Atom entry body accepted, in bytes. Media uploads are not affected. */ - static final int MAX_ENTRY_BYTES = 10 * 1024 * 1024; + /** Default maximum Atom entry body size, in bytes. Media uploads are not affected. */ + static final int DEFAULT_MAX_ENTRY_BYTES = 1024 * 1024; + + static final String MAX_ENTRY_SIZE_PROPERTY = "webservices.atomPubMaxEntrySize"; private static final String ATOM_CONTENT_TYPE = "application/atom+xml"; @@ -88,20 +93,32 @@ protected void service(HttpServletRequest req, HttpServletResponse res) } req.setAttribute(HANDLER_ATTRIBUTE, handler); - byte[] body = readBody(req.getInputStream()); - if (body == null) { + int maxEntryBytes = maxEntryBytes(); + // Read one byte past the limit, so an oversized body can be detected. + byte[] body = req.getInputStream().readNBytes(maxEntryBytes + 1); + if (body.length > maxEntryBytes) { sendText(res, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, "Entry is too large"); return; } + DefaultHandler contentHandler = new DefaultHandler() { + @Override + public void error(SAXParseException e) throws SAXException { + throw e; + } + }; + XMLReader reader; + try { + reader = SecureXmlParsers.newSAXParserFactory().newSAXParser().getXMLReader(); + } catch (ParserConfigurationException | SAXException e) { + throw new ServletException("Could not create an Atom entry parser", e); + } + reader.setContentHandler(contentHandler); + reader.setErrorHandler(contentHandler); try { // Propono reads the entry as UTF-8 text, so check the same text. - SafeSAXBuilder saxBuilder = new SafeSAXBuilder(); - saxBuilder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - saxBuilder.setFeature("http://xml.org/sax/features/external-general-entities", false); - saxBuilder.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - saxBuilder.build(new InputStreamReader( - new ByteArrayInputStream(body), StandardCharsets.UTF_8)); - } catch (JDOMException e) { + reader.parse(new InputSource(new InputStreamReader( + new ByteArrayInputStream(body), StandardCharsets.UTF_8))); + } catch (SAXException e) { LOG.debug("Rejecting Atom entry that could not be parsed", e); sendText(res, HttpServletResponse.SC_BAD_REQUEST, "Invalid Atom entry"); return; @@ -136,20 +153,21 @@ static boolean carriesEntry(HttpServletRequest req) { return false; } - /** Reads the whole body, or returns null when it exceeds the limit. */ - private static byte[] readBody(InputStream in) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buffer = new byte[8192]; - int total = 0; - int read; - while ((read = in.read(buffer)) != -1) { - total += read; - if (total > MAX_ENTRY_BYTES) { - return null; + /** Uses the default when an older installation has no setting or its value is invalid. */ + private static int maxEntryBytes() { + String value = WebloggerRuntimeConfig.getProperty(MAX_ENTRY_SIZE_PROPERTY); + if (value != null) { + try { + int limit = Integer.parseInt(value.trim()); + if (limit > 0 && limit < Integer.MAX_VALUE) { + return limit; + } + } catch (NumberFormatException e) { + // Fall back to the default below. } - out.write(buffer, 0, read); + LOG.warn("Invalid " + MAX_ENTRY_SIZE_PROPERTY + "; using the default entry limit"); } - return out.toByteArray(); + return DEFAULT_MAX_ENTRY_BYTES; } private static void sendText(HttpServletResponse res, int status, String message) diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 5d6117f7b..8128a7b3c 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -339,6 +339,7 @@ configForm.editorPages=Editor Pages configForm.webServicesSettings=Web Services Settings configForm.enableAtomPub=Enable Atom Publishing Protocol configForm.AtomPubAuth=AtomPub authentication (basic or oauth) +configForm.atomPubMaxEntrySize=Maximum AtomPub entry size (bytes) configForm.enableXmlRpc=Enable Blogger / MetaWeblog API configForm.weblogSettings=Weblog Rendering Settings @@ -1135,6 +1136,7 @@ ConfigForm.error.saveFailed=Error saving Planet configuration ConfigForm.invalidBooleanProperty=Property {0} must be a boolean: {1} ConfigForm.invalidIntegerProperty=Property {0} must be an integer: {1} +ConfigForm.invalidAtomPubMaxEntrySize=Maximum AtomPub entry size must be between 1 and 2147483646 bytes. ConfigForm.invalidFloatProperty=Property {0} must be a float: {1} ConfigForm.invalidProperty=Property {0} is null diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/runtimeConfigDefs.xml b/app/src/main/resources/org/apache/roller/weblogger/config/runtimeConfigDefs.xml index 12091fe1c..5ac86d35a 100644 --- a/app/src/main/resources/org/apache/roller/weblogger/config/runtimeConfigDefs.xml +++ b/app/src/main/resources/org/apache/roller/weblogger/config/runtimeConfigDefs.xml @@ -130,6 +130,11 @@ basic + + integer + 1048576 + + diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfigAtomPubLimitTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfigAtomPubLimitTest.java new file mode 100644 index 000000000..cbf6de23d --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfigAtomPubLimitTest.java @@ -0,0 +1,113 @@ +/* + * 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 + * + * 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. + */ +package org.apache.roller.weblogger.ui.struts2.admin; + +import java.util.HashMap; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; + +import org.apache.roller.weblogger.business.PropertiesManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.Parameter; +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.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class GlobalConfigAtomPubLimitTest { + + private static final String LIMIT_PROPERTY = "webservices.atomPubMaxEntrySize"; + + @Test + void invalidLimitsAreNotSaved() throws Exception { + PropertiesManager properties = mock(PropertiesManager.class); + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getPropertiesManager()).thenReturn(properties); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + for (String value : new String[] {"0", "-1", "2147483647", "2147483648", "not a number", ""}) { + GlobalConfig action = action(value); + assertEquals(GlobalConfig.ERROR, action.save(), value); + assertTrue(action.hasActionErrors(), value); + assertEquals("1048576", action.getProperties().get(LIMIT_PROPERTY).getValue()); + } + verify(properties, never()).saveProperties(any()); + } + } + + @Test + void validLimitsAreSaved() throws Exception { + PropertiesManager properties = mock(PropertiesManager.class); + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getPropertiesManager()).thenReturn(properties); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + for (String value : new String[] {"1", "1048576", "2097152", "2147483646"}) { + clearInvocations(properties); + GlobalConfig action = action(value); + assertEquals(GlobalConfig.SUCCESS, action.save(), value); + assertFalse(action.hasActionErrors(), value); + assertEquals(value, action.getProperties().get(LIMIT_PROPERTY).getValue()); + verify(properties).saveProperties(action.getProperties()); + } + } + } + + @Test + void serverSettingsDefineAOneMiBDefault() { + assertEquals("1048576", WebloggerRuntimeConfig.getRuntimeConfigDefs() + .getConfigDefs().get(0).getPropertyDef(LIMIT_PROPERTY).getDefaultValue()); + } + + private GlobalConfig action(String value) { + GlobalConfig action = spy(new GlobalConfig()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString(), anyList()); + action.setGlobalConfigDef(WebloggerRuntimeConfig.getRuntimeConfigDefs().getConfigDefs().get(0)); + Map values = new HashMap<>(); + values.put(LIMIT_PROPERTY, new RuntimeConfigProperty(LIMIT_PROPERTY, "1048576")); + values.put("users.comments.plugins", new RuntimeConfigProperty("users.comments.plugins", "")); + action.setProperties(values); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getMethod()).thenReturn("POST"); + action.setServletRequest(request); + Parameter incomingLimit = mock(Parameter.class); + when(incomingLimit.getValue()).thenReturn(value); + HttpParameters parameters = mock(HttpParameters.class); + when(parameters.get(LIMIT_PROPERTY)).thenReturn(incomingLimit); + when(parameters.get("users.comments.plugins")).thenReturn(mock(Parameter.class)); + action.setParameters(parameters); + return action; + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java index 92b520b06..106fdcfe4 100644 --- a/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServletTest.java @@ -24,23 +24,30 @@ import java.io.StringWriter; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParserFactory; import javax.servlet.ReadListener; +import javax.servlet.ServletException; import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import com.rometools.propono.atom.server.AtomHandler; import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.util.SecureXmlParsers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -137,8 +144,15 @@ void entryUpdateWithDoctypeIsRefusedWhateverItsContentType() throws Exception { } @Test - void mediaUploadIsForwardedWithoutParsing() throws Exception { - HttpServletRequest request = request("POST", "/blog/resources", "image/png", "not xml"); + void mediaUploadsBypassTheEntryLimit() throws Exception { + mediaUploadIsForwardedWithoutApplyingTheEntryLimit("POST", "/blog/resources"); + mediaUploadIsForwardedWithoutApplyingTheEntryLimit("PUT", "/blog/resources/image.png"); + } + + private void mediaUploadIsForwardedWithoutApplyingTheEntryLimit(String method, String path) throws Exception { + config.when(() -> WebloggerRuntimeConfig.getProperty(RollerAtomServlet.MAX_ENTRY_SIZE_PROPERTY)) + .thenReturn("1"); + HttpServletRequest request = request(method, path, "image/png", "not xml"); servlet.service(request, response); @@ -148,9 +162,74 @@ void mediaUploadIsForwardedWithoutParsing() throws Exception { @Test void oversizedEntryIsRefused() throws Exception { - byte[] big = new byte[RollerAtomServlet.MAX_ENTRY_BYTES + 1]; + byte[] big = new byte[RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES + 100]; Arrays.fill(big, (byte) ' '); HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ""); + ServletInputStream input = stream(big); + when(request.getInputStream()).thenReturn(input); + + servlet.service(request, response); + + verify(response).setStatus(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + assertNull(servlet.forwarded); + assertEquals(99, input.available(), "Read only one byte past the entry limit"); + } + + @Test + void entryExactlyAtTheDefaultLimitIsAccepted() throws Exception { + String body = ENTRY + " ".repeat( + RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES - ENTRY.getBytes(StandardCharsets.UTF_8).length); + + servlet.service(request("POST", "/blog/entries", "application/atom+xml", body), response); + + assertNotNull(servlet.forwarded); + assertArrayEquals(body.getBytes(StandardCharsets.UTF_8), servlet.forwardedBody); + } + + @Test + void configuredLimitAppliesToEntryPostsAndUpdates() throws Exception { + configuredLimitCountsUtf8BytesAndChangesOnTheNextRequest("POST", "/blog/entries"); + servlet.forwarded = null; + clearInvocations(response); + configuredLimitCountsUtf8BytesAndChangesOnTheNextRequest("PUT", "/blog/entry/abc"); + } + + private void configuredLimitCountsUtf8BytesAndChangesOnTheNextRequest(String method, String path) throws Exception { + String body = ENTRY.replace("Hello", "Hello δΈ–η•Œ"); + int size = body.getBytes(StandardCharsets.UTF_8).length; + config.when(() -> WebloggerRuntimeConfig.getProperty(RollerAtomServlet.MAX_ENTRY_SIZE_PROPERTY)) + .thenReturn(Integer.toString(size - 1), Integer.toString(size)); + + servlet.service(request(method, path, "application/atom+xml", body), response); + + verify(response).setStatus(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE); + assertNull(servlet.forwarded); + + servlet.service(request(method, path, "application/atom+xml", body), response); + + assertNotNull(servlet.forwarded); + assertArrayEquals(body.getBytes(StandardCharsets.UTF_8), servlet.forwardedBody); + } + + @Test + void missingOrInvalidLimitsUseTheDefault() throws Exception { + for (String value : new String[] {null, "", "0", "-1", "2147483647", "2147483648", "not a number"}) { + clearInvocations(response); + missingOrInvalidLimitUsesTheDefault(value); + } + } + + private void missingOrInvalidLimitUsesTheDefault(String value) throws Exception { + config.when(() -> WebloggerRuntimeConfig.getProperty(RollerAtomServlet.MAX_ENTRY_SIZE_PROPERTY)) + .thenReturn(value); + String body = ENTRY + " ".repeat( + RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES - ENTRY.getBytes(StandardCharsets.UTF_8).length); + servlet.service(request("POST", "/blog/entries", "application/atom+xml", body), response); + assertNotNull(servlet.forwarded); + assertEquals(RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES, servlet.forwardedBody.length); + servlet.forwarded = null; + byte[] big = new byte[RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES + 1]; + HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ""); when(request.getInputStream()).thenReturn(stream(big)); servlet.service(request, response); @@ -160,9 +239,55 @@ void oversizedEntryIsRefused() throws Exception { } @Test - void unauthenticatedEntryPostIsRefusedWithoutReadingTheBody() throws Exception { + void configuredLimitCanBeRaisedAboveTheDefault() throws Exception { + String body = ENTRY + " ".repeat(RollerAtomServlet.DEFAULT_MAX_ENTRY_BYTES); + config.when(() -> WebloggerRuntimeConfig.getProperty(RollerAtomServlet.MAX_ENTRY_SIZE_PROPERTY)) + .thenReturn(Integer.toString(body.getBytes(StandardCharsets.UTF_8).length)); + + servlet.service(request("POST", "/blog/entries", "application/atom+xml", body), response); + + assertNotNull(servlet.forwarded); + } + + @Test + void malformedEntryPostsAndUpdatesAreRefused() throws Exception { + malformedEntryIsRefused("POST", "/blog/entries"); + clearInvocations(response); + malformedEntryIsRefused("PUT", "/blog/entry/abc"); + } + + private void malformedEntryIsRefused(String method, String path) throws Exception { + servlet.service(request(method, path, "application/atom+xml", "</entry>"), response); + + verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertNull(servlet.forwarded); + } + + @Test + void parserSetupFailureIsAServerError() throws Exception { + try (MockedStatic<SecureXmlParsers> parsers = mockStatic(SecureXmlParsers.class)) { + SAXParserFactory factory = mock(SAXParserFactory.class); + parsers.when(SecureXmlParsers::newSAXParserFactory).thenReturn(factory); + when(factory.newSAXParser()).thenThrow(new ParserConfigurationException("Cannot create parser")); + + assertThrows(ServletException.class, () -> servlet.service( + request("POST", "/blog/entries", "application/atom+xml", ENTRY), response)); + + verify(response, never()).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertNull(servlet.forwarded); + } + } + + @Test + void unauthenticatedEntryPostsAndUpdatesDoNotReadTheBody() throws Exception { + unauthenticatedEntryIsRefusedWithoutReadingTheBody("POST", "/blog/entries"); + clearInvocations(response); + unauthenticatedEntryIsRefusedWithoutReadingTheBody("PUT", "/blog/entry/abc"); + } + + private void unauthenticatedEntryIsRefusedWithoutReadingTheBody(String method, String path) throws Exception { servlet.userName = null; - HttpServletRequest request = request("POST", "/blog/entries", "application/atom+xml", ENTRY); + HttpServletRequest request = request(method, path, "application/atom+xml", ENTRY); servlet.service(request, response); @@ -206,6 +331,11 @@ public int read(byte[] b, int off, int len) { return in.read(b, off, len); } + @Override + public int available() { + return in.available(); + } + @Override public boolean isFinished() { return in.available() == 0; diff --git a/docs/roller-user-guide.adoc b/docs/roller-user-guide.adoc index 07af8b845..8a0a33f2d 100644 --- a/docs/roller-user-guide.adoc +++ b/docs/roller-user-guide.adoc @@ -1288,6 +1288,15 @@ when they load the feed in their browsers. image::user-guide-29-fileupload.png[] +* *Maximum AtomPub entry size (bytes)*: Maximum size of the XML body of one +AtomPub entry, set under Web Services Settings. The default is 1048576 bytes +(1 MiB). The `webservices.atomPubMaxEntrySize` setting takes effect on the next +request without a restart and must be between 1 and 2147483646 bytes. An entry +over the limit returns HTTP 413. A missing or invalid setting uses the default. +This limit does not apply to AtomPub media uploads, which use the file upload +settings below. Raising it does not increase the capacity of the database's +entry content columns. + * *Enable File Uploads*: Are users allowed to upload files? * *Allowed Extensions*: Comma-separated list of file extensions that users are allowed to upload.