diff --git a/CHANGELOG.md b/CHANGELOG.md index 54d93caf09..ec0bf37b40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Automatic user converter for external users ([#1380](https://github.com/scm-manager/scm-manager/pull/1380)) ### Fixed +- Internal server error for git sub modules without tree object ([#1397](https://github.com/scm-manager/scm-manager/pull/1397)) - Do not expose subversion commit with id 0 ([#1395](https://github.com/scm-manager/scm-manager/pull/1395)) ## [2.8.0] - 2020-10-27 diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java index 97115ca159..75b96d16f2 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java @@ -55,16 +55,18 @@ import sonia.scm.store.BlobStore; import sonia.scm.util.Util; import sonia.scm.web.lfs.LfsBlobStoreFactory; +import javax.annotation.Nullable; import javax.inject.Inject; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Deque; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Stack; import java.util.function.Consumer; import static java.util.Optional.empty; @@ -141,12 +143,12 @@ public class GitBrowseCommand extends AbstractGitCommand if (Util.isEmpty(request.getRevision())) { return getDefaultBranch(repo); } else { - ObjectId revId = GitUtil.getRevisionId(repo, request.getRevision()); - if (revId == null) { + ObjectId revisionId = GitUtil.getRevisionId(repo, request.getRevision()); + if (revisionId == null) { logger.error("could not find revision {}", request.getRevision()); throw notFound(entity("Revision", request.getRevision()).in(this.repository)); } - return revId; + return revisionId; } } @@ -212,7 +214,9 @@ public class GitBrowseCommand extends AbstractGitCommand private FileObject getEntry() throws IOException { try (RevWalk revWalk = new RevWalk(repo); TreeWalk treeWalk = new TreeWalk(repo)) { - logger.debug("load repository browser for revision {}", revId.name()); + if (logger.isDebugEnabled()) { // method call in logger call + logger.debug("load repository browser for revision {}", revId.name()); + } if (!isRootRequest()) { treeWalk.setFilter(PathFilter.create(request.getPath())); @@ -275,7 +279,7 @@ public class GitBrowseCommand extends AbstractGitCommand } private void createTree(TreeEntry parent, TreeWalk treeWalk) throws IOException { - Stack parents = new Stack<>(); + Deque parents = new ArrayDeque<>(); parents.push(parent); while (treeWalk.next()) { final String currentPath = treeWalk.getPathString(); @@ -283,11 +287,15 @@ public class GitBrowseCommand extends AbstractGitCommand parents.pop(); } TreeEntry currentParent = parents.peek(); - TreeEntry treeEntry = new TreeEntry(repo, treeWalk); - currentParent.addChild(treeEntry); - if (request.isRecursive() && treeEntry.getType() == TreeType.DIRECTORY) { - treeWalk.enterSubtree(); - parents.push(treeEntry); + TreeEntry treeEntry = createTreeEntry(repo, treeWalk); + if (treeEntry != null) { + currentParent.addChild(treeEntry); + if (request.isRecursive() && treeEntry.getType() == TreeType.DIRECTORY) { + treeWalk.enterSubtree(); + parents.push(treeEntry); + } + } else { + logger.warn("failed to find tree entry for {}", currentPath); } } } @@ -304,7 +312,12 @@ public class GitBrowseCommand extends AbstractGitCommand currentDepth++; if (currentDepth >= limit) { - return createFileObject(new TreeEntry(repo, treeWalk)); + TreeEntry treeEntry = createTreeEntry(repo, treeWalk); + if (treeEntry != null) { + return createFileObject(treeEntry); + } else { + logger.warn("could not find tree entry at {}", name); + } } else { treeWalk.enterSubtree(); } @@ -328,8 +341,12 @@ public class GitBrowseCommand extends AbstractGitCommand } } - private SubRepository getSubRepository(String path) - throws IOException { + @Nullable + private SubRepository getSubRepository(String path) throws IOException { + if (request.isDisableSubRepositoryDetection()) { + return null; + } + Map subRepositories = subrepositoryCache.get(revId); if (subRepositories == null) { @@ -447,6 +464,23 @@ public class GitBrowseCommand extends AbstractGitCommand FILE, DIRECTORY, SUB_REPOSITORY } + @Nullable + TreeEntry createTreeEntry(org.eclipse.jgit.lib.Repository repo, TreeWalk treeWalk) throws IOException { + String pathString = treeWalk.getPathString(); + ObjectId objectId = treeWalk.getObjectId(0); + SubRepository subRepository = getSubRepository(pathString); + if (subRepository != null) { + return new TreeEntry(pathString, treeWalk.getNameString(), objectId, subRepository); + } else if (repo.getObjectDatabase().has(objectId)) { + TreeType type = TreeType.FILE; + if (repo.open(objectId).getType() == Constants.OBJ_TREE) { + type = TreeType.DIRECTORY; + } + return new TreeEntry(pathString, treeWalk.getNameString(), objectId, type); + } + return null; + } + private class TreeEntry { private final String pathString; @@ -466,21 +500,20 @@ public class GitBrowseCommand extends AbstractGitCommand type = TreeType.DIRECTORY; } - TreeEntry(org.eclipse.jgit.lib.Repository repo, TreeWalk treeWalk) throws IOException { - this.pathString = treeWalk.getPathString(); - this.nameString = treeWalk.getNameString(); - this.objectId = treeWalk.getObjectId(0); + TreeEntry(String pathString, String nameString, ObjectId objectId, SubRepository subRepository) { + this.pathString = pathString; + this.nameString = nameString; + this.objectId = objectId; + this.type = TreeType.SUB_REPOSITORY; + this.subRepository = subRepository; + } - if (!request.isDisableSubRepositoryDetection() && GitBrowseCommand.this.getSubRepository(pathString) != null) { - subRepository = GitBrowseCommand.this.getSubRepository(pathString); - type = TreeType.SUB_REPOSITORY; - } else if (repo.open(objectId).getType() == Constants.OBJ_TREE) { - subRepository = null; - type = TreeType.DIRECTORY; - } else { - subRepository = null; - type = TreeType.FILE; - } + TreeEntry(String pathString, String nameString, ObjectId objectId, TreeType type) { + this.pathString = pathString; + this.nameString = nameString; + this.objectId = objectId; + this.type = type; + this.subRepository = null; } String getPathString() { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBrowseCommand_BrokenSubmoduleTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBrowseCommand_BrokenSubmoduleTest.java new file mode 100644 index 0000000000..4d73bef0f2 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitBrowseCommand_BrokenSubmoduleTest.java @@ -0,0 +1,129 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.spi; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.repository.BrowserResult; +import sonia.scm.repository.FileObject; +import sonia.scm.web.lfs.LfsBlobStoreFactory; + +import javax.annotation.Nonnull; +import java.io.IOException; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static sonia.scm.repository.spi.SyncAsyncExecutors.synchronousExecutor; + +@RunWith(MockitoJUnitRunner.class) +public class GitBrowseCommand_BrokenSubmoduleTest extends AbstractGitCommandTestBase { + + @Mock + private LfsBlobStoreFactory lfsBlobStoreFactory; + + private GitBrowseCommand command; + + @Before + public void createCommand() { + command = new GitBrowseCommand(createContext(), lfsBlobStoreFactory, synchronousExecutor()); + } + + @Test + public void testBrowse() throws IOException { + BrowserResult result = command.getBrowserResult(new BrowseCommandRequest()); + Collection children = result.getFile().getChildren(); + + List subrepos = subRepositoriesOnly(children); + assertThat(subrepos).containsExactly( + "anonymous-access", + "hasselhoffme", + "recipes", + "scm-redmine-plugin" + ); + + List directories = directoriesOnly(children); + assertThat(directories).containsExactly( + "dir", + "plugins" + ); + + List files = filesOnly(children); + assertThat(files) + .containsExactly( + ".gitmodules", + "README.md", + "test.txt" + ); + } + + @Test + public void testBrowseRecursive() throws IOException { + BrowseCommandRequest request = new BrowseCommandRequest(); + request.setRecursive(true); + BrowserResult result = command.getBrowserResult(request); + Collection children = result.getFile().getChildren(); + FileObject fileObject = children.stream().filter(f -> "plugins".equals(f.getPath())).findFirst().get(); + assertThat(fileObject.getChildren()).hasSize(3); + List subrepos = subRepositoriesOnly(fileObject.getChildren()); + assertThat(subrepos) + .containsExactly( + "plugins/scm-branchwp-plugin", + "plugins/scm-jira-plugin", + "plugins/statistic-plugin" + ); + } + + @Nonnull + private List filesOnly(Collection children) { + return children.stream().filter(f -> !f.isDirectory()).map(FileObject::getPath).collect(Collectors.toList()); + } + + @Nonnull + private List directoriesOnly(Collection children) { + return children.stream() + .filter(FileObject::isDirectory) + .filter(f -> f.getSubRepository() == null) + .map(FileObject::getPath) + .collect(Collectors.toList()); + } + + @Nonnull + private List subRepositoriesOnly(Collection children) { + return children.stream() + .filter(f -> f.getSubRepository() != null) + .map(FileObject::getPath) + .collect(Collectors.toList()); + } + + @Override + protected String getZippedRepositoryResource() { + return "sonia/scm/repository/spi/scm-git-broken-submodule-repo.zip"; + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-broken-submodule-repo.zip b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-broken-submodule-repo.zip new file mode 100644 index 0000000000..e123c32400 Binary files /dev/null and b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-broken-submodule-repo.zip differ