Merge pull request #34450 from overleaf/dp-handle-dot-git-error

[git-bridge] Surface GitUserException messages to the git client

GitOrigin-RevId: 45256ca8005142a0f70839e2567341a5984955a7

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolver.java

@@ -2,6 +2,7 @@ package uk.ac.ic.wlgitbridge.git.handler;
 
 import com.google.api.client.auth.oauth2.Credential;
 import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.Optional;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -102,8 +103,18 @@ public class WLRepositoryResolver implements RepositoryResolver<HttpServletReque
     } catch (ForbiddenException e) {
       throw new ServiceNotAuthorizedException();
     } catch (GitUserException e) {
+      /*
+       * Deliver the user-facing message as a git "ERR" pkt-line. ServiceMayNotContinueException
+       * defaults to a 403 status, but the git client only parses the pkt-line body (and so only
+       * displays the message) when the smart-HTTP advertisement is returned with a 200 status; on
+       * any other status it just prints "The requested URL returned error: <code>" and discards
+       * the body. So we explicitly use a 200 status here to surface messages such as the
+       * "project contains a '.git' entity" error to the user.
+       */
       throw new ServiceMayNotContinueException(
-          e.getMessage() + "\n" + String.join("\n", e.getDescriptionLines()), e);
+          e.getMessage() + "\n" + String.join("\n", e.getDescriptionLines()),
+          e,
+          HttpServletResponse.SC_OK);
     } catch (IOException e) {
       Log.warn("IOException when trying to open repo: " + projName, e);
       throw new ServiceMayNotContinueException("Internal server error.");

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java

@@ -88,14 +88,8 @@ public abstract class Request<T extends Result> {
             if ("projectHasDotGit".equals(code)) {
               throw new MissingRepositoryException(
                   Arrays.asList(
-                      "This project contains a '.git' entity at the top level, indicating that it is",
-                      "already a git repository. The Overleaf git-bridge cannot work with this project",
-                      "due to a known problem with handling these '.git' folders.",
-                      "",
-                      "We recommend removing the .git folder before trying again.",
-                      "",
-                      "If this is unexpected, please contact us at support@overleaf.com, or",
-                      "see https://www.overleaf.com/learn/how-to/Git_integration for more information."));
+                      "Git access won't work when a project contains a folder named '.git'.",
+                      "Please remove any folder named '.git' from your project in Overleaf and try again."));
             } else {
               throw new MissingRepositoryException(Arrays.asList("Conflict: 409"));
             }

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/git/handler/WLRepositoryResolverTest.java

@@ -0,0 +1,51 @@
+package uk.ac.ic.wlgitbridge.git.handler;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.util.Arrays;
+import org.eclipse.jgit.transport.ServiceMayNotContinueException;
+import org.junit.Test;
+import uk.ac.ic.wlgitbridge.bridge.Bridge;
+import uk.ac.ic.wlgitbridge.snapshot.base.MissingRepositoryException;
+
+public class WLRepositoryResolverTest {
+
+  private static final String DOT_GIT_LINE =
+      "Git access won't work when a project contains a folder named '.git'.";
+
+  /*
+   * A GitUserException (e.g. the projectHasDotGit error) must be surfaced to the git client with a
+   * 200 status code. The git smart-HTTP client only parses (and displays) the "ERR" pkt-line body
+   * when the advertisement is returned with a 200 status; on any other status it just prints
+   * "The requested URL returned error: <code>" and discards the body. JGit's
+   * ServiceMayNotContinueException otherwise defaults to a 403, which would hide the message.
+   */
+  @Test
+  public void gitUserExceptionIsReturnedWithA200StatusAndMessage() throws Exception {
+    Bridge bridge = mock(Bridge.class);
+    when(bridge.getUpdatedRepo(any(), eq("proj")))
+        .thenThrow(
+            new MissingRepositoryException(
+                Arrays.asList(
+                    DOT_GIT_LINE,
+                    "Please remove any folder named '.git' from your project in Overleaf and try"
+                        + " again.")));
+    WLRepositoryResolver resolver = new WLRepositoryResolver(bridge);
+    HttpServletRequest request = mock(HttpServletRequest.class);
+
+    ServiceMayNotContinueException e =
+        assertThrows(
+            ServiceMayNotContinueException.class, () -> resolver.open(request, "proj.git"));
+
+    assertEquals(HttpServletResponse.SC_OK, e.getStatusCode());
+    assertTrue(e.getMessage().contains(DOT_GIT_LINE));
+  }
+}