Handle exceptions when sending error message in servlet
According to Sonarqube, even though the signatures for methods in a
servlet include throws IOException, ServletException, it's a bad idea to
let such exceptions be thrown. Failure to catch exceptions in a servlet
could leave a system in a vulnerable state, possibly resulting in
denial-of-service attacks, or the exposure of sensitive information
because when a servlet throws an exception, the servlet container
typically sends debugging information back to the user.
Change-Id: Id163a403daafb50e89920c918d815bd2c387a40e
diff --git a/src/main/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServlet.java
index d83579b..2aece3d 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServlet.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServlet.java
@@ -66,11 +66,11 @@
dispatcher.postEvent(event);
rsp.setStatus(SC_NO_CONTENT);
} catch (OrmException e) {
- rsp.sendError(SC_NOT_FOUND, "Change not found\n");
logger.debug("Error trying to find a change ", e);
+ sendError(rsp, SC_NOT_FOUND, "Change not found\n");
} catch (IOException e) {
- rsp.sendError(SC_BAD_REQUEST, e.getMessage());
logger.error("Unable to re-trigger event", e);
+ sendError(rsp, SC_BAD_REQUEST, e.getMessage());
} finally {
Context.unsetForwardedEvent();
}
@@ -87,4 +87,13 @@
}
return null;
}
+
+ private static void sendError(HttpServletResponse rsp, int statusCode,
+ String message) {
+ try {
+ rsp.sendError(statusCode, message);
+ } catch (IOException e) {
+ logger.error("Failed to send error messsage: " + e.getMessage(), e);
+ }
+ }
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServletTest.java b/src/test/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServletTest.java
index ecf73b9..8f417b3 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServletTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/syncevents/SyncEventsRestApiServletTest.java
@@ -14,6 +14,7 @@
package com.ericsson.gerrit.plugins.syncevents;
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
import static org.easymock.EasyMock.expect;
@@ -103,6 +104,16 @@
verifyAll();
}
+ @Test
+ public void testDoPostErrorWhileSendingErrorMessage() throws Exception {
+ expect(req.getReader()).andThrow(new IOException("someError"));
+ rsp.sendError(SC_BAD_REQUEST, "someError");
+ expectLastCall().andThrow(new IOException("someOtherError"));
+ replayAll();
+ syncEventsRestApiServlet.doPost(req, rsp);
+ verifyAll();
+ }
+
static class RefReplicationDoneEvent extends RefEvent {
public static final String TYPE = "ref-replication-done";
public final String project;
