Validation on Invalid Filter Expression in User-Set Notifications

Currently, if a user sets a notification with an invalid query as
the filter, gerrit accepts the filter and fails on each query parse
attempt, resulting in many error messages to the error_log. With
this change, the user instead receives a 400 'invalid query' message
when they attempt to save the notification setting, and the invalid
query is not saved.

Change-Id: I74ea902902956d4b039e67e54930c35eb413d568
Release-Notes: Invalid filter expressions for notifications are rejected

3 files changed, 17 insertions, 0 deletions

diff --git a/antlr3/BUILD b/antlr3/BUILD
index 549946a2a0..23641e3e2b 100644
--- a/antlr3/BUILD
+++ b/antlr3/BUILD
@@ -22,6 +22,7 @@ java_library(
     srcs = [":query"],
     visibility = [
         "//java/com/google/gerrit/index:__subpackages__",
+        "//java/com/google/gerrit/server:__subpackages__",
         "//javatests/com/google/gerrit:__subpackages__",
         "//javatests/com/google/gerrit/index:__pkg__",
         "//plugins:__pkg__",
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 3f28a03de1..4c07d8aca3 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -8,6 +8,7 @@ java_library(
     name = "restapi",
     srcs = glob(["**/*.java"]),
     deps = [
+        "//antlr3:query_parser",
         "//java/com/google/gerrit/common:annotations",
         "//java/com/google/gerrit/common:server",
         "//java/com/google/gerrit/entities",
@@ -31,6 +32,7 @@ java_library(
         "//lib:guava",
         "//lib:jgit",
         "//lib:servlet-api",
+        "//lib/antlr:java-runtime",
         "//lib/auto:auto-value",
         "//lib/auto:auto-value-annotations",
         "//lib/commons:compress",
diff --git a/java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java b/java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java
index c80bf57fa3..17e31bdb20 100644
--- a/java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java
+++ b/java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java
@@ -14,12 +14,15 @@
 
 package com.google.gerrit.server.restapi.account;
 
+import com.google.common.base.Strings;
 import com.google.gerrit.entities.NotifyConfig.NotifyType;
 import com.google.gerrit.extensions.client.ProjectWatchInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.index.query.QueryParser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.AccountResource;
@@ -95,6 +98,17 @@ public class PostWatchedProjects
         throw new BadRequestException("project name must be specified");
       }
 
+      if (!Strings.isNullOrEmpty(info.filter)) {
+        try {
+          QueryParser.parse(info.filter);
+        } catch (QueryParseException e) {
+          throw new BadRequestException(
+              String.format(
+                  "invalid filter expression for project %s: %s", info.project, e.getMessage()),
+              e);
+        }
+      }
+
       ProjectWatchKey key =
           ProjectWatchKey.create(projectsCollection.parse(info.project).getNameKey(), info.filter);
       if (m.containsKey(key)) {