Merge "ChangeEditModifier: Reject invalid file paths as '400 Bad Request'"
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index c05a47d..128388d 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -53,6 +53,7 @@
import java.util.List;
import java.util.Optional;
import java.util.TimeZone;
+import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -250,13 +251,14 @@
* @param filePath the path of the file whose contents should be modified
* @param newContent the new file content
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file already had the specified content
* @throws PermissionBackendException
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void modifyFile(
Repository repository, ChangeNotes notes, String filePath, RawInput newContent)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new ChangeFileContentModification(filePath, newContent));
}
@@ -269,12 +271,13 @@
* @param notes the {@link ChangeNotes} of the change whose change edit should be modified
* @param file path of the file which should be deleted
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file does not exist
* @throws PermissionBackendException
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void deleteFile(Repository repository, ChangeNotes notes, String file)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new DeleteFileModification(file));
}
@@ -288,6 +291,7 @@
* @param currentFilePath the current path/name of the file
* @param newFilePath the desired path/name of the file
* @throws AuthException if the user isn't authenticated or not allowed to use change edits
+ * @throws BadRequestException if the user provided bad input (e.g. invalid file paths)
* @throws InvalidChangeOperationException if the file was already renamed to the specified new
* name
* @throws PermissionBackendException
@@ -295,7 +299,7 @@
*/
public void renameFile(
Repository repository, ChangeNotes notes, String currentFilePath, String newFilePath)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new RenameFileModification(currentFilePath, newFilePath));
}
@@ -313,14 +317,14 @@
* @throws PermissionBackendException
*/
public void restoreFile(Repository repository, ChangeNotes notes, String file)
- throws AuthException, InvalidChangeOperationException, IOException,
+ throws AuthException, BadRequestException, InvalidChangeOperationException, IOException,
PermissionBackendException, ResourceConflictException {
modifyTree(repository, notes, new RestoreFileModification(file));
}
private void modifyTree(
Repository repository, ChangeNotes notes, TreeModification treeModification)
- throws AuthException, IOException, InvalidChangeOperationException,
+ throws AuthException, BadRequestException, IOException, InvalidChangeOperationException,
PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
@@ -370,8 +374,8 @@
ChangeNotes notes,
PatchSet patchSet,
List<TreeModification> treeModifications)
- throws AuthException, IOException, InvalidChangeOperationException, MergeConflictException,
- PermissionBackendException, ResourceConflictException {
+ throws AuthException, BadRequestException, IOException, InvalidChangeOperationException,
+ MergeConflictException, PermissionBackendException, ResourceConflictException {
assertCanEdit(notes);
Optional<ChangeEdit> optionalChangeEdit = lookupChangeEdit(notes);
@@ -486,10 +490,15 @@
private static ObjectId createNewTree(
Repository repository, RevCommit baseCommit, List<TreeModification> treeModifications)
- throws IOException, InvalidChangeOperationException {
- TreeCreator treeCreator = new TreeCreator(baseCommit);
- treeCreator.addTreeModifications(treeModifications);
- ObjectId newTreeId = treeCreator.createNewTreeAndGetId(repository);
+ throws BadRequestException, IOException, InvalidChangeOperationException {
+ ObjectId newTreeId;
+ try {
+ TreeCreator treeCreator = new TreeCreator(baseCommit);
+ treeCreator.addTreeModifications(treeModifications);
+ newTreeId = treeCreator.createNewTreeAndGetId(repository);
+ } catch (InvalidPathException e) {
+ throw new BadRequestException(e.getMessage());
+ }
if (ObjectId.isEqual(newTreeId, baseCommit.getTree())) {
throw new InvalidChangeOperationException("no changes were made");
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index 5adb75a..cbc1b79 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -138,7 +138,8 @@
@Override
public Response<?> apply(ChangeResource rsrc, IdString id, Input in)
- throws IOException, AuthException, ResourceConflictException, PermissionBackendException {
+ throws IOException, AuthException, BadRequestException, ResourceConflictException,
+ PermissionBackendException {
return deleteContent.apply(rsrc, id.get());
}
}
@@ -240,7 +241,8 @@
@Override
public Response<?> apply(ChangeResource resource, Post.Input input)
- throws AuthException, IOException, ResourceConflictException, PermissionBackendException {
+ throws AuthException, BadRequestException, IOException, ResourceConflictException,
+ PermissionBackendException {
Project.NameKey project = resource.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
if (isRestoreFile(input)) {
@@ -326,12 +328,14 @@
@Override
public Response<?> apply(ChangeEditResource rsrc, Input input)
- throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ throws AuthException, BadRequestException, ResourceConflictException, IOException,
+ PermissionBackendException {
return apply(rsrc.getChangeResource(), rsrc.getPath());
}
public Response<?> apply(ChangeResource rsrc, String filePath)
- throws AuthException, IOException, ResourceConflictException, PermissionBackendException {
+ throws AuthException, BadRequestException, IOException, ResourceConflictException,
+ PermissionBackendException {
try (Repository repository = repositoryManager.openRepository(rsrc.getProject())) {
editModifier.deleteFile(repository, rsrc.getNotes(), filePath);
} catch (InvalidChangeOperationException e) {
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index b0f183e..2883d8c 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -59,6 +59,7 @@
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -436,6 +437,16 @@
}
@Test
+ public void renameExistingFileToInvalidPath() throws Exception {
+ createEmptyEditFor(changeId);
+ BadRequestException badRequest =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId).edit().renameFile(FILE_NAME, "invalid/path/"));
+ assertThat(badRequest.getMessage()).isEqualTo("Invalid path: invalid/path/");
+ }
+
+ @Test
public void createEditByDeletingExistingFileRest() throws Exception {
adminRestSession.delete(urlEditFile(changeId, FILE_NAME)).assertNoContent();
assertThat(getFileContentOfEdit(changeId, FILE_NAME)).isAbsent();
