Fix internal server error for git sub modules without tree object

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

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,12 +284,16 @@ public class GitBrowseCommand extends AbstractGitCommand
         parents.pop();
       }
       TreeEntry currentParent = parents.peek();
-      TreeEntry treeEntry = new TreeEntry(repo, treeWalk);
+      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)
+  @Nullable
-    throws IOException {
+  private SubRepository getSubRepository(String path) throws IOException {
+    if (request.isDisableSubRepositoryDetection()) {
+      return null;
+    }
+
     Map<String, SubRepository> 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 {
+    TreeEntry(String pathString, String nameString, ObjectId objectId, SubRepository subRepository) {
-      this.pathString = treeWalk.getPathString();
+      this.pathString = pathString;
-      this.nameString = treeWalk.getNameString();
+      this.nameString = nameString;
-      this.objectId = treeWalk.getObjectId(0);
+      this.objectId = objectId;
-
+      this.type = TreeType.SUB_REPOSITORY;
-      if (!request.isDisableSubRepositoryDetection() && GitBrowseCommand.this.getSubRepository(pathString) != null) {
+      this.subRepository = subRepository;
-        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() {

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<FileObject> children = result.getFile().getChildren();
+
+    List<String> subrepos = subRepositoriesOnly(children);
+    assertThat(subrepos).containsExactly(
+      "anonymous-access",
+      "hasselhoffme",
+      "recipes",
+      "scm-redmine-plugin"
+    );
+
+    List<String> directories = directoriesOnly(children);
+    assertThat(directories).containsExactly(
+      "dir",
+      "plugins"
+      );
+
+    List<String> 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<FileObject> children = result.getFile().getChildren();
+    FileObject fileObject = children.stream().filter(f -> "plugins".equals(f.getPath())).findFirst().get();
+    assertThat(fileObject.getChildren()).hasSize(3);
+    List<String> subrepos = subRepositoriesOnly(fileObject.getChildren());
+    assertThat(subrepos)
+      .containsExactly(
+        "plugins/scm-branchwp-plugin",
+        "plugins/scm-jira-plugin",
+        "plugins/statistic-plugin"
+      );
+  }
+
+  @Nonnull
+  private List<String> filesOnly(Collection<FileObject> children) {
+    return children.stream().filter(f -> !f.isDirectory()).map(FileObject::getPath).collect(Collectors.toList());
+  }
+
+  @Nonnull
+  private List<String> directoriesOnly(Collection<FileObject> children) {
+    return children.stream()
+      .filter(FileObject::isDirectory)
+      .filter(f -> f.getSubRepository() == null)
+      .map(FileObject::getPath)
+      .collect(Collectors.toList());
+  }
+
+  @Nonnull
+  private List<String> subRepositoriesOnly(Collection<FileObject> 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";
+  }
+}