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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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.ui.core.filters;

import java.io.IOException;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.roller.weblogger.config.WebloggerRuntimeConfig;

/**
* Gates the XML-RPC endpoint on <code>webservices.enableXmlRpc</code>, ahead of
* the servlet, so a disabled service is not reachable.
*/
public class XmlRpcEnabledFilter implements Filter {

private static final Log LOG = LogFactory.getLog(XmlRpcEnabledFilter.class);

@Override
public void init(FilterConfig filterConfig) {
// nothing to configure
}

@Override
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {

if (!WebloggerRuntimeConfig.getBooleanProperty("webservices.enableXmlRpc")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getBooleanProperty swallows exceptions and returns false, so a DB hiccup or a request before bootstrap answers 404 rather than an error. A LOG.warn when refusing would make that diagnosable; debug is easy to miss.

if (LOG.isDebugEnabled()) {
LOG.debug("XML-RPC service is disabled; rejecting request");
}
((HttpServletResponse) response).sendError(HttpServletResponse.SC_NOT_FOUND);

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sendError(404) gets routed to /roller-ui/errors/404.jsp, so a disabled endpoint now answers a desktop client's POST with an HTML page instead of the XML-RPC fault BLOGGERAPI_DISABLED_MSG that BaseAPIHandler still produces (line 152) but can no longer reach. Clients like MarsEdit will show a generic "invalid response". Consider writing a small XML-RPC fault with text/xml here, or at least setStatus(404) with a plain-text body so the error JSP isn't rendered.

return;
}

chain.doFilter(request, response);
}

@Override
public void destroy() {
// nothing to release
}
}
14 changes: 13 additions & 1 deletion app/src/main/webapp/WEB-INF/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
<filter-class>org.apache.roller.weblogger.ui.core.filters.BootstrapFilter</filter-class>
</filter>

<filter>
<filter-name>XmlRpcEnabledFilter</filter-name>
<filter-class>org.apache.roller.weblogger.ui.core.filters.XmlRpcEnabledFilter</filter-class>
</filter>

<filter>
<filter-name>RequestMappingFilter</filter-name>
<filter-class>org.apache.roller.weblogger.ui.rendering.filters.RequestMappingFilter</filter-class>
Expand Down Expand Up @@ -154,6 +159,11 @@
</filter-mapping>

<!-- Request mapping, this is what allows the urls to work -->
<filter-mapping>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: no <dispatcher> so this applies to REQUEST only; nothing forwards to /roller-services/xmlrpc today, but adding FORWARD / INCLUDE makes the gate match what the javadoc claims.

<filter-name>XmlRpcEnabledFilter</filter-name>
<url-pattern>/roller-services/xmlrpc</url-pattern>
</filter-mapping>

<filter-mapping>
<filter-name>RequestMappingFilter</filter-name>
<url-pattern>/*</url-pattern>
Expand Down Expand Up @@ -296,9 +306,11 @@
<init-param>
<description>
Sets whether the servlet supports vendor extensions for XML-RPC.
Roller uses only the standard XML-RPC value types; the library's
vendor extension types are unused and are switched off.
</description>
<param-name>enabledForExtensions</param-name>
<param-value>true</param-value>
<param-value>false</param-value>
</init-param>
</servlet>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* 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.xmlrpc;

import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.apache.xmlrpc.parser.XmlRpcRequestParser;
import org.apache.xmlrpc.common.XmlRpcHttpRequestConfigImpl;
import org.apache.xmlrpc.common.TypeFactoryImpl;
import org.apache.xmlrpc.common.XmlRpcController;
import org.apache.xmlrpc.server.XmlRpcServer;
import org.junit.jupiter.api.Test;
import org.xml.sax.InputSource;
import org.xml.sax.XMLReader;
import org.xml.sax.helpers.XMLReaderFactory;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Covers how the XML-RPC endpoint treats vendor extension types.
*
* <p>Roller's XML-RPC API uses only the standard value types, so the library's
* vendor extension types are switched off in <code>web.xml</code> and the
* endpoint is closed entirely while the service is disabled.
*/
public class XmlRpcExtensionTypeTest {

private static final Path WEB_XML =
Paths.get("src", "main", "webapp", "WEB-INF", "web.xml");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cwd-relative; surefire sets project.build.directory for this module and ApplicationResourcesTest / SQLScriptRunnerTest derive their paths from it.


private String parseRequest(String xml, boolean extensionsEnabled) throws Exception {
XmlRpcServer server = new XmlRpcServer();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: extensionsEnabled is only ever false, the (XmlRpcController) cast is a no-op, and XMLReaderFactory has been deprecated since Java 9 (SAXParserFactory.newInstance().newSAXParser().getXMLReader()).

XmlRpcHttpRequestConfigImpl config = new XmlRpcHttpRequestConfigImpl();
config.setEnabledForExtensions(extensionsEnabled);

XmlRpcRequestParser parser = new XmlRpcRequestParser(
config, new TypeFactoryImpl((XmlRpcController) server));
XMLReader reader = XMLReaderFactory.createXMLReader();
reader.setContentHandler(parser);
reader.parse(new InputSource(
new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8))));
return parser.getMethodName();
}

private static final String ORDINARY_REQUEST =
"<?xml version=\"1.0\"?><methodCall>"
+ "<methodName>blogger.getUsersBlogs</methodName>"
+ "<params><param><value><string>hello</string></value></param></params>"
+ "</methodCall>";

/** The shipped configuration must not enable the extension types. */
@Test
public void shippedConfigurationDisablesExtensionTypes() throws Exception {
String webXml = new String(Files.readAllBytes(WEB_XML), StandardCharsets.UTF_8);
int idx = webXml.indexOf("enabledForExtensions");
assertTrue(idx > 0, "enabledForExtensions param not found in web.xml");
String tail = webXml.substring(idx, Math.min(idx + 300, webXml.length()));
assertTrue(tail.contains("<param-value>false</param-value>"),
"the XML-RPC servlet must not enable vendor extension types:\n" + tail);
}

/** The endpoint is closed by a filter while the service is switched off. */
@Test
public void endpointIsGatedWhileTheServiceIsDisabled() throws Exception {
String webXml = new String(Files.readAllBytes(WEB_XML), StandardCharsets.UTF_8);
assertTrue(webXml.contains("XmlRpcEnabledFilter"),
"a filter must gate the XML-RPC endpoint");
int mapping = webXml.indexOf("<filter-name>XmlRpcEnabledFilter</filter-name>",
webXml.indexOf("<filter-mapping>"));
assertTrue(mapping > 0, "the gating filter must be mapped");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

indexOf("/roller-services/xmlrpc", mapping) > 0 is also satisfied by the later <servlet-mapping> for XmlRpcServlet, so a typo in the filter's url-pattern still passes. Parse the <filter-mapping> element and compare its url-pattern directly.

assertTrue(webXml.indexOf("/roller-services/xmlrpc", mapping) > 0,
"the gating filter must be mapped to the XML-RPC endpoint");
}

/** Ordinary XML-RPC calls must still parse with extensions disabled. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The description says extension types are rejected and the filter is tested, but this class only parses an ordinary call. A request containing <ex:serializable> (or <ex:nil/>) asserted to fail with extensions disabled, plus an XmlRpcEnabledFilter test with a mocked sendError, would actually pin the security property this PR introduces.

@Test
public void ordinaryCallsStillParseWithExtensionsDisabled() throws Exception {
String methodName = parseRequest(ORDINARY_REQUEST, false);
assertNotNull(methodName, "an ordinary XML-RPC call must still parse");
assertEquals("blogger.getUsersBlogs", methodName);
}
}
Loading