From 74b740412b5d2bb75ee0d5341fc401c8dba1413f Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Wed, 11 Nov 2020 10:15:56 +0100 Subject: [PATCH] auth-check: Set content length only if authorization is valid 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 --- .../com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java | 2 +- .../gerrit/acceptance/rest/auth/AuthenticationCheckIT.java | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java index d92da180f4..e3e96df9aa 100644 --- a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java +++ b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java @@ -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); diff --git a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java index 9298b439e6..00b1c5502f 100644 --- a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java @@ -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"); } }