Revert "Fail 'Get Diff' requests for file sizes that exceed 50Mb"
This reverts commit 7653854d27087b49787a7c31b5cc4cffb5eeaf8a.
Reason for revert: Breaks callers, so we need to revert in the interim
Change-Id: I74fbe8589692e0e961264bfea121efd58e59cff2
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index a6bbeab..e0a4269 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -175,9 +175,7 @@
import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs;
import com.google.gerrit.server.notedb.NoteDbModule;
import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
-import com.google.gerrit.server.patch.DiffFileSizeValidator;
import com.google.gerrit.server.patch.DiffOperationsImpl;
-import com.google.gerrit.server.patch.DiffValidator;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.patch.PatchScriptFactory;
import com.google.gerrit.server.patch.PatchScriptFactoryForAutoFix;
@@ -405,8 +403,6 @@
.to(SubmitRequirementConfigValidator.class);
DynamicSet.bind(binder(), CommitValidationListener.class).to(PrologRulesWarningValidator.class);
DynamicSet.setOf(binder(), CommentValidator.class);
- DynamicSet.setOf(binder(), DiffValidator.class);
- DynamicSet.bind(binder(), DiffValidator.class).to(DiffFileSizeValidator.class);
DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
DynamicSet.setOf(binder(), OnSubmitValidationListener.class);
diff --git a/java/com/google/gerrit/server/git/LargeObjectException.java b/java/com/google/gerrit/server/git/LargeObjectException.java
index 145b631..04db42c 100644
--- a/java/com/google/gerrit/server/git/LargeObjectException.java
+++ b/java/com/google/gerrit/server/git/LargeObjectException.java
@@ -25,10 +25,6 @@
private static final long serialVersionUID = 1L;
- public LargeObjectException(String message) {
- super(message);
- }
-
public LargeObjectException(String message, org.eclipse.jgit.errors.LargeObjectException cause) {
super(message, cause);
}
diff --git a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java b/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java
deleted file mode 100644
index 14a0f7b..0000000
--- a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION;
-import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BIGFILE_THRESHOLD;
-import static org.eclipse.jgit.storage.pack.PackConfig.DEFAULT_BIG_FILE_THRESHOLD;
-
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.inject.Inject;
-import org.eclipse.jgit.lib.Config;
-
-public class DiffFileSizeValidator implements DiffValidator {
- static final int DEFAULT_MAX_FILE_SIZE = DEFAULT_BIG_FILE_THRESHOLD;
- private static final String ERROR_MESSAGE =
- "File size for file %s exceeded the max file size threshold. Threshold = %d bytes, Actual size = %d bytes";
-
- final int maxFileSize;
-
- @Inject
- public DiffFileSizeValidator(@GerritServerConfig Config cfg) {
- this.maxFileSize =
- cfg.getInt(CONFIG_CORE_SECTION, CONFIG_KEY_BIGFILE_THRESHOLD, DEFAULT_MAX_FILE_SIZE);
- }
-
- @Override
- public void validate(FileDiffOutput fileDiff) throws LargeObjectException {
- if (fileDiff.size() > maxFileSize) {
- throw new LargeObjectException(
- String.format(ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.size()));
- }
- if (fileDiff.sizeDelta() > maxFileSize) {
- throw new LargeObjectException(
- String.format(
- ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.sizeDelta()));
- }
- }
-}
diff --git a/java/com/google/gerrit/server/patch/DiffValidator.java b/java/com/google/gerrit/server/patch/DiffValidator.java
deleted file mode 100644
index aee3c8b..0000000
--- a/java/com/google/gerrit/server/patch/DiffValidator.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-
-/** Interface to validate diff outputs. */
-@ExtensionPoint
-public interface DiffValidator {
- void validate(FileDiffOutput fileDiffOutput)
- throws LargeObjectException, DiffNotAvailableException;
-}
diff --git a/java/com/google/gerrit/server/patch/DiffValidators.java b/java/com/google/gerrit/server/patch/DiffValidators.java
deleted file mode 100644
index 964353d..0000000
--- a/java/com/google/gerrit/server/patch/DiffValidators.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.inject.Inject;
-
-/** Validates {@link FileDiffOutput}(s) after they are computed by the {@link DiffOperations}. */
-public class DiffValidators {
- DynamicSet<DiffValidator> diffValidators;
-
- @Inject
- public DiffValidators(DynamicSet<DiffValidator> diffValidators) {
- this.diffValidators = diffValidators;
- }
-
- public void validate(FileDiffOutput fileDiffOutput)
- throws LargeObjectException, DiffNotAvailableException {
- for (DiffValidator validator : diffValidators) {
- validator.validate(fileDiffOutput);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 5015c768..3baa3b1 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -94,7 +94,6 @@
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final DiffOperations diffOperations;
- private final DiffValidators diffValidators;
private final Change.Id changeId;
@@ -110,7 +109,6 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
- DiffValidators diffValidators,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted("patchSetA") @Nullable PatchSet.Id patchSetA,
@@ -126,7 +124,6 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.diffOperations = diffOperations;
- this.diffValidators = diffValidators;
this.fileName = fileName;
this.psa = patchSetA;
@@ -147,7 +144,6 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
DiffOperations diffOperations,
- DiffValidators diffValidators,
@Assisted ChangeNotes notes,
@Assisted String fileName,
@Assisted int parentNum,
@@ -163,7 +159,6 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.diffOperations = diffOperations;
- this.diffValidators = diffValidators;
this.fileName = fileName;
this.psa = null;
@@ -225,14 +220,13 @@
}
private PatchScript getPatchScript(Repository git, ObjectId aId, ObjectId bId)
- throws IOException, DiffNotAvailableException, LargeObjectException {
+ throws IOException, DiffNotAvailableException {
FileDiffOutput fileDiffOutput =
aId == null
? diffOperations.getModifiedFileAgainstParent(
notes.getProjectName(), bId, parentNum, fileName, diffPrefs.ignoreWhitespace)
: diffOperations.getModifiedFile(
notes.getProjectName(), aId, bId, fileName, diffPrefs.ignoreWhitespace);
- diffValidators.validate(fileDiffOutput);
return newBuilder().toPatchScript(git, fileDiffOutput);
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
index 9107dde..9286f47 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
@@ -64,10 +64,6 @@
*/
public abstract Optional<String> newPath();
- public String getDefaultPath() {
- return oldPath().isPresent() ? oldPath().get() : newPath().get();
- }
-
/**
* The file mode of the old file at the old git tree diff identified by {@link #oldCommitId()}
* ()}.
diff --git a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java b/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java
deleted file mode 100644
index fa1d09e..0000000
--- a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (C) 2023 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.patch;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.Patch.ChangeType;
-import com.google.gerrit.entities.Patch.FileMode;
-import com.google.gerrit.entities.Patch.PatchType;
-import com.google.gerrit.server.git.LargeObjectException;
-import com.google.gerrit.server.patch.filediff.FileDiffOutput;
-import com.google.gerrit.testing.InMemoryModule;
-import com.google.inject.Guice;
-import com.google.inject.Inject;
-import com.google.inject.Injector;
-import java.util.Optional;
-import org.eclipse.jgit.lib.ObjectId;
-import org.junit.Before;
-import org.junit.Test;
-
-/** Test class for {@link DiffValidators}. */
-public class DiffValidatorsTest {
- @Inject private DiffValidators diffValidators;
-
- @Before
- public void setUpInjector() throws Exception {
- Injector injector = Guice.createInjector(new InMemoryModule());
- injector.injectMembers(this);
- }
-
- @Test
- public void fileSizeExceeded() {
- int largeSize = 100000000;
- FileDiffOutput fileDiff =
- FileDiffOutput.builder()
- .oldCommitId(ObjectId.zeroId())
- .newCommitId(ObjectId.zeroId())
- .comparisonType(ComparisonType.againstRoot())
- .changeType(ChangeType.ADDED)
- .patchType(Optional.of(PatchType.UNIFIED))
- .oldPath(Optional.empty())
- .newPath(Optional.of("f.txt"))
- .oldMode(Optional.empty())
- .newMode(Optional.of(FileMode.REGULAR_FILE))
- .headerLines(ImmutableList.of())
- .edits(ImmutableList.of())
- .size(largeSize)
- .sizeDelta(largeSize)
- .build();
- Exception thrown =
- assertThrows(LargeObjectException.class, () -> diffValidators.validate(fileDiff));
- assertThat(thrown)
- .hasMessageThat()
- .isEqualTo(
- String.format(
- "File size for file f.txt exceeded the max file size threshold."
- + " Threshold = %d bytes, Actual size = %d bytes",
- DiffFileSizeValidator.DEFAULT_MAX_FILE_SIZE, largeSize));
- }
-}
