From f4db64b875744ceb4ab1f9458e764dbd804bfae2 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Fri, 2 Oct 2026 17:55:01 -0400 Subject: [PATCH 1/4] Turn Planet off by default and require POST for its admin changes Planet is now off unless planet.aggregator.enabled=true is set. While it is off, the Planet admin actions are refused and /planetrss returns 404, rather than only the menu being hidden. Sites that use Planet need to set the property in roller-custom.properties when they upgrade. Add a @RequiresPost annotation and RequiresPostInterceptor to the Roller stack: a marked action method refuses any request that is not a POST. Unmarked methods are unchanged. The Planet save and delete methods are marked; their forms already submit by POST. UISecurityEnforced gains a default isFeatureEnabled(), checked first by UISecurityInterceptor, so an optional feature can switch off all of its actions in one place. --- .../weblogger/planet/ui/PlanetConfig.java | 2 + .../weblogger/planet/ui/PlanetGroupSubs.java | 4 + .../weblogger/planet/ui/PlanetGroups.java | 2 + .../weblogger/planet/ui/PlanetUIAction.java | 9 ++ .../rendering/servlets/PlanetFeedServlet.java | 6 + .../ui/struts2/util/RequiresPost.java | 36 +++++ .../struts2/util/RequiresPostInterceptor.java | 70 +++++++++ .../ui/struts2/util/UISecurityEnforced.java | 9 ++ .../struts2/util/UISecurityInterceptor.java | 8 + .../roller/weblogger/config/roller.properties | 8 +- app/src/main/resources/struts.xml | 3 + .../planet/ui/PlanetAvailabilityTest.java | 139 ++++++++++++++++++ .../util/RequiresPostInterceptorTest.java | 112 ++++++++++++++ 13 files changed, 405 insertions(+), 3 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java index 8d24508f5e..a950126ea4 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java @@ -18,6 +18,7 @@ package org.apache.roller.weblogger.planet.ui; +import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.RollerException; @@ -107,6 +108,7 @@ public String execute() { } + @RequiresPost public String save() { try { diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java index 7d6ed549e3..6c60bfa923 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java @@ -16,6 +16,7 @@ package org.apache.roller.weblogger.planet.ui; +import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -113,6 +114,7 @@ public String execute() { /** * Save group. */ + @RequiresPost public String saveGroup() { validateGroup(); @@ -170,6 +172,7 @@ private void validateGroup() { /** * Save subscription, add to current group */ + @RequiresPost public String saveSubscription() { valudateNewSub(); @@ -222,6 +225,7 @@ public String saveSubscription() { /** * Delete subscription, reset form */ + @RequiresPost public String deleteSubscription() { if (getSubUrl() != null) { diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java index cea9025766..f01c2840ff 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java @@ -16,6 +16,7 @@ package org.apache.roller.weblogger.planet.ui; +import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.planet.business.PlanetManager; @@ -66,6 +67,7 @@ public String execute() { /** * Delete group */ + @RequiresPost public String delete() { if (getGroup() != null) { diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java index 2fe3232b7b..db34aa4ecc 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java @@ -21,6 +21,7 @@ import org.apache.roller.planet.business.PlanetManager; import org.apache.roller.planet.pojos.Planet; import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.ui.struts2.util.UIAction; @@ -37,6 +38,14 @@ public abstract class PlanetUIAction extends UIAction { private Planet planet = null; + /** + * Planet actions are only available while the Planet aggregator is enabled. + */ + @Override + public boolean isFeatureEnabled() { + return WebloggerConfig.getBooleanProperty("planet.aggregator.enabled"); + } + public Planet getPlanet() { if(planet == null) { try { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java index 432a0c33a1..af0558e5f5 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/PlanetFeedServlet.java @@ -32,6 +32,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.util.RollerConstants; +import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.apache.roller.planet.business.PlanetManager; import org.apache.roller.planet.config.PlanetRuntimeConfig; @@ -79,6 +80,11 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) log.debug("Entering"); + if (!WebloggerConfig.getBooleanProperty("planet.aggregator.enabled")) { + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + PlanetManager planet = WebloggerFactory.getWeblogger() .getPlanetManager(); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java new file mode 100644 index 0000000000..e39e72bd8e --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java @@ -0,0 +1,36 @@ +/* + * 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. 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.ui.struts2.util; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a Struts action method that changes state. {@link RequiresPostInterceptor} + * refuses any request to it that is not a POST. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface RequiresPost { +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java new file mode 100644 index 0000000000..77480c14cd --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java @@ -0,0 +1,70 @@ +/* + * 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. 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.ui.struts2.util; + +import static org.apache.struts2.StrutsStatics.HTTP_REQUEST; + +import java.lang.reflect.Method; +import javax.servlet.http.HttpServletRequest; + +import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.interceptor.AbstractInterceptor; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +/** + * Refuses requests that are not POSTs to action methods marked + * {@link RequiresPost}. Unmarked methods are not affected. + */ +public class RequiresPostInterceptor extends AbstractInterceptor { + + private static final long serialVersionUID = 1L; + private static final Log log = LogFactory.getLog(RequiresPostInterceptor.class); + + @Override + public String intercept(ActionInvocation invocation) throws Exception { + if (requiresPost(invocation)) { + HttpServletRequest request = (HttpServletRequest) + invocation.getInvocationContext().get(HTTP_REQUEST); + if (request == null || !"POST".equalsIgnoreCase(request.getMethod())) { + if (log.isDebugEnabled()) { + log.debug("Refusing " + (request == null ? "unknown" : request.getMethod()) + + " request to " + invocation.getProxy().getActionName() + + "!" + invocation.getProxy().getMethod()); + } + return UIAction.DENIED; + } + } + return invocation.invoke(); + } + + static boolean requiresPost(ActionInvocation invocation) { + String methodName = invocation.getProxy().getMethod(); + if (methodName == null) { + methodName = "execute"; + } + try { + Method method = invocation.getAction().getClass().getMethod(methodName); + return method.isAnnotationPresent(RequiresPost.class); + } catch (NoSuchMethodException e) { + return false; + } + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityEnforced.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityEnforced.java index f40f45b5ec..b51fdf5694 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityEnforced.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityEnforced.java @@ -62,4 +62,13 @@ public interface UISecurityEnforced { * List of weblog permissions required to access action if applicable. */ List requiredGlobalPermissionActions(); + + /** + * Whether the feature this action belongs to is turned on. Actions for + * optional features override this; while it returns false the action is + * refused. + */ + default boolean isFeatureEnabled() { + return true; + } } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityInterceptor.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityInterceptor.java index 8e97899b82..2e2ce00a70 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityInterceptor.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UISecurityInterceptor.java @@ -56,6 +56,14 @@ public String doIntercept(ActionInvocation invocation) throws Exception { final UISecurityEnforced theAction = (UISecurityEnforced) action; + // is the feature this action belongs to turned on? + if (!theAction.isFeatureEnabled()) { + if (log.isDebugEnabled()) { + log.debug("DENIED: feature is disabled"); + } + return UIAction.DENIED; + } + // are we requiring an authenticated user? if (theAction.isUserRequired()) { diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties index cdb7d5252a..6a2baebc3e 100644 --- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties +++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties @@ -198,9 +198,11 @@ site.bannedwordslist.enable.referrers=false #---------------------------------- # Planet Aggregator settings -# Set to true to enable the Planet aggregator. You also need to enable the -# RefreshRollerPlanetTask task below to get the feed fetcher running. -planet.aggregator.enabled=true +# Set to true to enable the Planet aggregator. It is off by default. While it +# is off, the Planet admin pages and the /planetrss feed are unavailable. You +# also need to enable the RefreshRollerPlanetTask task below to get the feed +# fetcher running. +planet.aggregator.enabled=false # Planet backend guice module, customized for use with Weblogger planet.aggregator.guice.module=\ diff --git a/app/src/main/resources/struts.xml b/app/src/main/resources/struts.xml index ea14c602a4..13021933f7 100644 --- a/app/src/main/resources/struts.xml +++ b/app/src/main/resources/struts.xml @@ -39,6 +39,8 @@ class="org.apache.roller.weblogger.ui.struts2.util.UIActionPrepareInterceptor" /> + diff --git a/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java new file mode 100644 index 0000000000..33f0123d10 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java @@ -0,0 +1,139 @@ +/* + * 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. 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.planet.ui; + +import java.io.InputStream; +import java.util.Properties; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.parsers.DocumentBuilderFactory; + +import com.opensymphony.xwork2.ActionInvocation; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.WebloggerConfig; +import org.apache.roller.weblogger.ui.rendering.servlets.PlanetFeedServlet; +import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; +import org.apache.roller.weblogger.ui.struts2.util.UIAction; +import org.apache.roller.weblogger.ui.struts2.util.UISecurityInterceptor; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; + +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.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 PlanetAvailabilityTest { + + private static final String ENABLED = "planet.aggregator.enabled"; + + @Test + void planetIsOffByDefault() throws Exception { + Properties defaults = new Properties(); + try (InputStream in = WebloggerConfig.class.getResourceAsStream( + "/org/apache/roller/weblogger/config/roller.properties")) { + defaults.load(in); + } + assertEquals("false", defaults.getProperty(ENABLED)); + } + + @Test + void planetActionsFollowTheSetting() { + PlanetUIAction action = new PlanetGroups(); + try (MockedStatic config = mockStatic(WebloggerConfig.class)) { + config.when(() -> WebloggerConfig.getBooleanProperty(ENABLED)).thenReturn(false); + assertFalse(action.isFeatureEnabled()); + + config.when(() -> WebloggerConfig.getBooleanProperty(ENABLED)).thenReturn(true); + assertTrue(action.isFeatureEnabled()); + } + } + + @Test + void disabledFeatureIsRefusedBeforeAnythingElse() throws Exception { + UIAction action = new UIAction() { + @Override + public boolean isFeatureEnabled() { + return false; + } + }; + ActionInvocation invocation = mock(ActionInvocation.class); + when(invocation.getAction()).thenReturn(action); + + assertEquals(UIAction.DENIED, new UISecurityInterceptor().doIntercept(invocation)); + verify(invocation, never()).invoke(); + } + + @Test + void planetChangesRequirePost() throws Exception { + assertTrue(PlanetConfig.class.getMethod("save").isAnnotationPresent(RequiresPost.class)); + assertTrue(PlanetGroupSubs.class.getMethod("saveGroup").isAnnotationPresent(RequiresPost.class)); + assertTrue(PlanetGroupSubs.class.getMethod("saveSubscription").isAnnotationPresent(RequiresPost.class)); + assertTrue(PlanetGroupSubs.class.getMethod("deleteSubscription").isAnnotationPresent(RequiresPost.class)); + assertTrue(PlanetGroups.class.getMethod("delete").isAnnotationPresent(RequiresPost.class)); + } + + @Test + void rollerStackIncludesThePostCheck() throws Exception { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + Document doc; + try (InputStream in = getClass().getResourceAsStream("/struts.xml")) { + doc = factory.newDocumentBuilder().parse(in); + } + NodeList stacks = doc.getElementsByTagName("interceptor-stack"); + boolean found = false; + for (int i = 0; i < stacks.getLength(); i++) { + Element stack = (Element) stacks.item(i); + if (!"rollerStack".equals(stack.getAttribute("name"))) { + continue; + } + NodeList refs = stack.getElementsByTagName("interceptor-ref"); + for (int j = 0; j < refs.getLength(); j++) { + if ("RequiresPostInterceptor".equals(((Element) refs.item(j)).getAttribute("name"))) { + found = true; + } + } + } + assertTrue(found); + } + + @Test + void planetFeedIsNotServedWhileOff() throws Exception { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + try (MockedStatic config = mockStatic(WebloggerConfig.class); + MockedStatic factory = mockStatic(WebloggerFactory.class)) { + config.when(() -> WebloggerConfig.getBooleanProperty(ENABLED)).thenReturn(false); + + new PlanetFeedServlet().doGet(request, response); + + verify(response).sendError(HttpServletResponse.SC_NOT_FOUND); + factory.verifyNoInteractions(); + } + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java new file mode 100644 index 0000000000..47c71926f7 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java @@ -0,0 +1,112 @@ +/* + * 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. 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.ui.struts2.util; + +import java.util.HashMap; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; + +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.ActionProxy; +import org.apache.struts2.StrutsStatics; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class RequiresPostInterceptorTest { + + /** An action with one marked and one unmarked method. */ + public static class SampleAction { + @RequiresPost + public String save() { + return "saved"; + } + + public String execute() { + return "shown"; + } + } + + private final RequiresPostInterceptor interceptor = new RequiresPostInterceptor(); + + @Test + void markedMethodRefusesGet() throws Exception { + ActionInvocation invocation = invocation("save", "GET"); + + assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); + verify(invocation, never()).invoke(); + } + + @Test + void markedMethodAcceptsPost() throws Exception { + ActionInvocation invocation = invocation("save", "POST"); + + assertEquals("next", interceptor.intercept(invocation)); + verify(invocation).invoke(); + } + + @Test + void unmarkedMethodAcceptsGet() throws Exception { + ActionInvocation invocation = invocation("execute", "GET"); + + assertEquals("next", interceptor.intercept(invocation)); + verify(invocation).invoke(); + } + + @Test + void defaultMethodIsTreatedAsExecute() throws Exception { + ActionInvocation invocation = invocation(null, "GET"); + + assertEquals("next", interceptor.intercept(invocation)); + verify(invocation).invoke(); + } + + @Test + void markedMethodRefusedWhenRequestIsMissing() throws Exception { + ActionInvocation invocation = invocation("save", null); + + assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); + verify(invocation, never()).invoke(); + } + + private static ActionInvocation invocation(String methodName, String httpMethod) throws Exception { + Map contextMap = new HashMap<>(); + if (httpMethod != null) { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getMethod()).thenReturn(httpMethod); + contextMap.put(StrutsStatics.HTTP_REQUEST, request); + } + ActionProxy proxy = mock(ActionProxy.class); + when(proxy.getMethod()).thenReturn(methodName); + when(proxy.getActionName()).thenReturn("sample"); + + ActionInvocation invocation = mock(ActionInvocation.class); + when(invocation.getProxy()).thenReturn(proxy); + when(invocation.getAction()).thenReturn(new SampleAction()); + when(invocation.getInvocationContext()).thenReturn(new ActionContext(contextMap)); + when(invocation.invoke()).thenReturn("next"); + return invocation; + } +} From 2e967e233fade49c462a21cebbb36dad1765bb04 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 08:22:39 -0400 Subject: [PATCH 2/4] Apply @RequiresPost to overrides and idle Planet tasks while Planet is off RequiresPostInterceptor now walks the superclass chain, so an override that does not repeat the annotation still requires POST. RefreshRollerPlanetTask and SyncWebsitesTask return early while planet.aggregator.enabled is false. Add a 6.1.7 CHANGES.md section for the Planet changes. --- CHANGES.md | 12 +++++++ .../planet/tasks/RefreshRollerPlanetTask.java | 4 +++ .../planet/tasks/SyncWebsitesTask.java | 4 +++ .../struts2/util/RequiresPostInterceptor.java | 24 +++++++++++--- .../planet/ui/PlanetAvailabilityTest.java | 15 +++++++++ .../util/RequiresPostInterceptorTest.java | 31 ++++++++++++++++++- 6 files changed, 84 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2f4ad53f88..ab74004f91 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,17 @@ # Apache Roller — Changes +## 6.1.7 + +### Behaviour changes worth reading before upgrading + +- **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 + nothing. A site that uses Planet must set `planet.aggregator.enabled=true` + in `roller-custom.properties` before upgrading. +- **Planet admin changes require POST.** Saving or deleting Planet groups and + subscriptions is refused unless the request is a POST from the admin form. + ## 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/planet/tasks/RefreshRollerPlanetTask.java b/app/src/main/java/org/apache/roller/weblogger/planet/tasks/RefreshRollerPlanetTask.java index 367a8958b2..9babf25c6e 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/tasks/RefreshRollerPlanetTask.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/tasks/RefreshRollerPlanetTask.java @@ -131,6 +131,10 @@ public void init(String name) throws WebloggerException { @Override public void runTask() { + if (!WebloggerConfig.getBooleanProperty("planet.aggregator.enabled")) { + log.debug("Planet is disabled; not running " + getName()); + return; + } try { log.info("Refreshing Planet subscriptions"); diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/tasks/SyncWebsitesTask.java b/app/src/main/java/org/apache/roller/weblogger/planet/tasks/SyncWebsitesTask.java index 95675b2095..80f4b23789 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/tasks/SyncWebsitesTask.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/tasks/SyncWebsitesTask.java @@ -141,6 +141,10 @@ public void init(String name) throws WebloggerException { */ @Override public void runTask() { + if (!WebloggerConfig.getBooleanProperty("planet.aggregator.enabled")) { + log.debug("Planet is disabled; not running " + getName()); + return; + } log.info("Syncing local weblogs with planet subscriptions list"); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java index 77480c14cd..a87ad08b5c 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java @@ -60,11 +60,25 @@ static boolean requiresPost(ActionInvocation invocation) { if (methodName == null) { methodName = "execute"; } - try { - Method method = invocation.getAction().getClass().getMethod(methodName); - return method.isAnnotationPresent(RequiresPost.class); - } catch (NoSuchMethodException e) { - return false; + return isMarked(invocation.getAction().getClass(), methodName); + } + + /** + * True when the method, or any superclass method it overrides, is marked. + * Method annotations are not inherited, so an override that does not + * repeat the annotation must not drop the check. + */ + static boolean isMarked(Class type, String methodName) { + for (Class c = type; c != null; c = c.getSuperclass()) { + try { + Method method = c.getMethod(methodName); + if (method.isAnnotationPresent(RequiresPost.class)) { + return true; + } + } catch (NoSuchMethodException e) { + return false; + } } + return false; } } diff --git a/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java index 33f0123d10..d4a2306008 100644 --- a/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java @@ -28,6 +28,8 @@ import com.opensymphony.xwork2.ActionInvocation; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.config.WebloggerConfig; +import org.apache.roller.weblogger.planet.tasks.RefreshRollerPlanetTask; +import org.apache.roller.weblogger.planet.tasks.SyncWebsitesTask; import org.apache.roller.weblogger.ui.rendering.servlets.PlanetFeedServlet; import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.roller.weblogger.ui.struts2.util.UIAction; @@ -136,4 +138,17 @@ void planetFeedIsNotServedWhileOff() throws Exception { factory.verifyNoInteractions(); } } + + @Test + void planetTasksDoNothingWhileOff() { + try (MockedStatic config = mockStatic(WebloggerConfig.class); + MockedStatic factory = mockStatic(WebloggerFactory.class)) { + config.when(() -> WebloggerConfig.getBooleanProperty(ENABLED)).thenReturn(false); + + new RefreshRollerPlanetTask().runTask(); + new SyncWebsitesTask().runTask(); + + factory.verifyNoInteractions(); + } + } } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java index 47c71926f7..708a0e5907 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java @@ -49,6 +49,14 @@ public String execute() { } } + /** Overrides the marked method without repeating the annotation. */ + public static class OverridingAction extends SampleAction { + @Override + public String save() { + return "saved again"; + } + } + private final RequiresPostInterceptor interceptor = new RequiresPostInterceptor(); @Test @@ -83,6 +91,22 @@ void defaultMethodIsTreatedAsExecute() throws Exception { verify(invocation).invoke(); } + @Test + void overrideOfMarkedMethodStillRefusesGet() throws Exception { + ActionInvocation invocation = invocation("save", "GET", new OverridingAction()); + + assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); + verify(invocation, never()).invoke(); + } + + @Test + void overrideOfMarkedMethodAcceptsPost() throws Exception { + ActionInvocation invocation = invocation("save", "POST", new OverridingAction()); + + assertEquals("next", interceptor.intercept(invocation)); + verify(invocation).invoke(); + } + @Test void markedMethodRefusedWhenRequestIsMissing() throws Exception { ActionInvocation invocation = invocation("save", null); @@ -92,6 +116,11 @@ void markedMethodRefusedWhenRequestIsMissing() throws Exception { } private static ActionInvocation invocation(String methodName, String httpMethod) throws Exception { + return invocation(methodName, httpMethod, new SampleAction()); + } + + private static ActionInvocation invocation(String methodName, String httpMethod, + Object action) throws Exception { Map contextMap = new HashMap<>(); if (httpMethod != null) { HttpServletRequest request = mock(HttpServletRequest.class); @@ -104,7 +133,7 @@ private static ActionInvocation invocation(String methodName, String httpMethod) ActionInvocation invocation = mock(ActionInvocation.class); when(invocation.getProxy()).thenReturn(proxy); - when(invocation.getAction()).thenReturn(new SampleAction()); + when(invocation.getAction()).thenReturn(action); when(invocation.getInvocationContext()).thenReturn(new ActionContext(contextMap)); when(invocation.invoke()).thenReturn("next"); return invocation; From 08cdba335b49e40d4c72aa2476c0a8b42d4e36c5 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 09:00:21 -0400 Subject: [PATCH 3/4] Check for POST inline in the Planet actions Replace the @RequiresPost annotation and RequiresPostInterceptor with an isPostRequest() check in PlanetUIAction, called at the start of the five methods that change Planet state, as FrontpageSetup already does. The methods return DENIED for any other request method. --- .../weblogger/planet/ui/PlanetConfig.java | 5 +- .../weblogger/planet/ui/PlanetGroupSubs.java | 13 +- .../weblogger/planet/ui/PlanetGroups.java | 5 +- .../weblogger/planet/ui/PlanetUIAction.java | 11 ++ .../ui/struts2/util/RequiresPost.java | 36 ----- .../struts2/util/RequiresPostInterceptor.java | 84 ----------- app/src/main/resources/struts.xml | 3 - .../planet/ui/PlanetAvailabilityTest.java | 66 ++++---- .../util/RequiresPostInterceptorTest.java | 141 ------------------ 9 files changed, 60 insertions(+), 304 deletions(-) delete mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java delete mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java delete mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java index a950126ea4..ed331ed3c4 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetConfig.java @@ -18,7 +18,6 @@ package org.apache.roller.weblogger.planet.ui; -import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.RollerException; @@ -108,8 +107,10 @@ public String execute() { } - @RequiresPost public String save() { + if (!isPostRequest()) { + return DENIED; + } try { String incomingProp = null; diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java index 6c60bfa923..b4959f5471 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroupSubs.java @@ -16,7 +16,6 @@ package org.apache.roller.weblogger.planet.ui; -import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -114,8 +113,10 @@ public String execute() { /** * Save group. */ - @RequiresPost public String saveGroup() { + if (!isPostRequest()) { + return DENIED; + } validateGroup(); @@ -172,8 +173,10 @@ private void validateGroup() { /** * Save subscription, add to current group */ - @RequiresPost public String saveSubscription() { + if (!isPostRequest()) { + return DENIED; + } valudateNewSub(); @@ -225,8 +228,10 @@ public String saveSubscription() { /** * Delete subscription, reset form */ - @RequiresPost public String deleteSubscription() { + if (!isPostRequest()) { + return DENIED; + } if (getSubUrl() != null) { try { diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java index f01c2840ff..45ce5e58b4 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetGroups.java @@ -16,7 +16,6 @@ package org.apache.roller.weblogger.planet.ui; -import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.planet.business.PlanetManager; @@ -67,8 +66,10 @@ public String execute() { /** * Delete group */ - @RequiresPost public String delete() { + if (!isPostRequest()) { + return DENIED; + } if (getGroup() != null) { try { diff --git a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java index db34aa4ecc..c8976e8214 100644 --- a/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java +++ b/app/src/main/java/org/apache/roller/weblogger/planet/ui/PlanetUIAction.java @@ -20,9 +20,11 @@ import org.apache.commons.logging.LogFactory; import org.apache.roller.planet.business.PlanetManager; import org.apache.roller.planet.pojos.Planet; +import javax.servlet.http.HttpServletRequest; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.ui.struts2.util.UIAction; +import org.apache.struts2.ServletActionContext; /** @@ -46,6 +48,15 @@ public boolean isFeatureEnabled() { return WebloggerConfig.getBooleanProperty("planet.aggregator.enabled"); } + /** + * Planet changes are made only by POST, which the CSRF salt filter + * checks. Methods that change Planet state return DENIED otherwise. + */ + protected boolean isPostRequest() { + HttpServletRequest req = ServletActionContext.getRequest(); + return req != null && "POST".equalsIgnoreCase(req.getMethod()); + } + public Planet getPlanet() { if(planet == null) { try { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java deleted file mode 100644 index e39e72bd8e..0000000000 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPost.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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. 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.ui.struts2.util; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Marks a Struts action method that changes state. {@link RequiresPostInterceptor} - * refuses any request to it that is not a POST. - */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.METHOD) -public @interface RequiresPost { -} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java deleted file mode 100644 index a87ad08b5c..0000000000 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptor.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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. 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.ui.struts2.util; - -import static org.apache.struts2.StrutsStatics.HTTP_REQUEST; - -import java.lang.reflect.Method; -import javax.servlet.http.HttpServletRequest; - -import com.opensymphony.xwork2.ActionInvocation; -import com.opensymphony.xwork2.interceptor.AbstractInterceptor; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -/** - * Refuses requests that are not POSTs to action methods marked - * {@link RequiresPost}. Unmarked methods are not affected. - */ -public class RequiresPostInterceptor extends AbstractInterceptor { - - private static final long serialVersionUID = 1L; - private static final Log log = LogFactory.getLog(RequiresPostInterceptor.class); - - @Override - public String intercept(ActionInvocation invocation) throws Exception { - if (requiresPost(invocation)) { - HttpServletRequest request = (HttpServletRequest) - invocation.getInvocationContext().get(HTTP_REQUEST); - if (request == null || !"POST".equalsIgnoreCase(request.getMethod())) { - if (log.isDebugEnabled()) { - log.debug("Refusing " + (request == null ? "unknown" : request.getMethod()) - + " request to " + invocation.getProxy().getActionName() - + "!" + invocation.getProxy().getMethod()); - } - return UIAction.DENIED; - } - } - return invocation.invoke(); - } - - static boolean requiresPost(ActionInvocation invocation) { - String methodName = invocation.getProxy().getMethod(); - if (methodName == null) { - methodName = "execute"; - } - return isMarked(invocation.getAction().getClass(), methodName); - } - - /** - * True when the method, or any superclass method it overrides, is marked. - * Method annotations are not inherited, so an override that does not - * repeat the annotation must not drop the check. - */ - static boolean isMarked(Class type, String methodName) { - for (Class c = type; c != null; c = c.getSuperclass()) { - try { - Method method = c.getMethod(methodName); - if (method.isAnnotationPresent(RequiresPost.class)) { - return true; - } - } catch (NoSuchMethodException e) { - return false; - } - } - return false; - } -} diff --git a/app/src/main/resources/struts.xml b/app/src/main/resources/struts.xml index 13021933f7..ea14c602a4 100644 --- a/app/src/main/resources/struts.xml +++ b/app/src/main/resources/struts.xml @@ -39,8 +39,6 @@ class="org.apache.roller.weblogger.ui.struts2.util.UIActionPrepareInterceptor" /> - diff --git a/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java index d4a2306008..5604b7462b 100644 --- a/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/planet/ui/PlanetAvailabilityTest.java @@ -20,25 +20,24 @@ package org.apache.roller.weblogger.planet.ui; import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.xml.parsers.DocumentBuilderFactory; +import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.planet.tasks.RefreshRollerPlanetTask; import org.apache.roller.weblogger.planet.tasks.SyncWebsitesTask; import org.apache.roller.weblogger.ui.rendering.servlets.PlanetFeedServlet; -import org.apache.roller.weblogger.ui.struts2.util.RequiresPost; import org.apache.roller.weblogger.ui.struts2.util.UIAction; import org.apache.roller.weblogger.ui.struts2.util.UISecurityInterceptor; +import org.apache.struts2.StrutsStatics; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; -import org.w3c.dom.Document; -import org.w3c.dom.Element; -import org.w3c.dom.NodeList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -91,37 +90,40 @@ public boolean isFeatureEnabled() { } @Test - void planetChangesRequirePost() throws Exception { - assertTrue(PlanetConfig.class.getMethod("save").isAnnotationPresent(RequiresPost.class)); - assertTrue(PlanetGroupSubs.class.getMethod("saveGroup").isAnnotationPresent(RequiresPost.class)); - assertTrue(PlanetGroupSubs.class.getMethod("saveSubscription").isAnnotationPresent(RequiresPost.class)); - assertTrue(PlanetGroupSubs.class.getMethod("deleteSubscription").isAnnotationPresent(RequiresPost.class)); - assertTrue(PlanetGroups.class.getMethod("delete").isAnnotationPresent(RequiresPost.class)); + void planetChangesAreRefusedUnlessPosted() { + HttpServletRequest get = mock(HttpServletRequest.class); + when(get.getMethod()).thenReturn("GET"); + Map context = new HashMap<>(); + context.put(StrutsStatics.HTTP_REQUEST, get); + ActionContext.setContext(new ActionContext(context)); + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + assertEquals(UIAction.DENIED, new PlanetConfig().save()); + assertEquals(UIAction.DENIED, new PlanetGroups().delete()); + assertEquals(UIAction.DENIED, new PlanetGroupSubs().saveGroup()); + assertEquals(UIAction.DENIED, new PlanetGroupSubs().saveSubscription()); + assertEquals(UIAction.DENIED, new PlanetGroupSubs().deleteSubscription()); + factory.verifyNoInteractions(); + } finally { + ActionContext.setContext(null); + } } @Test - void rollerStackIncludesThePostCheck() throws Exception { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - Document doc; - try (InputStream in = getClass().getResourceAsStream("/struts.xml")) { - doc = factory.newDocumentBuilder().parse(in); - } - NodeList stacks = doc.getElementsByTagName("interceptor-stack"); - boolean found = false; - for (int i = 0; i < stacks.getLength(); i++) { - Element stack = (Element) stacks.item(i); - if (!"rollerStack".equals(stack.getAttribute("name"))) { - continue; - } - NodeList refs = stack.getElementsByTagName("interceptor-ref"); - for (int j = 0; j < refs.getLength(); j++) { - if ("RequiresPostInterceptor".equals(((Element) refs.item(j)).getAttribute("name"))) { - found = true; - } - } + void onlyAPostCountsAsAPostRequest() { + PlanetUIAction action = new PlanetGroups(); + try { + ActionContext.setContext(new ActionContext(new HashMap<>())); + assertFalse(action.isPostRequest()); + + HttpServletRequest post = mock(HttpServletRequest.class); + when(post.getMethod()).thenReturn("post"); + Map context = new HashMap<>(); + context.put(StrutsStatics.HTTP_REQUEST, post); + ActionContext.setContext(new ActionContext(context)); + assertTrue(action.isPostRequest()); + } finally { + ActionContext.setContext(null); } - assertTrue(found); } @Test diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java deleted file mode 100644 index 708a0e5907..0000000000 --- a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/util/RequiresPostInterceptorTest.java +++ /dev/null @@ -1,141 +0,0 @@ -/* - * 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. 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.ui.struts2.util; - -import java.util.HashMap; -import java.util.Map; -import javax.servlet.http.HttpServletRequest; - -import com.opensymphony.xwork2.ActionContext; -import com.opensymphony.xwork2.ActionInvocation; -import com.opensymphony.xwork2.ActionProxy; -import org.apache.struts2.StrutsStatics; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -class RequiresPostInterceptorTest { - - /** An action with one marked and one unmarked method. */ - public static class SampleAction { - @RequiresPost - public String save() { - return "saved"; - } - - public String execute() { - return "shown"; - } - } - - /** Overrides the marked method without repeating the annotation. */ - public static class OverridingAction extends SampleAction { - @Override - public String save() { - return "saved again"; - } - } - - private final RequiresPostInterceptor interceptor = new RequiresPostInterceptor(); - - @Test - void markedMethodRefusesGet() throws Exception { - ActionInvocation invocation = invocation("save", "GET"); - - assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); - verify(invocation, never()).invoke(); - } - - @Test - void markedMethodAcceptsPost() throws Exception { - ActionInvocation invocation = invocation("save", "POST"); - - assertEquals("next", interceptor.intercept(invocation)); - verify(invocation).invoke(); - } - - @Test - void unmarkedMethodAcceptsGet() throws Exception { - ActionInvocation invocation = invocation("execute", "GET"); - - assertEquals("next", interceptor.intercept(invocation)); - verify(invocation).invoke(); - } - - @Test - void defaultMethodIsTreatedAsExecute() throws Exception { - ActionInvocation invocation = invocation(null, "GET"); - - assertEquals("next", interceptor.intercept(invocation)); - verify(invocation).invoke(); - } - - @Test - void overrideOfMarkedMethodStillRefusesGet() throws Exception { - ActionInvocation invocation = invocation("save", "GET", new OverridingAction()); - - assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); - verify(invocation, never()).invoke(); - } - - @Test - void overrideOfMarkedMethodAcceptsPost() throws Exception { - ActionInvocation invocation = invocation("save", "POST", new OverridingAction()); - - assertEquals("next", interceptor.intercept(invocation)); - verify(invocation).invoke(); - } - - @Test - void markedMethodRefusedWhenRequestIsMissing() throws Exception { - ActionInvocation invocation = invocation("save", null); - - assertEquals(UIAction.DENIED, interceptor.intercept(invocation)); - verify(invocation, never()).invoke(); - } - - private static ActionInvocation invocation(String methodName, String httpMethod) throws Exception { - return invocation(methodName, httpMethod, new SampleAction()); - } - - private static ActionInvocation invocation(String methodName, String httpMethod, - Object action) throws Exception { - Map contextMap = new HashMap<>(); - if (httpMethod != null) { - HttpServletRequest request = mock(HttpServletRequest.class); - when(request.getMethod()).thenReturn(httpMethod); - contextMap.put(StrutsStatics.HTTP_REQUEST, request); - } - ActionProxy proxy = mock(ActionProxy.class); - when(proxy.getMethod()).thenReturn(methodName); - when(proxy.getActionName()).thenReturn("sample"); - - ActionInvocation invocation = mock(ActionInvocation.class); - when(invocation.getProxy()).thenReturn(proxy); - when(invocation.getAction()).thenReturn(action); - when(invocation.getInvocationContext()).thenReturn(new ActionContext(contextMap)); - when(invocation.invoke()).thenReturn("next"); - return invocation; - } -} From 1725b2a7302169c2847ecdf68468d8d50eeb3766 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 09:06:54 -0400 Subject: [PATCH 4/4] Enable Planet in PlanetManagerLocalTest The Planet tasks now do nothing while planet.aggregator.enabled is false, which is the new default, so the test turns it on while it runs them. --- .../weblogger/business/PlanetManagerLocalTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/src/test/java/org/apache/roller/weblogger/business/PlanetManagerLocalTest.java b/app/src/test/java/org/apache/roller/weblogger/business/PlanetManagerLocalTest.java index 846849bb7e..1a46e326d4 100644 --- a/app/src/test/java/org/apache/roller/weblogger/business/PlanetManagerLocalTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/business/PlanetManagerLocalTest.java @@ -23,6 +23,7 @@ import org.apache.roller.planet.pojos.Subscription; import org.apache.roller.planet.pojos.SubscriptionEntry; import org.apache.roller.weblogger.TestUtils; +import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.planet.tasks.RefreshRollerPlanetTask; import org.apache.roller.weblogger.planet.tasks.SyncWebsitesTask; import org.apache.roller.weblogger.pojos.User; @@ -32,6 +33,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import java.sql.Timestamp; import java.util.Date; @@ -40,6 +42,8 @@ import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mockStatic; /** @@ -138,7 +142,11 @@ public void tearDown() throws Exception { @Test public void testRefreshEntries() { - try { + // Planet is off by default, and its tasks do nothing while it is off. + try (MockedStatic config = + mockStatic(WebloggerConfig.class, CALLS_REAL_METHODS)) { + config.when(() -> WebloggerConfig.getBooleanProperty("planet.aggregator.enabled")) + .thenReturn(true); PlanetManager planet = WebloggerFactory.getWeblogger().getPlanetManager(); // run sync task to fill aggregator with websites created by super