opendev/gerrit
Code Issues Proposed changes

auth-check: Set content length only if authorization is valid

Browse Source 
The HTTP Content-Length header is optional. We set it in the
auth-check servlet, so that clients can close the connection
early in case there is no content.

For HTTP FORBIDDEN, the content length seems to be non-zero.
A discrepancy between the content length header and the
actual content length triggers a warning in some HTTP servers.

This commit sets the content length only in the NO_CONTENT
case.

Change-Id: I5ac27bbf77c964e998c7dc2bfba9e912a9c26ccb
This commit is contained in:
Patrick Hiesel
2020-11-11 10:15:56 +01:00
parent 5626c6ca1b
commit 74b740412b
2 changed files with 1 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
View File

@@ -43,8 +43,8 @@ public class AuthorizationCheckServlet extends HttpServlet {
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException {
CacheHeaders.setNotCacheable(res);
res.setContentLength(0);
if (user.get().isIdentifiedUser()) {
res.setContentLength(0);
res.setStatus(HttpServletResponse.SC_NO_CONTENT);
} else {
res.setStatus(HttpServletResponse.SC_FORBIDDEN);

3
javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
View File

@@ -14,8 +14,6 @@
package com.google.gerrit.acceptance.rest.auth;
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession;
@@ -34,6 +32,5 @@ public class AuthenticationCheckIT extends AbstractDaemonTest {
RestSession anonymous = new RestSession(server, null);
RestResponse r = anonymous.get("/auth-check");
r.assertForbidden();
assertThat(r.getHeader("Content-Length")).isEqualTo("0");
}
}