diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbd392b0f..2159d10d1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### 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..b80349cb2b 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,6 +55,7 @@ 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; @@ -283,11 +284,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 +309,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 +338,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 +461,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 +497,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