Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
Adapt for authenticated requests
Refactor test
about.md: Correct in-tree test command
Change-Id: I4bd3a092cce25e3e22458ab266c09645088db355
diff --git a/src/main/java/com/ericsson/gerrit/plugins/goimport/GoImportFilter.java b/src/main/java/com/ericsson/gerrit/plugins/goimport/GoImportFilter.java
index 3675d92..ea8a4dc 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/goimport/GoImportFilter.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/goimport/GoImportFilter.java
@@ -14,21 +14,16 @@
package com.ericsson.gerrit.plugins.goimport;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.HtmlDomUtil;
-import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.util.http.CacheHeaders;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.io.OutputStream;
@@ -36,6 +31,7 @@
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.regex.Pattern;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
@@ -45,6 +41,25 @@
@Singleton
public class GoImportFilter extends AllRequestFilter {
+ @VisibleForTesting static final String CONTENT_PLH = "${content}";
+
+ @VisibleForTesting
+ static final String PAGE_200 =
+ "<!DOCTYPE html>\n"
+ + "<html>\n"
+ + "<head>\n"
+ + " <title>Gerrit-Go-Import</title>\n"
+ + " <meta http-equiv=\"Content-Type\" content=\"text/html; charset=UTF-8\"/>\n"
+ + " <meta name=\"go-import\" content=\""
+ + CONTENT_PLH
+ + "\"/>\n"
+ + "</head>\n"
+ + "<body>\n"
+ + "<div>\n"
+ + " Gerrit-Go-Import\n"
+ + "</div>\n"
+ + "</body>\n"
+ + "</html>";
private static final String PAGE_404 =
"<!DOCTYPE html>\n"
@@ -58,36 +73,14 @@
+ "</body>\n"
+ "</html>";
- private static final String PAGE_200 =
- "<!DOCTYPE html>\n"
- + "<html>\n"
- + "<head>\n"
- + " <title>Gerrit-Go-Import</title>\n"
- + " <meta http-equiv=\"Content-Type\" content=\"text/html; charset=UTF-8\"/>\n"
- + " <meta name=\"go-import\" content=\"${content}\"/>\n"
- + "</head>\n"
- + "<body>\n"
- + "<div>\n"
- + " Gerrit-Go-Import\n"
- + "</div>\n"
- + "</body>\n"
- + "</html>";
-
- private final Provider<AnonymousUser> anonProvider;
- private final PermissionBackend permissions;
+ private static final Pattern AUTHENTICATED_REQ = Pattern.compile("^/a/.*");
private final ProjectCache projectCache;
final String webUrl;
final String projectPrefix;
@Inject
- GoImportFilter(
- Provider<AnonymousUser> anonProvider,
- PermissionBackend permissions,
- ProjectCache projectCache,
- @CanonicalWebUrl String webUrl)
+ GoImportFilter(ProjectCache projectCache, @CanonicalWebUrl String webUrl)
throws URISyntaxException {
- this.anonProvider = anonProvider;
- this.permissions = permissions;
this.projectCache = projectCache;
this.webUrl = webUrl.replaceFirst("/?$", "/");
this.projectPrefix = generateProjectPrefix();
@@ -104,8 +97,11 @@
if (request instanceof HttpServletRequest) {
HttpServletRequest req = (HttpServletRequest) request;
HttpServletResponse rsp = (HttpServletResponse) response;
- String servletPath = req.getServletPath();
if ("1".equals(req.getParameter("go-get"))) {
+ boolean authenticated = AUTHENTICATED_REQ.matcher(req.getServletPath()).matches();
+ // For authenticated requests remove prefix "/a" to get project name.
+ String path =
+ authenticated ? req.getServletPath().replaceFirst("^/a/", "/") : req.getServletPath();
// Because Gerrit allows for arbitrary-depth project names
// (that is, both "a" and "a/b/c" are both legal), we are going
// to find the most specific such project that matches the path.
@@ -118,11 +114,11 @@
// 3. If the requested path is "a/c", then project "a" would be chosen.
// 4. If the requested path is "a/b/c/d", then project "a/b" would be chosen.
// 5. If the requested path is "x/y/z", then this will fail with a 404 error.
- String existent = getLongestMatch(getProjectName(servletPath));
+ String existent = getLongestMatch(getProjectName(path));
byte[] toSend = PAGE_404.getBytes();
rsp.setStatus(404);
if (!Strings.isNullOrEmpty(existent)) {
- toSend = PAGE_200.replace("${content}", getContent(existent)).getBytes();
+ toSend = PAGE_200.replace(CONTENT_PLH, getContent(existent, authenticated)).getBytes();
rsp.setStatus(200);
}
CacheHeaders.setNotCacheable(rsp);
@@ -159,24 +155,14 @@
return servletPath.replaceFirst("/", "");
}
- private CharSequence getContent(String projectName) {
- return projectPrefix + projectName + " git " + getRepoRoot(projectName);
- }
-
- private String getRepoRoot(String projectName) {
- if (allowsAnonymousAccess(projectName)) {
- return webUrl + projectName;
- }
-
- return webUrl + "a/" + projectName;
- }
-
- private boolean allowsAnonymousAccess(String projectName) {
- AnonymousUser anonymous = anonProvider.get();
- Branch.NameKey heads =
- new Branch.NameKey(new Project.NameKey(projectName), RefNames.REFS_HEADS);
-
- return permissions.user(anonymous).ref(heads).testOrFalse(RefPermission.READ);
+ private CharSequence getContent(String projectName, boolean authenticated) {
+ return projectPrefix
+ + (authenticated ? "a/" : "")
+ + projectName
+ + " git "
+ + webUrl
+ + "a/"
+ + projectName;
}
private boolean projectExists(String projectName) {
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index 8e5bbf7..47eb5a9 100644
--- a/src/main/resources/Documentation/build.md
+++ b/src/main/resources/Documentation/build.md
@@ -61,7 +61,7 @@
To execute the tests run:
```
- bazel test plugins/@PLUGIN@:go_import_tests
+ bazel test plugins/@PLUGIN@:go-import_tests
```
How to build the Gerrit Plugin API is described in the [Gerrit
diff --git a/src/test/java/com/ericsson/gerrit/plugins/goimport/GoImportFilterTest.java b/src/test/java/com/ericsson/gerrit/plugins/goimport/GoImportFilterTest.java
index 3b24f18..6cf2b40 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/goimport/GoImportFilterTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/goimport/GoImportFilterTest.java
@@ -14,6 +14,8 @@
package com.ericsson.gerrit.plugins.goimport;
+import static com.ericsson.gerrit.plugins.goimport.GoImportFilter.CONTENT_PLH;
+import static com.ericsson.gerrit.plugins.goimport.GoImportFilter.PAGE_200;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
@@ -25,7 +27,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Provider;
@@ -44,28 +45,25 @@
@RunWith(MockitoJUnitRunner.class)
public class GoImportFilterTest {
+ private static final String PROD_FQDN = "gerrit-review.googlesource.com";
+ private static final String PROD_URL = "https://" + PROD_FQDN;
+ private static final String PROJECT_NAME = "projectName";
+ private static final String CONTENT_FORMAT = "%1$s/%3$s git %2$s/%3$s";
+ private static final String CONTENT =
+ String.format(CONTENT_FORMAT, PROD_FQDN, auth(PROD_URL), PROJECT_NAME);
+ private static final String AUTH_CONTENT =
+ String.format(CONTENT_FORMAT, auth(PROD_FQDN), auth(PROD_URL), PROJECT_NAME);
- private static final String PROD_URL = "https://gerrit-review.googlesource.com";
- private static final String PROJECT_URL_AUTH =
- "https://gerrit-review.googlesource.com/a/projectName";
- private static final String PROJECT_URL_ANON =
- "https://gerrit-review.googlesource.com/projectName";
- private static final String PAGE_200 =
- "<!DOCTYPE html>\n"
- + "<html>\n"
- + "<head>\n"
- + " <title>Gerrit-Go-Import</title>\n"
- + " <meta http-equiv=\"Content-Type\" content=\"text/html; charset=UTF-8\"/>\n"
- + " <meta name=\"go-import\" content=\"gerrit-review.googlesource.com/projectName git ${content}\"/>\n"
- + "</head>\n"
- + "<body>\n"
- + "<div>\n"
- + " Gerrit-Go-Import\n"
- + "</div>\n"
- + "</body>\n"
- + "</html>";
- private static final String PAGE_200_AUTH = PAGE_200.replace("${content}", PROJECT_URL_AUTH);
- private static final String PAGE_200_ANON = PAGE_200.replace("${content}", PROJECT_URL_ANON);
+ /** URLs that require authentication has an additional "/a" ($BASE/a/$ENDPOINT). */
+ private static String auth(String baseUrl) {
+ return baseUrl + "/a";
+ }
+
+ private static byte[] response200(boolean authenticated) {
+ return authenticated
+ ? PAGE_200.replace(CONTENT_PLH, AUTH_CONTENT).getBytes()
+ : PAGE_200.replace(CONTENT_PLH, CONTENT).getBytes();
+ }
private GoImportFilter unitUnderTest;
@@ -83,31 +81,23 @@
@Before
public void setUp() throws Exception {
- unitUnderTest = new GoImportFilter(mockAnonProvider, mockPerms, mockProjectCache, PROD_URL);
+ unitUnderTest = new GoImportFilter(mockProjectCache, PROD_URL);
assertThat(unitUnderTest).isNotNull();
when(mockResponse.getOutputStream()).thenReturn(mockOutputStream);
-
- when(mockAnonProvider.get()).thenReturn(mockAnon);
- when(mockPerms.user(mockAnon)).thenReturn(mockPermsWithUser);
- when(mockPermsWithUser.ref(any())).thenReturn(mockPermsForRef);
}
@Test
public void testConstructor() throws Exception {
assertThat(unitUnderTest.webUrl.endsWith("/")).isTrue();
unitUnderTest =
- new GoImportFilter(
- mockAnonProvider,
- mockPerms,
- mockProjectCache,
- "http://gerrit-review.googlesource.com:8080/");
+ new GoImportFilter(mockProjectCache, "http://gerrit-review.googlesource.com:8080/");
assertThat(unitUnderTest.webUrl.endsWith("/")).isTrue();
assertThat(unitUnderTest.projectPrefix).isNotNull();
}
@Test(expected = URISyntaxException.class)
public void testConstructorWithURISyntaxException() throws Exception {
- unitUnderTest = new GoImportFilter(mockAnonProvider, mockPerms, mockProjectCache, "\\\\");
+ unitUnderTest = new GoImportFilter(mockProjectCache, "\\\\");
}
@Test
@@ -118,7 +108,6 @@
@Test
public void testDoFilterWithoutGoGetParameter() throws Exception {
- when(mockRequest.getServletPath()).thenReturn("/projectName");
when(mockRequest.getParameter("go-get")).thenReturn(null);
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
verify(mockOutputStream, times(0)).write(any(byte[].class));
@@ -127,7 +116,6 @@
@Test
public void testDoFilterWithWrongGoGetParameterValue() throws Exception {
- when(mockRequest.getServletPath()).thenReturn("/projectName");
when(mockRequest.getParameter("go-get")).thenReturn("2");
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
verify(mockOutputStream, times(0)).write(any(byte[].class));
@@ -136,30 +124,38 @@
@Test
public void testDoFilterWithExistingProject() throws Exception {
- when(mockRequest.getServletPath()).thenReturn("/projectName");
+ when(mockRequest.getServletPath()).thenReturn("/" + PROJECT_NAME);
when(mockRequest.getParameter("go-get")).thenReturn("1");
- when(mockProjectCache.get(new Project.NameKey("projectName"))).thenReturn(mockProjectState);
- when(mockPermsForRef.testOrFalse(RefPermission.READ)).thenReturn(false);
+ when(mockProjectCache.get(new Project.NameKey(PROJECT_NAME))).thenReturn(mockProjectState);
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
- verify(mockOutputStream, times(1)).write(PAGE_200_AUTH.getBytes());
+ verify(mockOutputStream, times(1)).write(response200(false));
verify(mockChain, times(0)).doFilter(mockRequest, mockResponse);
verify(mockProjectCache, times(1)).get(any(Project.NameKey.class));
verify(mockResponse, times(1)).setStatus(200);
- verify(mockPermsForRef, times(1)).testOrFalse(RefPermission.READ);
+ }
+
+ @Test
+ public void testDoFilterAuthenticatedWithExistingProject() throws Exception {
+ when(mockRequest.getServletPath()).thenReturn("/a/" + PROJECT_NAME);
+ when(mockRequest.getParameter("go-get")).thenReturn("1");
+ when(mockProjectCache.get(new Project.NameKey(PROJECT_NAME))).thenReturn(mockProjectState);
+ unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
+ verify(mockOutputStream, times(1)).write(response200(true));
+ verify(mockChain, times(0)).doFilter(mockRequest, mockResponse);
+ verify(mockProjectCache, times(1)).get(any(Project.NameKey.class));
+ verify(mockResponse, times(1)).setStatus(200);
}
@Test
public void testDoFilterWithExistingProjectAndPackage() throws Exception {
- when(mockRequest.getServletPath()).thenReturn("/projectName/my/package");
+ when(mockRequest.getServletPath()).thenReturn("/" + PROJECT_NAME + "/my/package");
when(mockRequest.getParameter("go-get")).thenReturn("1");
- when(mockProjectCache.get(new Project.NameKey("projectName"))).thenReturn(mockProjectState);
- when(mockPermsForRef.testOrFalse(RefPermission.READ)).thenReturn(false);
+ when(mockProjectCache.get(new Project.NameKey(PROJECT_NAME))).thenReturn(mockProjectState);
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
- verify(mockOutputStream, times(1)).write(PAGE_200_AUTH.getBytes());
+ verify(mockOutputStream, times(1)).write(response200(false));
verify(mockChain, times(0)).doFilter(mockRequest, mockResponse);
verify(mockProjectCache, times(3)).get(any(Project.NameKey.class));
verify(mockResponse, times(1)).setStatus(200);
- verify(mockPermsForRef, times(1)).testOrFalse(RefPermission.READ);
}
@Test
@@ -167,18 +163,16 @@
when(mockRequest.getServletPath()).thenReturn("/projectName");
when(mockRequest.getParameter("go-get")).thenReturn("1");
when(mockProjectCache.get(new Project.NameKey("projectName"))).thenReturn(mockProjectState);
- when(mockPermsForRef.testOrFalse(RefPermission.READ)).thenReturn(true);
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
- verify(mockOutputStream, times(1)).write(PAGE_200_ANON.getBytes());
+ verify(mockOutputStream, times(1)).write(response200(false));
verify(mockChain, times(0)).doFilter(mockRequest, mockResponse);
verify(mockProjectCache, times(1)).get(any(Project.NameKey.class));
verify(mockResponse, times(1)).setStatus(200);
- verify(mockPermsForRef, times(1)).testOrFalse(RefPermission.READ);
}
@Test
public void testDoFilterWithNonExistingProject() throws Exception {
- when(mockRequest.getServletPath()).thenReturn("/projectName");
+ when(mockRequest.getServletPath()).thenReturn("/" + PROJECT_NAME);
when(mockRequest.getParameter("go-get")).thenReturn("1");
when(mockProjectCache.get(any(Project.NameKey.class))).thenReturn(null);
unitUnderTest.doFilter(mockRequest, mockResponse, mockChain);
@@ -191,7 +185,7 @@
@Test
public void testDoFilterWithIOException() throws Exception {
String msg = "test-io-error";
- when(mockRequest.getServletPath()).thenReturn("/projectName");
+ when(mockRequest.getServletPath()).thenReturn("/" + PROJECT_NAME);
when(mockRequest.getParameter("go-get")).thenReturn("1");
doThrow(new IOException(msg)).when(mockOutputStream).write(any(byte[].class));
when(mockProjectCache.get(any(Project.NameKey.class))).thenReturn(mockProjectState);
