From 9f50e113af21595904c5648486b3fb5a9b9f3bad Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrn@google.com>
Date: Thu, 22 Sep 2016 10:39:40 -0700
Subject: [PATCH] XsrfCookieFilter: handle null XGerritAuth

Gerrit's XSRF token handling sometimes allows a null token and
sometimes doesn't.  In particular, it is able to gracefully omit
X-Gerrit-Auth from the header of HTTP responses when xGerritAuth is
null but when trying to set a cookie it throws NullPointerException
and produces a 500 response.

Treat null as a request to clear the cookie instead.

Change-Id: I229f38ad8acdb4f409e06180ab320767d643126f
---
 .../src/main/java/com/google/gerrit/client/Gerrit.java    | 2 ++
 .../com/google/gerrit/httpd/CacheBasedWebSession.java     | 2 ++
 .../src/main/java/com/google/gerrit/httpd/WebSession.java | 3 ++-
 .../java/com/google/gerrit/httpd/XsrfCookieFilter.java    | 8 +++++---
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
index 93246cb1d8..d52d192e96 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
@@ -44,6 +44,7 @@ import com.google.gerrit.client.ui.LinkMenuItem;
 import com.google.gerrit.client.ui.MorphingTabPanel;
 import com.google.gerrit.client.ui.ProjectLinkMenuItem;
 import com.google.gerrit.client.ui.Screen;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.data.HostPageData;
 import com.google.gerrit.common.data.SystemInfoService;
@@ -287,6 +288,7 @@ public class Gerrit implements EntryPoint {
   }
 
   /** @return access token to prove user identity during REST API calls. */
+  @Nullable
   public static String getXGerritAuth() {
     return xGerritAuth;
   }
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index fa2e0e3e27..628edc2ba4 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -18,6 +18,7 @@ import static java.util.concurrent.TimeUnit.HOURS;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.common.data.HostPageData;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.httpd.WebSessionManager.Key;
 import com.google.gerrit.httpd.WebSessionManager.Val;
 import com.google.gerrit.reviewdb.client.Account;
@@ -109,6 +110,7 @@ public abstract class CacheBasedWebSession implements WebSession {
   }
 
   @Override
+  @Nullable
   public String getXGerritAuth() {
     return isSignedIn() ? val.getAuth() : null;
   }
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
index 327aaa389f..cddd04f5b8 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.httpd;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountExternalId;
 import com.google.gerrit.server.AccessPath;
@@ -22,7 +23,7 @@ import com.google.gerrit.server.account.AuthResult;
 
 public interface WebSession {
   boolean isSignedIn();
-  String getXGerritAuth();
+  @Nullable String getXGerritAuth();
   boolean isValidXGerritAuth(String keyIn);
   AccountExternalId.Key getLastLoginExternalId();
   CurrentUser getUser();
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/XsrfCookieFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/XsrfCookieFilter.java
index 842b2b40a6..0c2565c239 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/XsrfCookieFilter.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/XsrfCookieFilter.java
@@ -14,6 +14,8 @@
 
 package com.google.gerrit.httpd;
 
+import static com.google.common.base.Strings.nullToEmpty;
+
 import com.google.gerrit.common.data.HostPageData;
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.server.CurrentUser;
@@ -61,11 +63,11 @@ public class XsrfCookieFilter implements Filter {
 
   private void setXsrfTokenCookie(HttpServletRequest req,
       HttpServletResponse rsp, WebSession session) {
-    String v = session != null ? session.getXGerritAuth() : "";
-    Cookie c = new Cookie(HostPageData.XSRF_COOKIE_NAME, v);
+    String v = session != null ? session.getXGerritAuth() : null;
+    Cookie c = new Cookie(HostPageData.XSRF_COOKIE_NAME, nullToEmpty(v));
     c.setPath("/");
     c.setSecure(authConfig.getCookieSecure() && isSecure(req));
-    c.setMaxAge(session != null
+    c.setMaxAge(v != null
         ? -1 // Set the cookie for this browser session.
         : 0); // Remove the cookie (expire immediately).
     rsp.addCookie(c);