Replace ROME Propono AtomPub server with self-contained StAX implementation by snoopdave · Pull Request #161 · apache/roller

snoopdave · 2026-08-13T21:30:04Z

What

Reimplements the Atom Publishing Protocol (RFC 5023) server using only JDK StAX (javax.xml.stream) and plain DTOs — no ROME, no Propono.

Why

The AtomPub server was built on ROME's rome-propono, which is only available up to ROME 1.19.0 (the last release that ships Propono). That pin held the entire ROME stack at 1.19.0 for the whole app, even though feed rendering doesn't use Propono. Dropping rome-propono frees ROME-for-feeds to be upgraded independently in a later, deliberate step.

All Propono usage was confined to webservices/atomprotocol/; feeds and the Planet aggregator are untouched.

Changes

RollerAtomServlet (new) replaces Propono's AtomServlet — method dispatch, 201 Created + Location/Content-Location, and media streaming ported over. Wired in web.xml; propono.properties and RollerAtomHandlerFactory removed.
New wire model (AtomEntry, AtomFeed, AtomContent, AtomLink, AtomPerson, AtomCategory, plus the service-doc DTOs and AtomMediaResource) with StAX AtomWriter / AtomReader. AtomReader disables DTDs and external entities (XXE-safe).
RollerAtomHandler / RollerAtomService / EntryCollection / MediaCollection keep their Roller domain-mapping logic; only the ROME types they touched changed. Fixes a latent BASIC-auth bug that compared against the null instance field instead of the looked-up user's password.
Auth: keep BASIC + OAuth, drop WSSE (removes WSSEUtilities and the wsse choice from the admin config labels, en/ja/zh_CN).

Design decisions

XML: JDK StAX only — no JDOM/JAXB. javax.xml.stream is not part of the javax→jakarta migration, so it's safe.
Scope: server only (the AtomPub server receives requests; there is no outbound HTTP in this path).

Testing

Unit tests for the reader, writer, DTOs, and request wrapper.
Integration test (RollerAtomProtocolTest) driving the full create / retrieve / update / delete lifecycle plus service-doc and media upload against in-memory Derby.
Schema-validation tests (AtomSchemaValidationTest) that validate AtomWriter output against the RFC 4287 (Atom) and RFC 5023 (AtomPub) RELAX NG schemas using Jing.

All 34 new tests pass (mvn -pl app test -Dtest='...atomprotocol.*').

Note: the HTTP transport and BASIC auth over the wire need the Spring web context and aren't exercised by the JUnit reactor. Recommend confirming wire-format interop with an over-the-wire exerciser (e.g. APE) against a deployed instance, since the previous format was ROME-generated.

…tation Reimplements the Atom Publishing Protocol (RFC 5023) server using only JDK StAX (javax.xml.stream) and plain DTOs -- no ROME, no Propono. This removes the rome-propono dependency that pinned the entire ROME stack to 1.19.0, freeing ROME-for-feeds to be upgraded independently. - New Roller-owned servlet (RollerAtomServlet) replaces Propono's AtomServlet; method dispatch, 201 Created/Location handling and media streaming are ported over. - New wire model (AtomEntry/AtomFeed/AtomContent/AtomLink/AtomPerson/ AtomCategory and the service-doc DTOs) with StAX AtomWriter/AtomReader. AtomReader disables DTDs and external entities (XXE-safe). - RollerAtomHandler/RollerAtomService/EntryCollection/MediaCollection keep their domain-mapping logic; only the ROME types they touched changed. Fixes a latent BASIC-auth bug that compared against the null instance field instead of the looked-up user's password. - Auth: keep BASIC + OAuth, drop WSSE (removes WSSEUtilities and the wsse choice from the admin config labels). - Adds unit tests for the reader/writer/DTOs/request, an integration test driving the create/retrieve/update/delete lifecycle against in-memory Derby, and schema-validation tests that check AtomWriter output against the RFC 4287/5023 RELAX NG schemas via Jing. The HTTP transport and BASIC auth over the wire require the Spring web context and are not exercised by the JUnit reactor; verify those with an over-the-wire exerciser (e.g. APE) against a deployed instance.

mraible

I ran the same multi-agent review process over this PR that we've been using on the ROL-2183 stack (find in parallel, then adversarially verify every candidate; one finding below was verified against the rome-propono 1.19.0 bytecode). Nine verified findings and one lower-confidence one are inline on the relevant lines.

Two coordination notes that aren't tied to a single file:

The WSSE removal overlaps with #154, which keeps WSSE and currently tells admins AtomPub auth is "basic or wsse" (globalConfig text and ROL-2183 say the same). Happy to go either way, but the two PRs should agree, and whichever merges second inherits the reconciliation.

The Playwright branch of the stack (#157) has a WebServicesIT that exercises AtomPub end to end over Basic auth in CI, and it would have caught several of the inline findings here. Porting or extending it against this implementation is probably the highest-leverage follow-up, and I'm glad to do that once this lands.

Really glad to see AtomPub get a self-contained implementation, the Propono dependency has been a millstone for the Jakarta work.

mraible · 2026-08-14T01:40:52Z

+
+    private byte[] readBody(HttpServletRequest request) throws IOException {
+        try (InputStream in = request.getInputStream()) {
+            return in.readAllBytes();


readBody() pulls the entire request body into memory with readAllBytes() before any size or quota check runs. The Propono flow streamed the body to a temp file, so a multi-gigabyte POST to the media collection now turns into an OutOfMemoryError instead of a quota rejection, and any authenticated user can trigger it. Streaming to a temp file (or bounding the read against the media quota first) restores the old behavior.

mraible · 2026-08-14T01:40:52Z

+        String src = r.getAttributeValue(null, "src");
+        content.setSrc(src);
+        if (src == null) {
+            content.setValue(r.getElementText());


getElementText() throws when the element has child elements, so content, summary, and title with type="xhtml" (valid per RFC 4287, and accepted by the ROME parser this replaces) now fail to parse and the client gets a 500. Clients that publish xhtml content can't post at all. This needs a branch that captures the child XML as a string when type is xhtml.

mraible · 2026-08-14T01:40:52Z

+
    public void deleteEntry(AtomRequest areq) throws AtomException {
        try {
            String[] pathInfo = StringUtils.split(areq.getPathInfo(), "/");


In this method (lines 424-429, unchanged but carried forward): fileName strips the .media-link suffix but the lookup still passes the unstripped path to getMediaFileByPath, so it never finds the file, mf is null, and removeMediaFile NPEs. DELETE on the rel="edit" URI the server itself advertises always returns 500, which means media can never be deleted through AtomPub. Passing the stripped name to the lookup fixes it, and a delete test would keep it fixed.

mraible · 2026-08-14T01:40:52Z

@@ -120,10 +112,6 @@ public RollerAtomHandler(HttpServletRequest request, HttpServletResponse respons
        String userName;
        if ("oauth".equals(WebloggerRuntimeConfig.getProperty("webservices.atomPubAuth"))) {


webservices.atomPubAuth is a runtime property persisted in the roller_properties table, and existing installs can have wsse stored there. With the wsse branch gone, that value is silently reinterpreted as Basic, so clients sending X-WSSE headers get 401s with nothing in the logs pointing at the removed option. We hit the identical trap in #154 with the removed oauth value; the fix there was to refuse authentication for unrecognized values and log an error naming the property and the valid options. Worth deciding together whether wsse stays (it survives in #154) or goes, since the globalConfig text and ROL-2183 currently say "basic or wsse".

mraible · 2026-08-14T01:40:52Z

+                if (r.next() != XMLStreamConstants.START_ELEMENT) {
+                    continue;
+                }
+                String ns = r.getNamespaceURI();


parseEntry() matches START_ELEMENTs at any depth, so metadata inside a nested atom:source element (RFC 4287 4.2.11) is read as if it belonged to the entry: a posted entry that carries <title>Other Blog</title>2020-01-01... gets stored with the source's title and a back-dated timestamp. Silent wrong data, no error. Tracking element depth (or skipping the source subtree) avoids it.

mraible · 2026-08-14T01:40:52Z

+        w.writeCharacters(entry.isDraft() ? "yes" : "no");
+        w.writeEndElement();
+        if (entry.getEdited() != null) {
+            w.writeStartElement(APP_NS, "edited");


app:edited is written inside app:control, but RFC 5023 10.2 defines it as a direct child of atom:entry, and the Propono generator wrote it on the entry root (checked against rome-propono 1.19.0). Conforming clients looking for the edited timestamp in the spec location now see it as missing.

mraible · 2026-08-14T01:40:52Z

+
+        String contentType = areq.getContentType();
+        AtomEntry created;
+        if (contentType != null && contentType.startsWith("application/atom+xml")) {


A POST with no Content-Type header falls into the media branch with a null type and NPEs further down (Utilities.replaceNonAlphanumeric when Slug is also absent), surfacing as a 500. Propono answered this with a clear "No content-type specified in request" client error. A null check here that returns 415 keeps a malformed request from looking like a server bug. Fun fact: the Propono servlet had the inverse bug, an NPE on the exact mapping with no path info, which we fixed in #154.

mraible · 2026-08-14T01:40:52Z

+                try {
                    // Parse pathinfo to determine file path
                    String filePath = filePathFromPathInfo(pathInfo);
                    MediaFile mf = fmgr.getMediaFileByOriginalPath(website, filePath);


getMediaResource() doesn't null-check the getMediaFileByOriginalPath result, so a GET for a deleted or misspelled resource NPEs and returns 500 with the message "Unexpected error during file upload" for what should be a plain 404. Carried forward from the old code, but this rewrite is the right moment to fix it.

mraible · 2026-08-14T01:40:53Z

@@ -1,2 +0,0 @@
-com.rometools.propono.atom.server.AtomHandlerFactory=\


Deleting this and RollerAtomHandlerFactory removes the pluggable AtomHandlerFactory extension point, and the servlet now hard-codes new RollerAtomHandler(...). Any deployment that overrode the factory (custom auth is the classic case) silently loses its handler on upgrade. If dropping the extension point is intentional, a release-note line would spare those users a debugging session.

mraible · 2026-08-14T01:40:53Z

+    public static final String APP_NS = "http://www.w3.org/2007/app";
+
+    /** Media type for an Atom entry. */
+    public static final String ENTRY_MEDIA_TYPE = "application/atom+xml;type=entry";


Lower confidence than the rest, but flagging: ENTRY_MEDIA_TYPE has no charset parameter while FEED_MEDIA_TYPE and SERVICE_MEDIA_TYPE both declare charset=utf-8, and Propono declared it on entry responses too. Clients that fall back to ISO-8859-1 when charset is absent will mojibake non-ASCII titles on GET entry and 201 responses.

mraible reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

Replace ROME Propono AtomPub server with self-contained StAX implementation - #161

Replace ROME Propono AtomPub server with self-contained StAX implementation#161
snoopdave wants to merge 1 commit into
apache:masterfrom
snoopdave:replace-propono-atompub

snoopdave commented Aug 13, 2026

Uh oh!

mraible left a comment

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

mraible Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -120,10 +112,6 @@ public RollerAtomHandler(HttpServletRequest request, HttpServletResponse respons
		String userName;
		if ("oauth".equals(WebloggerRuntimeConfig.getProperty("webservices.atomPubAuth"))) {

		@@ -1,2 +0,0 @@
		com.rometools.propono.atom.server.AtomHandlerFactory=\

Uh oh!

Conversation

snoopdave commented Aug 13, 2026

What

Why

Changes

Design decisions

Testing

Uh oh!

mraible left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants