mraible commented on code in PR #161:
URL: https://github.com/apache/roller/pull/161#discussion_r3780389740


##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -0,0 +1,216 @@
+/*
+* 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.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * Dispatcher servlet for Roller's Atom Publishing Protocol (RFC 5023)
+ * implementation. Replaces the ROME Propono {@code AtomServlet}: it
+ * authenticates the request, routes by HTTP method and URI shape to
+ * {@link RollerAtomHandler}, and serializes/parses Atom XML via {@link 
AtomWriter}
+ * and {@link AtomReader}. No ROME or Propono types are involved.
+ */
+public class RollerAtomServlet extends HttpServlet {
+
+    private static final Log log =
+            LogFactory.getFactory().getInstance(RollerAtomServlet.class);
+
+    @Override
+    protected void doGet(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "GET");
+    }
+
+    @Override
+    protected void doPost(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "POST");
+    }
+
+    @Override
+    protected void doPut(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "PUT");
+    }
+
+    @Override
+    protected void doDelete(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "DELETE");
+    }
+
+    private void process(HttpServletRequest request, HttpServletResponse 
response, String method)
+            throws IOException {
+
+        RollerAtomHandler handler = new RollerAtomHandler(request, response);
+        String userName = handler.getAuthenticatedUsername();
+        if (userName == null) {
+            // The OAuth path may have already written a challenge/error 
response.
+            if (!response.isCommitted()) {
+                response.setHeader("WWW-Authenticate", "Basic 
realm=\"Roller\"");
+                response.sendError(HttpServletResponse.SC_UNAUTHORIZED, 
"Authentication error");
+            }
+            return;
+        }
+
+        byte[] body = null;
+        if ("POST".equals(method) || "PUT".equals(method)) {
+            body = readBody(request);
+        }
+        AtomRequest areq = new AtomRequest(request, body);
+
+        try {
+            switch (method) {
+                case "GET":
+                    doGet(handler, areq, response);
+                    break;
+                case "POST":
+                    doPost(handler, areq, response);
+                    break;
+                case "PUT":
+                    doPut(handler, areq, response);
+                    break;
+                case "DELETE":
+                    handler.deleteEntry(areq);
+                    response.setStatus(HttpServletResponse.SC_OK);
+                    break;
+                default:
+                    
response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+            }
+        } catch (AtomException ae) {
+            log.debug("Returning error to client: " + ae.getMessage(), ae);
+            if (!response.isCommitted()) {
+                response.sendError(ae.getStatus(), ae.getMessage());
+            }
+        } catch (Exception e) {
+            log.error("Unexpected error handling AtomPub request", e);
+            if (!response.isCommitted()) {
+                
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, 
e.getMessage());
+            }
+        }
+    }
+
+    private void doGet(RollerAtomHandler handler, AtomRequest areq, 
HttpServletResponse response)
+            throws AtomException, IOException {
+
+        if (handler.isAtomServiceURI(areq)) {
+            AtomServiceDoc service = handler.getAtomService(areq);
+            response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE);
+            new AtomWriter().writeServiceDoc(response.getOutputStream(), 
service);
+
+        } else if (handler.isCollectionURI(areq)) {
+            AtomFeed feed = handler.getCollection(areq);
+            response.setContentType(AtomConstants.FEED_MEDIA_TYPE);
+            new AtomWriter().writeFeed(response.getOutputStream(), feed);
+
+        } else if (handler.isEntryURI(areq)) {
+            AtomEntry entry = handler.getEntry(areq);
+            response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+            new AtomWriter().writeEntry(response.getOutputStream(), entry);
+
+        } else if (handler.isMediaEditURI(areq)) {
+            AtomMediaResource resource = handler.getMediaResource(areq);
+            if (resource.getContentType() != null) {
+                response.setContentType(resource.getContentType());
+            }
+            response.setContentLengthLong(resource.getContentLength());
+            if (resource.getLastModified() != null) {
+                response.setDateHeader("Last-Modified", 
resource.getLastModified().getTime());
+            }
+            try (InputStream in = resource.getInputStream()) {
+                in.transferTo(response.getOutputStream());
+            }
+
+        } else {
+            throw new AtomNotFoundException("Cannot find specified resource");
+        }
+    }
+
+    private void doPost(RollerAtomHandler handler, AtomRequest areq, 
HttpServletResponse response)
+            throws AtomException {
+
+        if (!handler.isCollectionURI(areq)) {
+            throw new AtomNotFoundException("Cannot POST to specified URI");
+        }
+
+        String contentType = areq.getContentType();
+        AtomEntry created;
+        if (contentType != null && 
contentType.startsWith("application/atom+xml")) {
+            AtomEntry entry = new 
AtomReader().parseEntry(areq.getInputStream());
+            created = handler.postEntry(areq, entry);
+        } else {
+            // Media POST: synthesize an entry carrying the request content 
type
+            // and Slug; the binary data is read from the request body.
+            AtomEntry mediaEntry = new AtomEntry();
+            AtomContent content = new AtomContent();
+            content.setType(contentType);
+            mediaEntry.setContent(content);
+            mediaEntry.setTitle(areq.getHeader("Slug"));
+            created = handler.postMedia(areq, mediaEntry);
+        }
+        writeCreated(response, created);
+    }
+
+    private void doPut(RollerAtomHandler handler, AtomRequest areq, 
HttpServletResponse response)
+            throws AtomException {
+
+        if (handler.isEntryURI(areq)) {
+            AtomEntry entry = new 
AtomReader().parseEntry(areq.getInputStream());
+            handler.putEntry(areq, entry);
+            response.setStatus(HttpServletResponse.SC_OK);
+        } else if (handler.isMediaEditURI(areq)) {
+            handler.putMedia(areq);
+            response.setStatus(HttpServletResponse.SC_OK);
+        } else {
+            throw new AtomNotFoundException("Cannot PUT to specified URI");
+        }
+    }
+
+    private void writeCreated(HttpServletResponse response, AtomEntry entry)
+            throws AtomException {
+        String editHref = entry.getLinkHref("edit");
+        if (editHref != null) {
+            response.setHeader("Location", editHref);
+            response.setHeader("Content-Location", editHref);
+        }
+        response.setStatus(HttpServletResponse.SC_CREATED);
+        response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+        try {
+            OutputStream out = response.getOutputStream();
+            new AtomWriter().writeEntry(out, entry);
+        } catch (IOException ioe) {
+            throw new AtomException("Error writing created entry", ioe);
+        }
+    }
+
+    private byte[] readBody(HttpServletRequest request) throws IOException {
+        try (InputStream in = request.getInputStream()) {
+            return in.readAllBytes();

Review Comment:
   `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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomReader.java:
##########
@@ -0,0 +1,146 @@
+/*
+* 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 static 
org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.APP_NS;
+import static 
org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.ATOM_NS;
+
+import java.io.InputStream;
+import java.time.Instant;
+import java.time.OffsetDateTime;
+import java.util.Date;
+
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+
+/**
+ * Parses an incoming AtomPub request body (an atom:entry) into the wire model
+ * using the JDK StAX API. Replaces ROME's Atom parser.
+ *
+ * <p>DTD processing and external entities are disabled to protect against XXE
+ * attacks.
+ */
+public class AtomReader {
+
+    private final XMLInputFactory factory;
+
+    public AtomReader() {
+        factory = XMLInputFactory.newInstance();
+        factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
+        factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, 
Boolean.FALSE);
+    }
+
+    static Date parseDate(String text) {
+        if (text == null || text.isBlank()) {
+            return null;
+        }
+        String trimmed = text.trim();
+        try {
+            return Date.from(OffsetDateTime.parse(trimmed).toInstant());
+        } catch (Exception ignored) {
+            try {
+                return Date.from(Instant.parse(trimmed));
+            } catch (Exception ignored2) {
+                return null;
+            }
+        }
+    }
+
+    /**
+     * Parse an atom:entry from the given stream. The author is intentionally 
not
+     * read; the server sets the entry's creator from the authenticated user.
+     */
+    public AtomEntry parseEntry(InputStream in) throws AtomException {
+        XMLStreamReader r = null;
+        try {
+            r = factory.createXMLStreamReader(in, "UTF-8");
+            AtomEntry entry = new AtomEntry();
+            while (r.hasNext()) {
+                if (r.next() != XMLStreamConstants.START_ELEMENT) {
+                    continue;
+                }
+                String ns = r.getNamespaceURI();
+                String name = r.getLocalName();
+                if (ATOM_NS.equals(ns)) {
+                    switch (name) {
+                        case "id":
+                            entry.setId(r.getElementText());
+                            break;
+                        case "title":
+                            entry.setTitle(r.getElementText());
+                            break;
+                        case "summary":
+                            entry.setSummary(readContent(r));
+                            break;
+                        case "content":
+                            if (entry.getContent() == null) {
+                                entry.setContent(readContent(r));
+                            }
+                            break;
+                        case "published":
+                            entry.setPublished(parseDate(r.getElementText()));
+                            break;
+                        case "updated":
+                            entry.setUpdated(parseDate(r.getElementText()));
+                            break;
+                        case "category":
+                            entry.getCategories().add(readCategory(r));
+                            break;
+                        default:
+                            break;
+                    }
+                } else if (APP_NS.equals(ns) && "draft".equals(name)) {
+                    String value = r.getElementText();
+                    entry.setDraft(value != null && 
value.trim().equalsIgnoreCase("yes"));
+                }
+            }
+            return entry;
+        } catch (XMLStreamException ex) {
+            throw new AtomException("Error parsing Atom entry", ex);
+        } finally {
+            if (r != null) {
+                try {
+                    r.close();
+                } catch (XMLStreamException ignored) {
+                    // nothing useful to do on close failure
+                }
+            }
+        }
+    }
+
+    private AtomContent readContent(XMLStreamReader r) throws 
XMLStreamException {
+        AtomContent content = new AtomContent();
+        content.setType(r.getAttributeValue(null, "type"));
+        String src = r.getAttributeValue(null, "src");
+        content.setSrc(src);
+        if (src == null) {
+            content.setValue(r.getElementText());

Review Comment:
   `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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java:
##########
@@ -420,15 +406,13 @@ public void putMedia(AtomRequest areq) throws 
AtomException {
                 }
             }
             throw new AtomException("Incorrect path information");
-        
+
         } catch (WebloggerException re) {
             throw new AtomException("Posting media");
-        } catch (IOException ioe) {
-            throw new AtomException("Posting media", ioe);
         }
     }
-    
-    
+
+
     public void deleteEntry(AtomRequest areq) throws AtomException {
         try {
             String[] pathInfo = StringUtils.split(areq.getPathInfo(), "/");

Review Comment:
   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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomHandler.java:
##########
@@ -120,10 +112,6 @@ public RollerAtomHandler(HttpServletRequest request, 
HttpServletResponse respons
         String userName;
         if 
("oauth".equals(WebloggerRuntimeConfig.getProperty("webservices.atomPubAuth"))) 
{

Review Comment:
   `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".



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomReader.java:
##########
@@ -0,0 +1,146 @@
+/*
+* 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 static 
org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.APP_NS;
+import static 
org.apache.roller.weblogger.webservices.atomprotocol.AtomConstants.ATOM_NS;
+
+import java.io.InputStream;
+import java.time.Instant;
+import java.time.OffsetDateTime;
+import java.util.Date;
+
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+
+/**
+ * Parses an incoming AtomPub request body (an atom:entry) into the wire model
+ * using the JDK StAX API. Replaces ROME's Atom parser.
+ *
+ * <p>DTD processing and external entities are disabled to protect against XXE
+ * attacks.
+ */
+public class AtomReader {
+
+    private final XMLInputFactory factory;
+
+    public AtomReader() {
+        factory = XMLInputFactory.newInstance();
+        factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
+        factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, 
Boolean.FALSE);
+    }
+
+    static Date parseDate(String text) {
+        if (text == null || text.isBlank()) {
+            return null;
+        }
+        String trimmed = text.trim();
+        try {
+            return Date.from(OffsetDateTime.parse(trimmed).toInstant());
+        } catch (Exception ignored) {
+            try {
+                return Date.from(Instant.parse(trimmed));
+            } catch (Exception ignored2) {
+                return null;
+            }
+        }
+    }
+
+    /**
+     * Parse an atom:entry from the given stream. The author is intentionally 
not
+     * read; the server sets the entry's creator from the authenticated user.
+     */
+    public AtomEntry parseEntry(InputStream in) throws AtomException {
+        XMLStreamReader r = null;
+        try {
+            r = factory.createXMLStreamReader(in, "UTF-8");
+            AtomEntry entry = new AtomEntry();
+            while (r.hasNext()) {
+                if (r.next() != XMLStreamConstants.START_ELEMENT) {
+                    continue;
+                }
+                String ns = r.getNamespaceURI();

Review Comment:
   `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 `<source><title>Other 
Blog</title>``<updated>2020-01-01...</updated>`</source> 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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -0,0 +1,216 @@
+/*
+* 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.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * Dispatcher servlet for Roller's Atom Publishing Protocol (RFC 5023)
+ * implementation. Replaces the ROME Propono {@code AtomServlet}: it
+ * authenticates the request, routes by HTTP method and URI shape to
+ * {@link RollerAtomHandler}, and serializes/parses Atom XML via {@link 
AtomWriter}
+ * and {@link AtomReader}. No ROME or Propono types are involved.
+ */
+public class RollerAtomServlet extends HttpServlet {
+
+    private static final Log log =
+            LogFactory.getFactory().getInstance(RollerAtomServlet.class);
+
+    @Override
+    protected void doGet(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "GET");
+    }
+
+    @Override
+    protected void doPost(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "POST");
+    }
+
+    @Override
+    protected void doPut(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "PUT");
+    }
+
+    @Override
+    protected void doDelete(HttpServletRequest request, HttpServletResponse 
response)
+            throws IOException {
+        process(request, response, "DELETE");
+    }
+
+    private void process(HttpServletRequest request, HttpServletResponse 
response, String method)
+            throws IOException {
+
+        RollerAtomHandler handler = new RollerAtomHandler(request, response);
+        String userName = handler.getAuthenticatedUsername();
+        if (userName == null) {
+            // The OAuth path may have already written a challenge/error 
response.
+            if (!response.isCommitted()) {
+                response.setHeader("WWW-Authenticate", "Basic 
realm=\"Roller\"");
+                response.sendError(HttpServletResponse.SC_UNAUTHORIZED, 
"Authentication error");
+            }
+            return;
+        }
+
+        byte[] body = null;
+        if ("POST".equals(method) || "PUT".equals(method)) {
+            body = readBody(request);
+        }
+        AtomRequest areq = new AtomRequest(request, body);
+
+        try {
+            switch (method) {
+                case "GET":
+                    doGet(handler, areq, response);
+                    break;
+                case "POST":
+                    doPost(handler, areq, response);
+                    break;
+                case "PUT":
+                    doPut(handler, areq, response);
+                    break;
+                case "DELETE":
+                    handler.deleteEntry(areq);
+                    response.setStatus(HttpServletResponse.SC_OK);
+                    break;
+                default:
+                    
response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+            }
+        } catch (AtomException ae) {
+            log.debug("Returning error to client: " + ae.getMessage(), ae);
+            if (!response.isCommitted()) {
+                response.sendError(ae.getStatus(), ae.getMessage());
+            }
+        } catch (Exception e) {
+            log.error("Unexpected error handling AtomPub request", e);
+            if (!response.isCommitted()) {
+                
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, 
e.getMessage());
+            }
+        }
+    }
+
+    private void doGet(RollerAtomHandler handler, AtomRequest areq, 
HttpServletResponse response)
+            throws AtomException, IOException {
+
+        if (handler.isAtomServiceURI(areq)) {
+            AtomServiceDoc service = handler.getAtomService(areq);
+            response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE);
+            new AtomWriter().writeServiceDoc(response.getOutputStream(), 
service);
+
+        } else if (handler.isCollectionURI(areq)) {
+            AtomFeed feed = handler.getCollection(areq);
+            response.setContentType(AtomConstants.FEED_MEDIA_TYPE);
+            new AtomWriter().writeFeed(response.getOutputStream(), feed);
+
+        } else if (handler.isEntryURI(areq)) {
+            AtomEntry entry = handler.getEntry(areq);
+            response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+            new AtomWriter().writeEntry(response.getOutputStream(), entry);
+
+        } else if (handler.isMediaEditURI(areq)) {
+            AtomMediaResource resource = handler.getMediaResource(areq);
+            if (resource.getContentType() != null) {
+                response.setContentType(resource.getContentType());
+            }
+            response.setContentLengthLong(resource.getContentLength());
+            if (resource.getLastModified() != null) {
+                response.setDateHeader("Last-Modified", 
resource.getLastModified().getTime());
+            }
+            try (InputStream in = resource.getInputStream()) {
+                in.transferTo(response.getOutputStream());
+            }
+
+        } else {
+            throw new AtomNotFoundException("Cannot find specified resource");
+        }
+    }
+
+    private void doPost(RollerAtomHandler handler, AtomRequest areq, 
HttpServletResponse response)
+            throws AtomException {
+
+        if (!handler.isCollectionURI(areq)) {
+            throw new AtomNotFoundException("Cannot POST to specified URI");
+        }
+
+        String contentType = areq.getContentType();
+        AtomEntry created;
+        if (contentType != null && 
contentType.startsWith("application/atom+xml")) {

Review Comment:
   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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/MediaCollection.java:
##########
@@ -216,13 +201,14 @@ public AtomMediaResource getMediaResource(AtomRequest 
areq) throws AtomException
                 throw new AtomNotAuthorizedException("Not authorized to edit 
weblog: " + handle);
             }
             if (pathInfo.length > 1) {
-                try {                                        
+                try {
                     // Parse pathinfo to determine file path
                     String filePath = filePathFromPathInfo(pathInfo);
                     MediaFile mf = fmgr.getMediaFileByOriginalPath(website, 
filePath);

Review Comment:
   `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.



##########
app/src/main/resources/propono.properties:
##########
@@ -1,2 +0,0 @@
-com.rometools.propono.atom.server.AtomHandlerFactory=\

Review Comment:
   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.



##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/AtomConstants.java:
##########
@@ -0,0 +1,42 @@
+/*
+* 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;
+
+/**
+ * Constants shared by the StAX-based AtomPub implementation.
+ */
+public final class AtomConstants {
+
+    private AtomConstants() {
+    }
+
+    /** Atom Syndication Format namespace (RFC 4287). */
+    public static final String ATOM_NS = "http://www.w3.org/2005/Atom";;
+
+    /** Atom Publishing Protocol namespace (RFC 5023). */
+    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";

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to