Discard request HTTP bodies before writing response
The HTTP RFCs require a server to fully consume the request body before
it can return a non-error status code.
Servlet API allows to use either InputStream or Reader. The code changes
switches the way to discard based on the request type.
There is a case that it discards the body even if it has an error status
code. The performance impact from this would be negligible because this
happens only when the REST API takes a raw input and it returns a
Response object with an error status code instead of throwing an
exception.
The same has been done in JGit in https://git.eclipse.org/r/#/c/4716/.
Change-Id: I3a40cbaf2c89e158451a7ed7c06fc98744d87f52
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index d9dd5d4..abf5323 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -119,6 +119,7 @@
import java.io.EOFException;
import java.io.FilterOutputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
@@ -144,6 +145,7 @@
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.http.server.ServletUtils;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.util.TemporaryBuffer;
import org.eclipse.jgit.util.TemporaryBuffer.Heap;
@@ -371,8 +373,10 @@
RestModifyView<RestResource, Object> m =
(RestModifyView<RestResource, Object>) viewData.view;
- inputRequestBody = parseRequest(req, inputType(m));
+ Type type = inputType(m);
+ inputRequestBody = parseRequest(req, type);
result = m.apply(rsrc, inputRequestBody);
+ consumeRawInputRequestBody(req, type);
} else {
throw new ResourceNotFoundException();
}
@@ -666,24 +670,30 @@
throws IOException, BadRequestException, SecurityException, IllegalArgumentException,
NoSuchMethodException, IllegalAccessException, InstantiationException,
InvocationTargetException, MethodNotAllowedException {
+ // HTTP/1.1 requires consuming the request body before writing non-error response (less than
+ // 400). Consume the request body for all but raw input request types here.
if (isType(JSON_TYPE, req.getContentType())) {
try (BufferedReader br = req.getReader();
JsonReader json = new JsonReader(br)) {
- json.setLenient(true);
-
- JsonToken first;
try {
- first = json.peek();
- } catch (EOFException e) {
- throw new BadRequestException("Expected JSON object");
+ json.setLenient(true);
+
+ JsonToken first;
+ try {
+ first = json.peek();
+ } catch (EOFException e) {
+ throw new BadRequestException("Expected JSON object");
+ }
+ if (first == JsonToken.STRING) {
+ return parseString(json.nextString(), type);
+ }
+ return OutputFormat.JSON.newGson().fromJson(json, type);
+ } finally {
+ // Reader.close won't consume the rest of the input. Explicitly consume the request body.
+ br.skip(Long.MAX_VALUE);
}
- if (first == JsonToken.STRING) {
- return parseString(json.nextString(), type);
- }
- return OutputFormat.JSON.newGson().fromJson(json, type);
}
- } else if (("PUT".equals(req.getMethod()) || "POST".equals(req.getMethod()))
- && acceptsRawInput(type)) {
+ } else if (rawInputRequest(req, type)) {
return parseRawInput(req, type);
} else if ("DELETE".equals(req.getMethod()) && hasNoBody(req)) {
return null;
@@ -706,6 +716,19 @@
}
}
+ private void consumeRawInputRequestBody(HttpServletRequest req, Type type) throws IOException {
+ if (rawInputRequest(req, type)) {
+ try (InputStream is = req.getInputStream()) {
+ ServletUtils.consumeRequestBody(is);
+ }
+ }
+ }
+
+ private static boolean rawInputRequest(HttpServletRequest req, Type type) {
+ String method = req.getMethod();
+ return ("PUT".equals(method) || "POST".equals(method)) && acceptsRawInput(type);
+ }
+
private static boolean hasNoBody(HttpServletRequest req) {
int len = req.getContentLength();
String type = req.getContentType();
