Check for and disallow pushing of invalid refs/meta/config

If the project.config or groups files are somehow invalid on
the refs/meta/config branch, or would be made invalid due to
a bad code review being submitted to this branch, reject the
user's attempt to push.

Bug: issue 1002
Change-Id: I9fafd651d875f50f501bd2a113441c2a47b3c0fa

gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java

@@ -71,6 +71,7 @@ public class ProjectConfig extends VersionedMetaData {
   private Project project;
   private Map<AccountGroup.UUID, GroupReference> groupsByUUID;
   private Map<String, AccessSection> accessSections;
+  private List<ValidationError> validationErrors;
 
   public static ProjectConfig read(MetaDataUpdate update) throws IOException,
       ConfigInvalidException {
@@ -165,6 +166,19 @@ public class ProjectConfig extends VersionedMetaData {
     return dirty;
   }
 
+  /**
+   * Get the validation errors, if any were discovered during load.
+   *
+   * @return list of errors; empty list if there are no errors.
+   */
+  public List<ValidationError> getValidationErrors() {
+    if (validationErrors != null) {
+      return Collections.unmodifiableList(validationErrors);
+    } else {
+      return Collections.emptyList();
+    }
+  }
+
   @Override
   protected String getRefName() {
     return GitRepositoryManager.REF_CONFIG;
@@ -172,7 +186,7 @@ public class ProjectConfig extends VersionedMetaData {
 
   @Override
   protected void onLoad() throws IOException, ConfigInvalidException {
-    Map<String,GroupReference> groupsByName = readGroupList();
+    Map<String, GroupReference> groupsByName = readGroupList();
 
     Config rc = readConfig(PROJECT_CONFIG);
     project = new Project(projectName);
@@ -184,12 +198,12 @@ public class ProjectConfig extends VersionedMetaData {
     }
     p.setParentName(rc.getString(ACCESS, null, KEY_INHERIT_FROM));
 
-    p.setUseContributorAgreements(rc.getBoolean(RECEIVE, KEY_REQUIRE_CONTRIBUTOR_AGREEMENT, false));
+    p.setUseContributorAgreements(getBoolean(rc, RECEIVE, KEY_REQUIRE_CONTRIBUTOR_AGREEMENT, false));
-    p.setUseSignedOffBy(rc.getBoolean(RECEIVE, KEY_REQUIRE_SIGNED_OFF_BY, false));
+    p.setUseSignedOffBy(getBoolean(rc, RECEIVE, KEY_REQUIRE_SIGNED_OFF_BY, false));
-    p.setRequireChangeID(rc.getBoolean(RECEIVE, KEY_REQUIRE_CHANGE_ID, false));
+    p.setRequireChangeID(getBoolean(rc, RECEIVE, KEY_REQUIRE_CHANGE_ID, false));
 
-    p.setSubmitType(rc.getEnum(SUBMIT, null, KEY_ACTION, defaultSubmitAction));
+    p.setSubmitType(getEnum(rc, SUBMIT, null, KEY_ACTION, defaultSubmitAction));
-    p.setUseContentMerge(rc.getBoolean(SUBMIT, null, KEY_MERGE_CONTENT, false));
+    p.setUseContentMerge(getBoolean(rc, SUBMIT, KEY_MERGE_CONTENT, false));
 
     accessSections = new HashMap<String, AccessSection>();
     for (String refName : rc.getSubsections(ACCESS)) {
@@ -214,9 +228,10 @@ public class ProjectConfig extends VersionedMetaData {
               try {
                 rule = PermissionRule.fromString(ruleString, useRange);
               } catch (IllegalArgumentException notRule) {
-                throw new ConfigInvalidException("Invalid rule in " + ACCESS
+                error(new ValidationError(PROJECT_CONFIG, "Invalid rule in " + ACCESS
                     + "." + refName + "." + varName + ": "
-                    + notRule.getMessage(), notRule);
+                    + notRule.getMessage()));
+                continue;
               }
 
               GroupReference ref = groupsByName.get(rule.getGroup().getName());
@@ -227,6 +242,8 @@ public class ProjectConfig extends VersionedMetaData {
                 //
                 ref = rule.getGroup();
                 groupsByName.put(ref.getName(), ref);
+                error(new ValidationError(PROJECT_CONFIG, "group \""
+                    + rule.getGroup().getName() + "\" not in " + GROUP_LIST));
               }
 
               rule.setGroup(ref);
@@ -238,22 +255,22 @@ public class ProjectConfig extends VersionedMetaData {
     }
   }
 
-  private Map<String, GroupReference> readGroupList() throws IOException,
+  private Map<String, GroupReference> readGroupList() throws IOException {
-      ConfigInvalidException {
     groupsByUUID = new HashMap<AccountGroup.UUID, GroupReference>();
     Map<String, GroupReference> groupsByName =
         new HashMap<String, GroupReference>();
 
     BufferedReader br = new BufferedReader(new StringReader(readUTF8(GROUP_LIST)));
     String s;
-    while ((s = br.readLine()) != null) {
+    for (int lineNumber = 1; (s = br.readLine()) != null; lineNumber++) {
       if (s.isEmpty() || s.startsWith("#")) {
         continue;
       }
 
       int tab = s.indexOf('\t');
       if (tab < 0) {
-        throw new ConfigInvalidException("Invalid group line: " + s);
+        error(new ValidationError(GROUP_LIST, lineNumber, "missing tab delimiter"));
+        continue;
       }
 
       AccountGroup.UUID uuid = new AccountGroup.UUID(s.substring(0, tab).trim());
@@ -370,6 +387,33 @@ public class ProjectConfig extends VersionedMetaData {
     saveUTF8(GROUP_LIST, buf.toString());
   }
 
+  private boolean getBoolean(Config rc, String section, String name,
+      boolean defaultValue) {
+    try {
+      return rc.getBoolean(section, name, defaultValue);
+    } catch (IllegalArgumentException err) {
+      error(new ValidationError(PROJECT_CONFIG, err.getMessage()));
+      return defaultValue;
+    }
+  }
+
+  private <E extends Enum<?>> E getEnum(Config rc, String section,
+      String subsection, String name, E defaultValue) {
+    try {
+      return rc.getEnum(section, subsection, name, defaultValue);
+    } catch (IllegalArgumentException err) {
+      error(new ValidationError(PROJECT_CONFIG, err.getMessage()));
+      return defaultValue;
+    }
+  }
+
+  private void error(ValidationError error) {
+    if (validationErrors == null) {
+      validationErrors = new ArrayList<ValidationError>(4);
+    }
+    validationErrors.add(error);
+  }
+
   private static String pad(int len, String src) {
     if (len <= src.length()) {
       return src;

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -615,6 +615,17 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
             try {
               ProjectConfig cfg = new ProjectConfig(project.getNameKey());
               cfg.load(repo, cmd.getNewId());
+              if (!cfg.getValidationErrors().isEmpty()) {
+                rp.sendError("Invalid project configuration:");
+                for (ValidationError err : cfg.getValidationErrors()) {
+                  rp.sendError("  " + err.getMessage());
+                }
+                reject(cmd, "invalid project configuration");
+                log.error("User " + currentUser.getUserName()
+                    + " tried to push invalid project configuration "
+                    + cmd.getNewId().name() + " for " + project.getName());
+                continue;
+              }
             } catch (Exception e) {
               reject(cmd, "invalid project configuration");
               log.error("User " + currentUser.getUserName()
@@ -1693,10 +1704,35 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 
     // Check for banned commits to prevent them from entering the tree again.
     if (rejectCommits.contains(c)) {
-      reject(newChange, "contains banned commit " + c.getName());
+      reject(cmd, "contains banned commit " + c.getName());
       return false;
     }
 
+    // If this is the special project configuration branch, validate the config.
+    if (GitRepositoryManager.REF_CONFIG.equals(ctl.getRefName())) {
+      try {
+        ProjectConfig cfg = new ProjectConfig(project.getNameKey());
+        cfg.load(repo, cmd.getNewId());
+        if (!cfg.getValidationErrors().isEmpty()) {
+          rp.sendError("Invalid project configuration:");
+          for (ValidationError err : cfg.getValidationErrors()) {
+            rp.sendError("  " + err.getMessage());
+          }
+          reject(cmd, "invalid project configuration");
+          log.error("User " + currentUser.getUserName()
+              + " tried to push invalid project configuration "
+              + cmd.getNewId().name() + " for " + project.getName());
+          return false;
+        }
+      } catch (Exception e) {
+        reject(cmd, "invalid project configuration");
+        log.error("User " + currentUser.getUserName()
+            + " tried to push invalid project configuration "
+            + cmd.getNewId().name() + " for " + project.getName(), e);
+        return false;
+      }
+    }
+
     return true;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/ValidationError.java

@@ -0,0 +1,41 @@
+// Copyright (C) 2011 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+/** Indicates a problem with Git based data. */
+public class ValidationError {
+  private final String message;
+
+  public ValidationError(String file, String message) {
+    this(file + ": " + message);
+  }
+
+  public ValidationError(String file, int line, String message) {
+    this(file + ":" + line + ": " + message);
+  }
+
+  public ValidationError(String message) {
+    this.message = message;
+  }
+
+  public String getMessage() {
+    return message;
+  }
+
+  @Override
+  public String toString() {
+    return "ValidationError[" + message + "]";
+  }
+}