From e595d6039c7ac8b0fcd1f3d3142c4fc8fad12490 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 5 Sep 2018 17:06:21 +0200 Subject: [PATCH 1/4] implement the diff endpoint for v2 --- .../main/java/sonia/scm/web/VndMediaType.java | 3 + .../sonia/scm/it/RepositoryAccessITCase.java | 37 ++++- .../api/v2/resources/DiffRootResource.java | 62 ++++++-- .../api/v2/resources/DiffResourceTest.java | 133 ++++++++++++++++++ 4 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java diff --git a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java index 73bcf10bd0..20735f83fb 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -12,6 +12,8 @@ public class VndMediaType { private static final String SUBTYPE_PREFIX = "vnd.scmm-"; public static final String PREFIX = TYPE + "/" + SUBTYPE_PREFIX; public static final String SUFFIX = "+json;v=" + VERSION; + public static final String PLAIN_TEXT_PREFIX = "text/" + SUBTYPE_PREFIX; + public static final String PLAIN_TEXT_SUFFIX = "+plain;v=" + VERSION; public static final String USER = PREFIX + "user" + SUFFIX; public static final String GROUP = PREFIX + "group" + SUFFIX; @@ -20,6 +22,7 @@ public class VndMediaType { public static final String CHANGESET = PREFIX + "changeset" + SUFFIX; public static final String CHANGESET_COLLECTION = PREFIX + "changesetCollection" + SUFFIX; public static final String BRANCH = PREFIX + "branch" + SUFFIX; + public static final String DIFF = PLAIN_TEXT_PREFIX + "diff" + PLAIN_TEXT_SUFFIX; public static final String USER_COLLECTION = PREFIX + "userCollection" + SUFFIX; public static final String GROUP_COLLECTION = PREFIX + "groupCollection" + SUFFIX; public static final String REPOSITORY_COLLECTION = PREFIX + "repositoryCollection" + SUFFIX; diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index a9139f18c8..1722c7e4fa 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -16,8 +16,8 @@ import java.io.IOException; import java.util.Collection; import java.util.List; -import static org.assertj.core.api.Assertions.assertThat; import static java.lang.Thread.sleep; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertNotNull; import static sonia.scm.it.RestUtil.given; @@ -152,5 +152,40 @@ public class RepositoryAccessITCase { assertThat(changesets).size().isBetween(2, 3); // svn has an implicit root revision '0' that is extra to the two commits } + + @Test + public void shouldFindDiffs() throws IOException { + RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder); + + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "a.txt", "a"); + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "b.txt", "b"); + + String changesetsUrl = given() + .when() + .get(TestData.getDefaultRepositoryUrl(repositoryType)) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .path("_links.changesets.href"); + + String diffUrl = given() + .when() + .get(changesetsUrl) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .path("_embedded.changesets[0]._links.diff.href"); + + given() + .when() + .get(diffUrl) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .body() + .asString() + .contains("diff"); + + } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java index 176a86dda7..e98b3bc868 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java @@ -1,27 +1,65 @@ package sonia.scm.api.v2.resources; -import javax.ws.rs.DefaultValue; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import sonia.scm.NotFoundException; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; +import sonia.scm.repository.RevisionNotFoundException; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.util.HttpUtil; +import sonia.scm.web.VndMediaType; + +import javax.inject.Inject; import javax.ws.rs.GET; import javax.ws.rs.Path; -import javax.ws.rs.QueryParam; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; public class DiffRootResource { + public static final String HEADER_CONTENT_DISPOSITION = "Content-Disposition"; + private final RepositoryServiceFactory serviceFactory; - @GET - @Path("") - public Response getAll(@DefaultValue("0") @QueryParam("page") int page, - @DefaultValue("10") @QueryParam("pageSize") int pageSize, - @QueryParam("sortBy") String sortBy, - @DefaultValue("false") @QueryParam("desc") boolean desc) { - throw new UnsupportedOperationException(); + @Inject + public DiffRootResource(RepositoryServiceFactory serviceFactory) { + this.serviceFactory = serviceFactory; } @GET - @Path("{id}") - public Response get(String id) { - throw new UnsupportedOperationException(); + @Path("{revision}") + @Produces(VndMediaType.DIFF) + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 400, condition = "Bad Request"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the diff"), + @ResponseCode(code = 404, condition = "not found, no revision with the specified param for the repository available or repository not found"), + @ResponseCode(code = 500, condition = "internal server error") + }) + public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision) throws NotFoundException { + HttpUtil.checkForCRLFInjection(revision); + try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { + Repository repository = repositoryService.getRepository(); + RepositoryPermissions.read(repository).check(); + StreamingOutput responseEntry = output -> { + try { + repositoryService.getDiffCommand() + .setRevision(revision) + .retriveContent(output); + } catch (RevisionNotFoundException e) { + throw new WebApplicationException(Response.Status.NOT_FOUND); + } + }; + return Response.ok(responseEntry) + .header(HEADER_CONTENT_DISPOSITION, HttpUtil.createContentDispositionAttachmentHeader(String.format("%s-%s.diff", name, revision))) + .build(); + } } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java new file mode 100644 index 0000000000..a6f9c26779 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java @@ -0,0 +1,133 @@ +package sonia.scm.api.v2.resources; + + +import lombok.extern.slf4j.Slf4j; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; +import org.jboss.resteasy.core.Dispatcher; +import org.jboss.resteasy.mock.MockDispatcherFactory; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.junit.After; +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.api.rest.AuthorizationExceptionMapper; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.RevisionNotFoundException; +import sonia.scm.repository.api.DiffCommandBuilder; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.web.VndMediaType; + +import java.net.URISyntaxException; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.Silent.class) +@Slf4j +public class DiffResourceTest { + + + public static final String DIFF_PATH = "space/repo/diff/"; + public static final String DIFF_URL = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + DIFF_PATH; + private final Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + + @Mock + private RepositoryServiceFactory serviceFactory; + + @Mock + private RepositoryService service; + + @Mock + private DiffCommandBuilder diffCommandBuilder; + + private DiffRootResource diffRootResource; + + + private final Subject subject = mock(Subject.class); + private final ThreadState subjectThreadState = new SubjectThreadState(subject); + + + @Before + public void prepareEnvironment() throws Exception { + diffRootResource = new DiffRootResource(serviceFactory); + RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider + .of(new RepositoryResource(null, null, null, null, null, + null, null, null, null, MockProvider.of(diffRootResource))), null); + dispatcher.getRegistry().addSingletonResource(repositoryRootResource); + when(serviceFactory.create(new NamespaceAndName("space", "repo"))).thenReturn(service); + when(serviceFactory.create(any(Repository.class))).thenReturn(service); + when(service.getRepository()).thenReturn(new Repository("repoId", "git", "space", "repo")); + dispatcher.getProviderFactory().registerProvider(NotFoundExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(AuthorizationExceptionMapper.class); + when(service.getDiffCommand()).thenReturn(diffCommandBuilder); + subjectThreadState.bind(); + ThreadContext.bind(subject); + when(subject.isPermitted(any(String.class))).thenReturn(true); + } + + @After + public void cleanupContext() { + ThreadContext.unbindSubject(); + } + + @Test + public void shouldGetDiffs() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenReturn(diffCommandBuilder); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(200, response.getStatus()); + log.info("Response :{}", response.getContentAsString()); + assertThat(response.getStatus()) + .isEqualTo(200); + assertThat(response.getContentAsString()) + .isNotNull(); + String expectedHeader = "Content-Disposition"; + String expectedValue = "attachment; filename=\"repo-revision.diff\"; filename*=utf-8''repo-revision.diff"; + assertThat(response.getOutputHeaders().containsKey(expectedHeader)).isTrue(); + assertThat((String) response.getOutputHeaders().get("Content-Disposition").get(0)) + .contains(expectedValue); + } + + @Test + public void shouldGet404OnMissingRepository() throws URISyntaxException, RepositoryNotFoundException { + when(serviceFactory.create(any(NamespaceAndName.class))).thenThrow(RepositoryNotFoundException.class); + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(404, response.getStatus()); + } + + @Test + public void shouldGet404OnMissingRevision() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenThrow(RevisionNotFoundException.class); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(404, response.getStatus()); + } + +} From 6acfb38132b75d959a18da8eaad4bdd0eeb96df3 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 6 Sep 2018 13:54:52 +0200 Subject: [PATCH 2/4] add crlf exception and exception mapper --- .../sonia/scm/util/CRLFInjectionException.java | 8 ++++++++ .../src/main/java/sonia/scm/util/HttpUtil.java | 3 +-- .../resources/CRLFInjectionExceptionMapper.java | 13 +++++++++++++ .../scm/api/v2/resources/DiffRootResource.java | 12 ++++++++++-- .../scm/api/v2/resources/DiffResourceTest.java | 16 ++++++++++++++++ 5 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java diff --git a/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java new file mode 100644 index 0000000000..16dca8e910 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java @@ -0,0 +1,8 @@ +package sonia.scm.util; + +public class CRLFInjectionException extends IllegalArgumentException{ + + public CRLFInjectionException(String message) { + super(message); + } +} diff --git a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java index bc3f4e74cd..2744addc62 100644 --- a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java +++ b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java @@ -344,8 +344,7 @@ public final class HttpUtil "parameter \"{}\" contains a character which could be an indicator for a crlf injection", parameter); - throw new IllegalArgumentException( - "parameter contains an illegal character"); + throw new CRLFInjectionException("parameter contains an illegal character"); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java new file mode 100644 index 0000000000..f4e8d3aa3c --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java @@ -0,0 +1,13 @@ +package sonia.scm.api.v2.resources; + +import sonia.scm.api.rest.StatusExceptionMapper; +import sonia.scm.util.CRLFInjectionException; + +import javax.ws.rs.core.Response; + +public class CRLFInjectionExceptionMapper extends StatusExceptionMapper { + + public CRLFInjectionExceptionMapper() { + super(CRLFInjectionException.class, Response.Status.BAD_REQUEST); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java index e98b3bc868..1a2b3d2189 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java @@ -31,6 +31,16 @@ public class DiffRootResource { this.serviceFactory = serviceFactory; } + + /** + * Get the repository diff of a revision + * + * @param namespace repository namespace + * @param name repository name + * @param revision the revision + * @return the dif of the revision + * @throws NotFoundException if the repository is not found + */ @GET @Path("{revision}") @Produces(VndMediaType.DIFF) @@ -45,8 +55,6 @@ public class DiffRootResource { public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision) throws NotFoundException { HttpUtil.checkForCRLFInjection(revision); try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - Repository repository = repositoryService.getRepository(); - RepositoryPermissions.read(repository).check(); StreamingOutput responseEntry = output -> { try { repositoryService.getDiffCommand() diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java index a6f9c26779..0daeb07b7c 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java @@ -72,6 +72,7 @@ public class DiffResourceTest { when(service.getRepository()).thenReturn(new Repository("repoId", "git", "space", "repo")); dispatcher.getProviderFactory().registerProvider(NotFoundExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(AuthorizationExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(CRLFInjectionExceptionMapper.class); when(service.getDiffCommand()).thenReturn(diffCommandBuilder); subjectThreadState.bind(); ThreadContext.bind(subject); @@ -130,4 +131,19 @@ public class DiffResourceTest { assertEquals(404, response.getStatus()); } + @Test + public void shouldGet400OnCrlfInjection() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenThrow(RevisionNotFoundException.class); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "ny%0D%0ASet-cookie:%20Tamper=3079675143472450634") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(400, response.getStatus()); + } + + + } From faea3cddcef8a59b363c9bcffa8bd23891443e4d Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 6 Sep 2018 14:57:52 +0200 Subject: [PATCH 3/4] add crlf exception and exception mapper --- .../main/java/sonia/scm/api/v2/resources/DiffRootResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java index 1a2b3d2189..99a996b02f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java @@ -4,8 +4,6 @@ import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import sonia.scm.NotFoundException; import sonia.scm.repository.NamespaceAndName; -import sonia.scm.repository.Repository; -import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.RevisionNotFoundException; import sonia.scm.repository.api.RepositoryService; import sonia.scm.repository.api.RepositoryServiceFactory; From f57d87b257bd565fc2a7064c46572e024b86a455 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 10 Sep 2018 11:06:03 +0000 Subject: [PATCH 4/4] Close branch feature/diff_endpoint_v2