improve git client detection at GitPermissionFilter to include jgit

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java

@@ -70,6 +70,7 @@ import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 import javax.servlet.http.HttpServletRequest;
+import sonia.scm.web.GitUserAgentProvider;
 
 /**
  *
@@ -78,6 +79,8 @@ import javax.servlet.http.HttpServletRequest;
 public final class GitUtil
 {
   
+  private static final GitUserAgentProvider GIT_USER_AGENT_PROVIDER = new GitUserAgentProvider();
+
   /** Field description */
   public static final String REF_HEAD = "HEAD";
 
@@ -698,7 +701,7 @@ public final class GitUtil
    */
   public static boolean isGitClient(HttpServletRequest request)
   {
-    return HttpUtil.userAgentStartsWith(request, USERAGENT_GIT);
+    return GIT_USER_AGENT_PROVIDER.parseUserAgent(request.getHeader(HttpUtil.HEADER_USERAGENT)) != null;
   }
 
   /**

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitPermissionFilter.java

@@ -55,6 +55,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 /**
+ * GitPermissionFilter decides if a git request requires write or read privileges.
  * 
  * @author Sebastian Sdorra
  */
@@ -62,73 +63,41 @@ import javax.servlet.http.HttpServletResponse;
 public class GitPermissionFilter extends ProviderPermissionFilter
 {
 
-  /** Field description */
-  public static final String PARAMETER_SERVICE = "service";
+  private static final String PARAMETER_SERVICE = "service";
 
-  /** Field description */
-  public static final String PARAMETER_VALUE_RECEIVE = "git-receive-pack";
+  private static final String PARAMETER_VALUE_RECEIVE = "git-receive-pack";
 
-  /** Field description */
-  public static final String URI_RECEIVE_PACK = "git-receive-pack";
+  private static final String URI_RECEIVE_PACK = "git-receive-pack";
 
-  /** Field description */
-  public static final String URI_REF_INFO = "/info/refs";
+  private static final String URI_REF_INFO = "/info/refs";
   
-  public static final String METHOD_LFS_UPLOAD = "PUT";
+  private static final String METHOD_LFS_UPLOAD = "PUT";
 
   //~--- constructors ---------------------------------------------------------
 
   /**
-   * Constructs ...
+   * Constructs a new instance of the GitPermissionFilter.
    *
-   * @param configuration
-   * @param repositoryProvider
+   * @param configuration scm main configuration
+   * @param repositoryProvider repository provider
    */
   @Inject
-  public GitPermissionFilter(ScmConfiguration configuration,
-    RepositoryProvider repositoryProvider)
-  {
+  public GitPermissionFilter(ScmConfiguration configuration, RepositoryProvider repositoryProvider) {
     super(configuration, repositoryProvider);
   }
 
-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param request
-   * @param response
-   *
-   * @throws IOException
-   */
   @Override
-  protected void sendNotEnoughPrivilegesError(HttpServletRequest request,
-    HttpServletResponse response)
-    throws IOException
-  {
-    if (GitUtil.isGitClient(request))
-    {
+  protected void sendNotEnoughPrivilegesError(HttpServletRequest request, HttpServletResponse response) 
+    throws IOException {
+    if (GitUtil.isGitClient(request)) {
       GitSmartHttpTools.sendError(request, response,
         HttpServletResponse.SC_FORBIDDEN,
         ClientMessages.get(request).notEnoughPrivileges());
-    }
-    else
-    {
+    } else {
       super.sendNotEnoughPrivilegesError(request, response);
     }
   }
 
-  //~--- get methods ----------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param request
-   *
-   * @return
-   */
   @Override
   protected boolean isWriteRequest(HttpServletRequest request) {
     return isReceivePackRequest(request) ||

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitUtilTest.java

@@ -42,8 +42,10 @@ import static org.mockito.Mockito.*;
 
 import java.io.File;
 import java.io.IOException;
+import javax.servlet.http.HttpServletRequest;
 
 import static org.junit.Assert.*;
+import sonia.scm.util.HttpUtil;
 
 /**
  * Unit tests for {@link GitUtil}.
@@ -114,4 +116,22 @@ public class GitUtilTest
 
     return repo;
   }
+  
+  @Test
+  public void testIsGitClient() {
+    HttpServletRequest request = mockRequestWithUserAgent("Git/2.9.3");
+    assertTrue(GitUtil.isGitClient(request));
+    
+    request = mockRequestWithUserAgent("JGit/2.9.3");
+    assertTrue(GitUtil.isGitClient(request));
+    
+    request = mockRequestWithUserAgent("Mozilla/5.0 (Linux; Android 4.0.4; Galaxy Nexus Build/IMM76B) ...");
+    assertFalse(GitUtil.isGitClient(request));
+  }
+  
+  private HttpServletRequest mockRequestWithUserAgent(String userAgent) {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getHeader(HttpUtil.HEADER_USERAGENT)).thenReturn(userAgent);    
+    return request;
+  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitPermissionFilterTest.java

@@ -17,6 +17,7 @@ import static org.mockito.Mockito.*;
 import org.mockito.runners.MockitoJUnitRunner;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.repository.RepositoryProvider;
+import sonia.scm.util.HttpUtil;
 
 /**
  * Unit tests for {@link GitPermissionFilter}.
@@ -84,8 +85,17 @@ public class GitPermissionFilterTest {
   
   @Test
   public void testSendNotEnoughPrivilegesErrorAsGitClient() throws IOException {
+    verifySendNotEnoughPrivilegesErrorAsGitClient("git/2.9.3");
+  }
+  
+  @Test
+  public void testSendNotEnoughPrivilegesErrorAsJGitClient() throws IOException {
+    verifySendNotEnoughPrivilegesErrorAsGitClient("JGit/4.2");
+  }
+  
+  private void verifySendNotEnoughPrivilegesErrorAsGitClient(String userAgent) throws IOException {
     HttpServletRequest request = mockGitReceivePackServiceRequest();
-    when(request.getHeader("User-Agent")).thenReturn("git/2.9.3");
+    when(request.getHeader(HttpUtil.HEADER_USERAGENT)).thenReturn(userAgent);
     
     CapturingServletOutputStream stream = new CapturingServletOutputStream();
     when(response.getOutputStream()).thenReturn(stream);

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitUserAgentProviderTest.java

@@ -33,8 +33,6 @@ package sonia.scm.web;
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.base.Strings;
-
 import org.junit.Test;
 
 import static org.junit.Assert.*;