github-advanced-security[bot] commented on code in PR #161:
URL: https://github.com/apache/roller/pull/161#discussion_r4238512315
##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -63,102 +57,134 @@
private static final String ATOM_CONTENT_TYPE = "application/atom+xml";
- /**
- * Request attribute that carries the handler authenticated by this servlet
- * to {@link RollerAtomHandlerFactory}, so Propono does not authenticate
the
- * request a second time.
- */
- static final String HANDLER_ATTRIBUTE = RollerAtomServlet.class.getName()
+ ".handler";
-
@Override
- protected void service(HttpServletRequest req, HttpServletResponse res)
- throws ServletException, IOException {
+ protected void service(HttpServletRequest request, HttpServletResponse
response)
+ throws IOException {
if
(!WebloggerRuntimeConfig.getBooleanProperty("webservices.enableAtomPub")) {
- LOG.debug("AtomPub service is disabled; rejecting request");
- sendText(res, HttpServletResponse.SC_NOT_FOUND, "AtomPub service
is disabled");
+ log.debug("AtomPub service is disabled; rejecting request");
+ sendText(response, HttpServletResponse.SC_NOT_FOUND, "AtomPub
service is disabled");
return;
}
- if (!carriesEntry(req)) {
- forward(req, res);
+ String method = request.getMethod();
+ if (!"GET".equals(method) && !"POST".equals(method)
+ && !"PUT".equals(method) && !"DELETE".equals(method)) {
+ response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
return;
}
- // Authenticate before reading the body, as Propono does.
- AtomHandler handler = createHandler(req, res);
- if (handler.getAuthenticatedUsername() == null) {
- res.setHeader("WWW-Authenticate", "BASIC realm=\"AtomPub\"");
- res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ // Authenticate before reading the body.
+ RollerAtomHandler handler = createHandler(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);
+ }
return;
}
- req.setAttribute(HANDLER_ATTRIBUTE, handler);
- int maxEntryBytes = maxEntryBytes();
- // Read one byte past the limit, so an oversized body can be detected.
- byte[] body = req.getInputStream().readNBytes(maxEntryBytes + 1);
- if (body.length > maxEntryBytes) {
- sendText(res, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE,
"Entry is too large");
+ if ("POST".equals(method) && request.getContentType() == null) {
+ sendText(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE,
+ "No content-type specified in request");
return;
}
- DefaultHandler contentHandler = new DefaultHandler() {
- @Override
- public void error(SAXParseException e) throws SAXException {
- throw e;
+
+ AtomRequest areq;
+ AtomEntry entry = null;
+ if (carriesEntry(request)) {
+ int maxEntryBytes = maxEntryBytes();
+ // Read one byte past the limit, so an oversized body can be
detected.
+ byte[] body = request.getInputStream().readNBytes(maxEntryBytes +
1);
+ if (body.length > maxEntryBytes) {
+ sendText(response,
HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE,
+ "Entry is too large");
+ return;
+ }
+ try {
+ entry = new AtomReader().parseEntry(new
ByteArrayInputStream(body));
+ } catch (AtomException e) {
+ log.debug("Rejecting Atom entry that could not be parsed", e);
+ sendText(response, HttpServletResponse.SC_BAD_REQUEST,
"Invalid Atom entry");
+ return;
}
- };
- XMLReader reader;
+ areq = new AtomRequest(request, body);
+ } else {
+ // Media bodies are streamed to a temporary file by
MediaCollection,
+ // where the upload size and quota are checked.
+ areq = AtomRequest.streaming(request);
+ }
+
try {
- reader =
SecureXmlParsers.newSAXParserFactory().newSAXParser().getXMLReader();
- // Hardening: DOCTYPE declarations should be rejected,
- // regardless whether the secure reader already does it.
- DefaultHandler2 doctypeRefuser = new DefaultHandler2() {
- @Override
- public void startDTD(String name, String publicId, String
systemId)
- throws SAXException {
- throw new SAXException("DOCTYPE is not allowed in an Atom
entry");
+ switch (method) {
+ case "GET":
+ doGet(handler, areq, response);
+ break;
+ case "POST":
+ doPost(handler, areq, entry, response);
+ break;
+ case "PUT":
+ doPut(handler, areq, entry, response);
+ break;
+ default:
+ handler.deleteEntry(areq);
+ response.setStatus(HttpServletResponse.SC_OK);
+ }
+ } catch (AtomException ae) {
+ if (!response.isCommitted()) {
+ if (ae.getStatus() >=
HttpServletResponse.SC_INTERNAL_SERVER_ERROR) {
+ // Server errors can carry internal details; keep them in
the log
+ log.error("Error handling AtomPub request", ae);
+ response.sendError(ae.getStatus());
+ } else {
+ // Client errors carry a message written by Roller for the
client
+ log.debug("Returning error to client: " + ae.getMessage(),
ae);
+ response.sendError(ae.getStatus(), ae.getMessage());
Review Comment:
## CodeQL / Information exposure through an error message
[Error information](1) can be exposed to an external user.
[Show more
details](https://github.com/apache/roller/security/code-scanning/139)
##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -188,61 +214,94 @@
res.getWriter().write(message);
}
- /** A request whose body has already been read into memory. */
- static final class BufferedBodyRequest extends HttpServletRequestWrapper {
+ private void doGet(RollerAtomHandler handler, AtomRequest areq,
HttpServletResponse response)
+ throws AtomException, IOException {
- private final byte[] body;
+ if (handler.isAtomServiceURI(areq)) {
+ AtomServiceDoc service = handler.getAtomService(areq);
+ response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE);
+ new AtomWriter().writeServiceDoc(response.getOutputStream(),
service);
- BufferedBodyRequest(HttpServletRequest request, byte[] body) {
- super(request);
- this.body = body;
- }
+ } else if (handler.isCollectionURI(areq)) {
+ AtomFeed feed = handler.getCollection(areq);
+ response.setContentType(AtomConstants.FEED_MEDIA_TYPE);
+ new AtomWriter().writeFeed(response.getOutputStream(), feed);
- @Override
- public ServletInputStream getInputStream() {
- final ByteArrayInputStream in = new ByteArrayInputStream(body);
- return new ServletInputStream() {
- @Override
- public int read() {
- return in.read();
- }
+ } else if (handler.isEntryURI(areq)) {
+ AtomEntry entry = handler.getEntry(areq);
+ response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+ new AtomWriter().writeEntry(response.getOutputStream(), entry);
- @Override
- public int read(byte[] b, int off, int len) {
- return in.read(b, off, len);
- }
+ } 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());
+ }
- @Override
- public boolean isFinished() {
- return in.available() == 0;
- }
+ } else {
+ throw new AtomNotFoundException("Cannot find specified resource");
+ }
+ }
- @Override
- public boolean isReady() {
- return true;
- }
+ private void doPost(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public void setReadListener(ReadListener listener) {
- throw new UnsupportedOperationException();
- }
- };
+ if (!handler.isCollectionURI(areq)) {
+ throw new AtomNotFoundException("Cannot POST to specified URI");
}
- @Override
- public BufferedReader getReader() {
- return new BufferedReader(new InputStreamReader(
- new ByteArrayInputStream(body), StandardCharsets.UTF_8));
+ String contentType = areq.getContentType();
+ AtomEntry created;
+ if (entry != null) {
+ 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(handler, response, created);
+ }
+
+ private void doPut(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public int getContentLength() {
- return body.length;
+ if (entry != null) {
+ 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");
}
+ }
- @Override
- public long getContentLengthLong() {
- return body.length;
+ private void writeCreated(RollerAtomHandler handler, HttpServletResponse
response,
+ AtomEntry entry) throws AtomException {
+ String location = safeLocation(entry.getLinkHref("edit"),
handler.getAtomURL());
+ if (location != null) {
+ response.setHeader("Location", location);
Review Comment:
## CodeQL / URL redirection from remote source
Untrusted URL redirection depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/roller/security/code-scanning/140)
##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -188,61 +214,94 @@
res.getWriter().write(message);
}
- /** A request whose body has already been read into memory. */
- static final class BufferedBodyRequest extends HttpServletRequestWrapper {
+ private void doGet(RollerAtomHandler handler, AtomRequest areq,
HttpServletResponse response)
+ throws AtomException, IOException {
- private final byte[] body;
+ if (handler.isAtomServiceURI(areq)) {
+ AtomServiceDoc service = handler.getAtomService(areq);
+ response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE);
+ new AtomWriter().writeServiceDoc(response.getOutputStream(),
service);
- BufferedBodyRequest(HttpServletRequest request, byte[] body) {
- super(request);
- this.body = body;
- }
+ } else if (handler.isCollectionURI(areq)) {
+ AtomFeed feed = handler.getCollection(areq);
+ response.setContentType(AtomConstants.FEED_MEDIA_TYPE);
+ new AtomWriter().writeFeed(response.getOutputStream(), feed);
- @Override
- public ServletInputStream getInputStream() {
- final ByteArrayInputStream in = new ByteArrayInputStream(body);
- return new ServletInputStream() {
- @Override
- public int read() {
- return in.read();
- }
+ } else if (handler.isEntryURI(areq)) {
+ AtomEntry entry = handler.getEntry(areq);
+ response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+ new AtomWriter().writeEntry(response.getOutputStream(), entry);
- @Override
- public int read(byte[] b, int off, int len) {
- return in.read(b, off, len);
- }
+ } 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());
+ }
- @Override
- public boolean isFinished() {
- return in.available() == 0;
- }
+ } else {
+ throw new AtomNotFoundException("Cannot find specified resource");
+ }
+ }
- @Override
- public boolean isReady() {
- return true;
- }
+ private void doPost(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public void setReadListener(ReadListener listener) {
- throw new UnsupportedOperationException();
- }
- };
+ if (!handler.isCollectionURI(areq)) {
+ throw new AtomNotFoundException("Cannot POST to specified URI");
}
- @Override
- public BufferedReader getReader() {
- return new BufferedReader(new InputStreamReader(
- new ByteArrayInputStream(body), StandardCharsets.UTF_8));
+ String contentType = areq.getContentType();
+ AtomEntry created;
+ if (entry != null) {
+ 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(handler, response, created);
+ }
+
+ private void doPut(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public int getContentLength() {
- return body.length;
+ if (entry != null) {
+ 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");
}
+ }
- @Override
- public long getContentLengthLong() {
- return body.length;
+ private void writeCreated(RollerAtomHandler handler, HttpServletResponse
response,
+ AtomEntry entry) throws AtomException {
+ String location = safeLocation(entry.getLinkHref("edit"),
handler.getAtomURL());
+ if (location != null) {
+ response.setHeader("Location", location);
+ response.setHeader("Content-Location", location);
Review Comment:
## CodeQL / HTTP response splitting
This header depends on a [user-provided value](1), which may cause a
response-splitting vulnerability.
[Show more
details](https://github.com/apache/roller/security/code-scanning/142)
##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/RollerAtomServlet.java:
##########
@@ -188,61 +214,94 @@
res.getWriter().write(message);
}
- /** A request whose body has already been read into memory. */
- static final class BufferedBodyRequest extends HttpServletRequestWrapper {
+ private void doGet(RollerAtomHandler handler, AtomRequest areq,
HttpServletResponse response)
+ throws AtomException, IOException {
- private final byte[] body;
+ if (handler.isAtomServiceURI(areq)) {
+ AtomServiceDoc service = handler.getAtomService(areq);
+ response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE);
+ new AtomWriter().writeServiceDoc(response.getOutputStream(),
service);
- BufferedBodyRequest(HttpServletRequest request, byte[] body) {
- super(request);
- this.body = body;
- }
+ } else if (handler.isCollectionURI(areq)) {
+ AtomFeed feed = handler.getCollection(areq);
+ response.setContentType(AtomConstants.FEED_MEDIA_TYPE);
+ new AtomWriter().writeFeed(response.getOutputStream(), feed);
- @Override
- public ServletInputStream getInputStream() {
- final ByteArrayInputStream in = new ByteArrayInputStream(body);
- return new ServletInputStream() {
- @Override
- public int read() {
- return in.read();
- }
+ } else if (handler.isEntryURI(areq)) {
+ AtomEntry entry = handler.getEntry(areq);
+ response.setContentType(AtomConstants.ENTRY_MEDIA_TYPE);
+ new AtomWriter().writeEntry(response.getOutputStream(), entry);
- @Override
- public int read(byte[] b, int off, int len) {
- return in.read(b, off, len);
- }
+ } 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());
+ }
- @Override
- public boolean isFinished() {
- return in.available() == 0;
- }
+ } else {
+ throw new AtomNotFoundException("Cannot find specified resource");
+ }
+ }
- @Override
- public boolean isReady() {
- return true;
- }
+ private void doPost(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public void setReadListener(ReadListener listener) {
- throw new UnsupportedOperationException();
- }
- };
+ if (!handler.isCollectionURI(areq)) {
+ throw new AtomNotFoundException("Cannot POST to specified URI");
}
- @Override
- public BufferedReader getReader() {
- return new BufferedReader(new InputStreamReader(
- new ByteArrayInputStream(body), StandardCharsets.UTF_8));
+ String contentType = areq.getContentType();
+ AtomEntry created;
+ if (entry != null) {
+ 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(handler, response, created);
+ }
+
+ private void doPut(RollerAtomHandler handler, AtomRequest areq, AtomEntry
entry,
+ HttpServletResponse response) throws AtomException {
- @Override
- public int getContentLength() {
- return body.length;
+ if (entry != null) {
+ 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");
}
+ }
- @Override
- public long getContentLengthLong() {
- return body.length;
+ private void writeCreated(RollerAtomHandler handler, HttpServletResponse
response,
+ AtomEntry entry) throws AtomException {
+ String location = safeLocation(entry.getLinkHref("edit"),
handler.getAtomURL());
+ if (location != null) {
+ response.setHeader("Location", location);
Review Comment:
## CodeQL / HTTP response splitting
This header depends on a [user-provided value](1), which may cause a
response-splitting vulnerability.
[Show more
details](https://github.com/apache/roller/security/code-scanning/141)
--
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]
