From 10d45357a082d0e44f33cd9262fa1d19b511a35f Mon Sep 17 00:00:00 2001
From: Edwin Kempin <ekempin@google.com>
Date: Fri, 29 Dec 2017 12:58:09 +0100
Subject: [PATCH] SetPreferences: Reject invalid my menu items

Name and URL are required fields for menu items. Fail with 400 Bad
Request if they are missing.

Change-Id: If814919d6b6c4c78c01f60010ad991bd0f22ad3f
Signed-off-by: Edwin Kempin <ekempin@google.com>
---
 .../api/accounts/GeneralPreferencesIT.java    | 23 +++++++++++++++++++
 .../gerrit/server/account/SetPreferences.java | 15 ++++++++++--
 .../gerrit/server/config/SetPreferences.java  |  2 +-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index ee2f5d9a75..1cfd8bf8ba 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -31,6 +31,7 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.ReviewCategoryStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
 import com.google.gerrit.extensions.client.MenuItem;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.inject.Inject;
@@ -168,4 +169,26 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
     assertThat(a.changesPerPage).isEqualTo(d.changesPerPage);
     assertPrefs(a, d, "my", "changeTable", "changesPerPage");
   }
+
+  @Test
+  public void rejectMyMenuWithoutName() throws Exception {
+    GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults();
+    i.my = new ArrayList<>();
+    i.my.add(new MenuItem(null, "url"));
+
+    exception.expect(BadRequestException.class);
+    exception.expectMessage("name for menu item is required");
+    gApi.accounts().id(user42.getId().toString()).setPreferences(i);
+  }
+
+  @Test
+  public void rejectMyMenuWithoutUrl() throws Exception {
+    GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults();
+    i.my = new ArrayList<>();
+    i.my.add(new MenuItem("name", null));
+
+    exception.expect(BadRequestException.class);
+    exception.expectMessage("URL for menu item is required");
+    gApi.accounts().id(user42.getId().toString()).setPreferences(i);
+  }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
index c033d9dd44..506d710dec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
@@ -100,7 +100,7 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
   }
 
   private void writeToGit(Account.Id id, GeneralPreferencesInfo i)
-      throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+      throws RepositoryNotFoundException, IOException, ConfigInvalidException, BadRequestException {
     VersionedAccountPreferences prefs;
     try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
       prefs = VersionedAccountPreferences.forUser(id);
@@ -121,11 +121,15 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
     }
   }
 
-  public static void storeMyMenus(VersionedAccountPreferences prefs, List<MenuItem> my) {
+  public static void storeMyMenus(VersionedAccountPreferences prefs, List<MenuItem> my)
+      throws BadRequestException {
     Config cfg = prefs.getConfig();
     if (my != null) {
       unsetSection(cfg, UserConfigSections.MY);
       for (MenuItem item : my) {
+        checkRequiredMenuItemField(item.name, "name");
+        checkRequiredMenuItemField(item.url, "URL");
+
         set(cfg, item.name, KEY_URL, item.url);
         set(cfg, item.name, KEY_TARGET, item.target);
         set(cfg, item.name, KEY_ID, item.id);
@@ -174,6 +178,13 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
     }
   }
 
+  private static void checkRequiredMenuItemField(String value, String name)
+      throws BadRequestException {
+    if (Strings.isNullOrEmpty(value)) {
+      throw new BadRequestException(name + " for menu item is required");
+    }
+  }
+
   private void checkDownloadScheme(String downloadScheme) throws BadRequestException {
     if (Strings.isNullOrEmpty(downloadScheme)) {
       return;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
index 55337d574c..cc96cf0901 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
@@ -75,7 +75,7 @@ public class SetPreferences implements RestModifyView<ConfigResource, GeneralPre
   }
 
   private GeneralPreferencesInfo writeToGit(GeneralPreferencesInfo i)
-      throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+      throws RepositoryNotFoundException, IOException, ConfigInvalidException, BadRequestException {
     try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
       VersionedAccountPreferences p = VersionedAccountPreferences.forDefault();
       p.load(md);