opendev/gerrit
Code Issues Proposed changes

Don't fail with ISE on creating an access change with an invalid ref

Browse Source 
If an invalid ref is provided in the user input we should rather fail
with '400 Bad Request' and provide an error message to the user.

Error in PUT /projects/All-Projects/access:review
com.google.gerrit.common.errors.InvalidNameException: Invalid Name: refs/heads/stable_*
  at com.google.gerrit.server.project.RefPattern.validate(RefPattern.java:93)
  at com.google.gerrit.server.restapi.project.SetAccessUtil.validateChanges(SetAccessUtil.java:152)
  at com.google.gerrit.server.restapi.project.CreateAccessChange.apply(CreateAccessChange.java:124)
  at com.google.gerrit.server.restapi.project.CreateAccessChange.apply(CreateAccessChange.java:60)
  at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:418)
  at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
  ...

Change-Id: I40053f0736f3f43281059da40f27d550eec68626
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-06-28 08:43:26 +02:00
committed by David Pursehouse
parent 29e0183333
commit 442689b8c4
2 changed files with 31 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java
View File

@@ -524,6 +524,34 @@ public class AccessIT extends AbstractDaemonTest {
assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers); assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers);
} }
@Test
public void addAccessSectionForInvalidRef() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
// 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
String invalidRef = Constants.R_HEADS + "stable_*";
accessInput.add.put(invalidRef, accessSectionInfo);
exception.expect(BadRequestException.class);
exception.expectMessage("Invalid Name: " + invalidRef);
pApi.access(accessInput);
}
@Test
public void createAccessChangeWithAccessSectionForInvalidRef() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
// 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
String invalidRef = Constants.R_HEADS + "stable_*";
accessInput.add.put(invalidRef, accessSectionInfo);
exception.expect(BadRequestException.class);
exception.expectMessage("Invalid Name: " + invalidRef);
pApi.accessChange(accessInput);
}
private ProjectAccessInput newProjectAccessInput() { private ProjectAccessInput newProjectAccessInput() {
ProjectAccessInput p = new ProjectAccessInput(); ProjectAccessInput p = new ProjectAccessInput();
p.add = new HashMap<>(); p.add = new HashMap<>();

3
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateAccessChange.java
View File

@@ -22,6 +22,7 @@ import com.google.gerrit.common.errors.PermissionDeniedException;
import com.google.gerrit.extensions.api.access.ProjectAccessInput; import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -145,6 +146,8 @@ public class CreateAccessChange implements RestModifyView<ProjectResource, Proje
bu.execute(); bu.execute();
return Response.created(jsonFactory.noOptions().format(ins.getChange())); return Response.created(jsonFactory.noOptions().format(ins.getChange()));
} }
} catch (InvalidNameException e) {
throw new BadRequestException(e.toString());
} }
} }