diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java index 6700ae3b8d..01fb21885e 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java @@ -51,13 +51,13 @@ import javax.servlet.http.HttpServletRequest; /** * Permission filter for mercurial repositories. - * + * * @author Sebastian Sdorra */ @Singleton public class HgPermissionFilter extends ProviderPermissionFilter { - + private static final Set READ_METHODS = ImmutableSet.of("GET", "HEAD", "OPTIONS", "TRACE"); /** @@ -78,6 +78,9 @@ public class HgPermissionFilter extends ProviderPermissionFilter @Override protected boolean isWriteRequest(HttpServletRequest request) { - return !READ_METHODS.contains(request.getMethod()); + if (READ_METHODS.contains(request.getMethod())) { + return WireProtocol.isWriteRequest(request); + } + return true; } } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/WireProtocol.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/WireProtocol.java new file mode 100644 index 0000000000..bab3083445 --- /dev/null +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/WireProtocol.java @@ -0,0 +1,192 @@ +/** + * Copyright (c) 2018, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + +package sonia.scm.web; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.collect.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.util.HttpUtil; + +import javax.servlet.http.HttpServletRequest; +import java.util.*; + +/** + * WireProtocol provides methods for handling the mercurial wire protocol. + * + * @see Mercurial Wire Protocol + */ +public final class WireProtocol { + + private static final Logger LOG = LoggerFactory.getLogger(WireProtocol.class); + + private static final Set READ_COMMANDS = ImmutableSet.of( + "batch", "between", "branchmap", "branches", "capabilities", "changegroup", "changegroupsubset", "clonebundles", + "getbundle", "heads", "hello", "listkeys", "lookup", "known", "stream_out", + // could not find lheads in the wireprotocol description but mercurial 4.5.2 uses it for clone + "lheads" + ); + + private static final Set WRITE_COMMANDS = ImmutableSet.of( + "pushkey", "unbundle" + ); + + private WireProtocol() { + } + + /** + * Returns {@code true} if the request is a write request. The method will always return {@code true}, expect for the + * following cases: + * + * - no command was specified with the request (is required for the hgweb ui) + * - the command in the query string was found in the list of read request + * - if query string contains the batch command, then all commands specified in X-HgArg headers must be + * in the list of read request + * + * @param request http request + * + * @return {@code true} for write requests. + */ + public static boolean isWriteRequest(HttpServletRequest request) { + List commands = commandsOf(request); + boolean write = isWriteRequest(commands); + LOG.trace("mercurial request {} is write: {}", commands, write); + return write; + } + + @VisibleForTesting + static boolean isWriteRequest(List commands) { + return !READ_COMMANDS.containsAll(commands); + } + + @VisibleForTesting + static List commandsOf(HttpServletRequest request) { + List listOfCmds = Lists.newArrayList(); + String cmd = getCommandFromQueryString(request); + if (cmd != null) { + listOfCmds.add(cmd); + if (isBatchCommand(cmd)) { + parseHgArgHeaders(request, listOfCmds); + } + } + return Collections.unmodifiableList(listOfCmds); + } + + private static void parseHgArgHeaders(HttpServletRequest request, List listOfCmds) { + Enumeration headerNames = request.getHeaderNames(); + while (headerNames.hasMoreElements()) { + String header = (String) headerNames.nextElement(); + parseHgArgHeader(request, listOfCmds, header); + } + } + + private static void parseHgArgHeader(HttpServletRequest request, List listOfCmds, String header) { + if (isHgArgHeader(header)) { + String value = getHeaderDecoded(request, header); + if (isHgArgCommandHeader(value)) { + parseHgCommandHeader(listOfCmds, value); + } + } + } + + private static void parseHgCommandHeader(List listOfCmds, String value) { + String[] cmds = value.substring(5).split(";"); + for (String cmd : cmds ) { + String normalizedCmd = normalize(cmd); + int index = normalizedCmd.indexOf(' '); + if (index > 0) { + listOfCmds.add(normalizedCmd.substring(0, index)); + } else { + listOfCmds.add(normalizedCmd); + } + } + } + + private static String normalize(String cmd) { + return cmd.trim().toLowerCase(Locale.ENGLISH); + } + + private static boolean isHgArgCommandHeader(String value) { + return value.startsWith("cmds="); + } + + private static String getHeaderDecoded(HttpServletRequest request, String header) { + return HttpUtil.decode(Strings.nullToEmpty(request.getHeader(header))); + } + + private static boolean isHgArgHeader(String header) { + return header.toLowerCase(Locale.ENGLISH).startsWith("x-hgarg-"); + } + + private static boolean isBatchCommand(String cmd) { + return "batch".equalsIgnoreCase(cmd); + } + + private static String getCommandFromQueryString(HttpServletRequest request) { + // we can't use getParameter, because this would inspect the body for form parameters as well + Multimap queryParameterMap = createQueryParameterMap(request); + + Collection cmd = queryParameterMap.get("cmd"); + Preconditions.checkArgument(cmd.size() <= 1, "found more than one cmd query parameter"); + Iterator iterator = cmd.iterator(); + + String command = null; + if (iterator.hasNext()) { + command = iterator.next(); + } + return command; + } + + private static Multimap createQueryParameterMap(HttpServletRequest request) { + Multimap parameterMap = HashMultimap.create(); + + String queryString = request.getQueryString(); + if (!Strings.isNullOrEmpty(queryString)) { + + String[] parameters = queryString.split("&"); + for (String parameter : parameters) { + int index = parameter.indexOf('='); + if (index > 0) { + parameterMap.put(parameter.substring(0, index), parameter.substring(index + 1)); + } else { + parameterMap.put(parameter, "true"); + } + } + + } + + return parameterMap; + } +} diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgPermissionFilterTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgPermissionFilterTest.java index 05250c8cf9..8319134078 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgPermissionFilterTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/HgPermissionFilterTest.java @@ -1,10 +1,10 @@ /** * Copyright (c) 2014, Sebastian Sdorra * All rights reserved. - * + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + * * 1. Redistributions of source code must retain the above copyright notice, * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, @@ -13,7 +13,7 @@ * 3. Neither the name of SCM-Manager; nor the names of its * contributors may be used to endorse or promote products derived from this * software without specific prior written permission. - * + * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE @@ -24,9 +24,9 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + * * http://bitbucket.org/sdorra/scm-manager - * + * */ package sonia.scm.web; @@ -48,7 +48,7 @@ import sonia.scm.repository.RepositoryProvider; /** * Unit tests for {@link HgPermissionFilter}. - * + * * @author Sebastian Sdorra */ @RunWith(MockitoJUnitRunner.class) @@ -56,7 +56,7 @@ public class HgPermissionFilterTest { @Mock private ScmConfiguration configuration; - + @Mock private RepositoryProvider repositoryProvider; @@ -64,7 +64,7 @@ public class HgPermissionFilterTest { @InjectMocks private HgPermissionFilter filter; - + /** * Tests {@link HgPermissionFilter#isWriteRequest(HttpServletRequest)}. */ @@ -75,7 +75,7 @@ public class HgPermissionFilterTest { assertFalse(isWriteRequest("HEAD")); assertFalse(isWriteRequest("TRACE")); assertFalse(isWriteRequest("OPTIONS")); - + // write methods assertTrue(isWriteRequest("POST")); assertTrue(isWriteRequest("PUT")); @@ -85,6 +85,7 @@ public class HgPermissionFilterTest { private boolean isWriteRequest(String method) { HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getQueryString()).thenReturn("cmd=capabilities"); when(request.getMethod()).thenReturn(method); return filter.isWriteRequest(request); } @@ -163,6 +164,17 @@ public class HgPermissionFilterTest { assertIsWriteRequest(wireProtocol.pushkey("markone&namespace=bookmarks&new=ef5993bb4abb32a0565c347844c6d939fc4f4b98&old=")); } + /** + * Tests {@link HgPermissionFilter#isWriteRequest(HttpServletRequest)} with a write request hidden in a batch GET + * request. + * + * @see Issue #970 + */ + @Test + public void testIsWriteRequestWithBookmarkPushInABatch() { + assertIsWriteRequest(wireProtocol.batch("pushkey key=markthree,namespace=bookmarks,new=187ddf37e237c370514487a0bb1a226f11a780b3,old=")); + } + private void assertIsReadRequest(HttpServletRequest request) { assertFalse(filter.isWriteRequest(request)); } diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolRequestMockFactory.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolRequestMockFactory.java index 3d2b6fab92..d1f5124b3a 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolRequestMockFactory.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolRequestMockFactory.java @@ -1,7 +1,11 @@ package sonia.scm.web; +import com.google.common.collect.Lists; + import javax.servlet.http.HttpServletRequest; +import java.util.Collections; +import java.util.List; import java.util.Locale; import static org.mockito.Mockito.*; @@ -21,55 +25,64 @@ public class WireProtocolRequestMockFactory { } public HttpServletRequest capabilities() { - return base("GET", "?cmd=capabilities"); + return base("GET", "cmd=capabilities"); } public HttpServletRequest listkeys(Namespace namespace) { - HttpServletRequest request = base("GET", "?cmd=capabilities"); + HttpServletRequest request = base("GET", "cmd=capabilities"); header(request, "vary", "X-HgArg-1"); header(request, "x-hgarg-1", namespaceValue(namespace)); return request; } public HttpServletRequest branchmap() { - return base("GET", "?cmd=branchmap"); + return base("GET", "cmd=branchmap"); } public HttpServletRequest batch(String... args) { - HttpServletRequest request = base("GET", "?cmd=batch"); + HttpServletRequest request = base("GET", "cmd=batch"); args(request, "cmds", args); return request; } public HttpServletRequest unbundle(long contentLength, String... heads) { - HttpServletRequest request = base("POST", "?cmd=unbundle"); + HttpServletRequest request = base("POST", "cmd=unbundle"); header(request, "Content-Length", String.valueOf(contentLength)); args(request, "heads", heads); return request; } public HttpServletRequest pushkey(String... keys) { - HttpServletRequest request = base("POST", "?cmd=pushkey"); + HttpServletRequest request = base("POST", "cmd=pushkey"); args(request, "key", keys); return request; } public HttpServletRequest known(String... nodes) { - HttpServletRequest request = base("POST", "?cmd=pushkey"); + HttpServletRequest request = base("GET", "cmd=known"); args(request, "nodes", nodes); return request; } private void args(HttpServletRequest request, String prefix, String[] values) { + List headers = Lists.newArrayList(); + StringBuilder vary = new StringBuilder(); for ( int i=0; i0) { vary.append(","); } - vary.append("X-HgArg-" + (i+1)); - header(request, "X-HgArg-" + (i+1), prefix + "=" + values[i]); + + vary.append(header); + headers.add(header); + + header(request, header, prefix + "=" + values[i]); } header(request, "Vary", vary.toString()); + + when(request.getHeaderNames()).thenReturn(Collections.enumeration(headers)); } private HttpServletRequest base(String method, String queryStringValue) { diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolTest.java new file mode 100644 index 0000000000..860b6a6392 --- /dev/null +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/web/WireProtocolTest.java @@ -0,0 +1,162 @@ +/** + * Copyright (c) 2018, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + +package sonia.scm.web; + +import com.google.common.collect.Lists; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import javax.servlet.http.HttpServletRequest; + +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.junit.Assert.*; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link WireProtocol}. + */ +@RunWith(MockitoJUnitRunner.class) +public class WireProtocolTest { + + @Mock + private HttpServletRequest request; + + @Test + public void testIsWriteRequestOnPost() { + assertIsWriteRequest("capabilities", "unbundle"); + } + + @Test + public void testIsWriteRequest() { + assertIsWriteRequest("unbundle"); + assertIsWriteRequest("capabilities", "unbundle"); + assertIsWriteRequest("capabilities", "postkeys"); + assertIsReadRequest(); + assertIsReadRequest("capabilities"); + assertIsReadRequest("capabilities", "branches", "branchmap"); + } + + private void assertIsWriteRequest(String... commands) { + List cmdList = Lists.newArrayList(commands); + assertTrue(WireProtocol.isWriteRequest(cmdList)); + } + + private void assertIsReadRequest(String... commands) { + List cmdList = Lists.newArrayList(commands); + assertFalse(WireProtocol.isWriteRequest(cmdList)); + } + + @Test + public void testGetCommandsOf() { + expectQueryCommand("capabilities", "cmd=capabilities"); + expectQueryCommand("unbundle", "cmd=unbundle"); + expectQueryCommand("unbundle", "prefix=stuff&cmd=unbundle"); + expectQueryCommand("unbundle", "cmd=unbundle&suffix=stuff"); + expectQueryCommand("unbundle", "prefix=stuff&cmd=unbundle&suffix=stuff"); + expectQueryCommand("unbundle", "bool=&cmd=unbundle"); + expectQueryCommand("unbundle", "bool&cmd=unbundle"); + expectQueryCommand("unbundle", "prefix=stu==ff&cmd=unbundle"); + } + + @Test + public void testGetCommandsOfWithBatch() { + prepareBatch("cmds=heads ;known nodes,ef5993bb4abb32a0565c347844c6d939fc4f4b98"); + List commands = WireProtocol.commandsOf(request); + assertThat(commands, contains("batch", "heads", "known")); + } + + @Test + public void testGetCommandsOfWithBatchEncoded() { + prepareBatch("cmds=heads+%3Bknown+nodes%3Def5993bb4abb32a0565c347844c6d939fc4f4b98"); + List commands = WireProtocol.commandsOf(request); + assertThat(commands, contains("batch", "heads", "known")); + } + + @Test + public void testGetCommandsOfWithBatchAndMutlipleLines() { + prepareBatch( + "cmds=heads+%3Bknown+nodes%3Def5993bb4abb32a0565c347844c6d939fc4f4b98", + "cmds=unbundle; postkeys", + "cmds= branchmap p1=r2,p2=r4; listkeys" + ); + List commands = WireProtocol.commandsOf(request); + assertThat(commands, contains("batch", "heads", "known", "unbundle", "postkeys", "branchmap", "listkeys")); + } + + private void prepareBatch(String... args) { + when(request.getQueryString()).thenReturn("cmd=batch"); + List headers = Lists.newArrayList(); + for (int i=0; i commands = WireProtocol.commandsOf(request); + assertEquals(1, commands.size()); + assertTrue(commands.contains(expected)); + } + +}