UploadPack: Test filtering by AdvertiseRefsHook in stateless transports
AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit. If this hook is not called, then all refs are treated as visible, causing the server to serve commits reachable from branches the client should not be able to access, if asked to via a request naming a guessed object id. Until 3a529361a76e8267467071e0b13ebb36b97d8fb2 (Call AdvertiseRefsHook before validating wants, 2018-12-18), UploadPack would invoke this hook at ref advertisement time but not during negotiation and when serving a pack file. Add a test to avoid regressing. Stateful bidirectional transports were not affected, so the test uses HTTP. [jn: split out when backporting the fix to stable-4.5. The test passes as long as v4.9.0.201710071750-r~169 (fetch: Accept any SHA-1 on lhs of refspec, 2017-06-04) is cherry picked along with it.] Change-Id: I8c017107336adc7cb4c826985779676bf043e648 Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This commit is contained in:
parent
3dd3fe9ea4
commit
78b18dbb83
|
@ -8,8 +8,8 @@ Bundle-Localization: plugin
|
|||
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
|
||||
Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
|
||||
javax.servlet.http;version="[2.5.0,3.2.0)",
|
||||
org.apache.commons.codec;version="[1.6.0, 2.0.0)",
|
||||
org.apache.commons.codec.binary;version="[1.6.0, 2.0.0)",
|
||||
org.apache.commons.codec;version="[1.6.0,2.0.0)",
|
||||
org.apache.commons.codec.binary;version="[1.6.0,2.0.0)",
|
||||
org.eclipse.jetty.continuation;version="[9.4.5,10.0.0)",
|
||||
org.eclipse.jetty.http;version="[9.4.5,10.0.0)",
|
||||
org.eclipse.jetty.io;version="[9.4.5,10.0.0)",
|
||||
|
@ -44,5 +44,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
|
|||
org.eclipse.jgit.util;version="[4.9.8,4.10.0)",
|
||||
org.hamcrest.core;version="[1.1.0,2.0.0)",
|
||||
org.junit;version="[4.0.0,5.0.0)",
|
||||
org.junit.rules;version="[4.0.0,5.0.0)",
|
||||
org.junit.runner;version="[4.0.0,5.0.0)",
|
||||
org.junit.runners;version="[4.0.0,5.0.0)"
|
||||
Require-Bundle: org.hamcrest.library;bundle-version="[1.1.0,2.0.0)"
|
||||
|
|
|
@ -70,6 +70,13 @@
|
|||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
<dependency>
|
||||
<groupId>org.hamcrest</groupId>
|
||||
<artifactId>hamcrest-library</artifactId>
|
||||
<scope>test</scope>
|
||||
<version>[1.1.0,2.0.0)</version>
|
||||
</dependency>
|
||||
|
||||
<dependency>
|
||||
<groupId>org.eclipse.jgit</groupId>
|
||||
<artifactId>org.eclipse.jgit</artifactId>
|
||||
|
|
|
@ -85,6 +85,7 @@
|
|||
import org.eclipse.jgit.errors.TransportException;
|
||||
import org.eclipse.jgit.errors.UnsupportedCredentialItem;
|
||||
import org.eclipse.jgit.http.server.GitServlet;
|
||||
import org.eclipse.jgit.http.server.resolver.DefaultUploadPackFactory;
|
||||
import org.eclipse.jgit.internal.JGitText;
|
||||
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
|
||||
import org.eclipse.jgit.junit.TestRepository;
|
||||
|
@ -105,24 +106,35 @@
|
|||
import org.eclipse.jgit.lib.StoredConfig;
|
||||
import org.eclipse.jgit.revwalk.RevBlob;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.storage.file.FileBasedConfig;
|
||||
import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
|
||||
import org.eclipse.jgit.transport.AdvertiseRefsHook;
|
||||
import org.eclipse.jgit.transport.CredentialItem;
|
||||
import org.eclipse.jgit.transport.CredentialsProvider;
|
||||
import org.eclipse.jgit.transport.FetchConnection;
|
||||
import org.eclipse.jgit.transport.HttpTransport;
|
||||
import org.eclipse.jgit.transport.RefSpec;
|
||||
import org.eclipse.jgit.transport.RemoteRefUpdate;
|
||||
import org.eclipse.jgit.transport.Transport;
|
||||
import org.eclipse.jgit.transport.TransportHttp;
|
||||
import org.eclipse.jgit.transport.URIish;
|
||||
import org.eclipse.jgit.transport.UploadPack;
|
||||
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
|
||||
import org.eclipse.jgit.transport.http.HttpConnectionFactory;
|
||||
import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory;
|
||||
import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory;
|
||||
import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
|
||||
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
||||
import org.eclipse.jgit.transport.resolver.UploadPackFactory;
|
||||
import org.eclipse.jgit.util.FS;
|
||||
import org.eclipse.jgit.util.HttpSupport;
|
||||
import org.eclipse.jgit.util.SystemReader;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.rules.ExpectedException;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.junit.runners.Parameterized;
|
||||
import org.junit.runners.Parameterized.Parameters;
|
||||
|
@ -131,6 +143,11 @@
|
|||
public class SmartClientSmartServerTest extends HttpTestCase {
|
||||
private static final String HDR_TRANSFER_ENCODING = "Transfer-Encoding";
|
||||
|
||||
@Rule
|
||||
public ExpectedException thrown = ExpectedException.none();
|
||||
|
||||
private AdvertiseRefsHook advertiseRefsHook;
|
||||
|
||||
private Repository remoteRepository;
|
||||
|
||||
private CredentialsProvider testCredentials = new UsernamePasswordCredentialsProvider(
|
||||
|
@ -148,7 +165,7 @@ public class SmartClientSmartServerTest extends HttpTestCase {
|
|||
|
||||
private RevBlob A_txt;
|
||||
|
||||
private RevCommit A, B;
|
||||
private RevCommit A, B, unreachableCommit;
|
||||
|
||||
@Parameters
|
||||
public static Collection<Object[]> data() {
|
||||
|
@ -175,6 +192,19 @@ public void setUp() throws Exception {
|
|||
ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true);
|
||||
|
||||
GitServlet gs = new GitServlet();
|
||||
gs.setUploadPackFactory(new UploadPackFactory<HttpServletRequest>() {
|
||||
@Override
|
||||
public UploadPack create(HttpServletRequest req, Repository db)
|
||||
throws ServiceNotEnabledException,
|
||||
ServiceNotAuthorizedException {
|
||||
DefaultUploadPackFactory f = new DefaultUploadPackFactory();
|
||||
UploadPack up = f.create(req, db);
|
||||
if (advertiseRefsHook != null) {
|
||||
up.setAdvertiseRefsHook(advertiseRefsHook);
|
||||
}
|
||||
return up;
|
||||
}
|
||||
});
|
||||
|
||||
ServletContextHandler app = addNormalContext(gs, src, srcName);
|
||||
|
||||
|
@ -200,6 +230,8 @@ public void setUp() throws Exception {
|
|||
B = src.commit().parent(A).add("A_txt", "C").add("B", "B").create();
|
||||
src.update(master, B);
|
||||
|
||||
unreachableCommit = src.commit().add("A_txt", A_txt).create();
|
||||
|
||||
src.update("refs/garbage/a/very/long/ref/name/to/compress", B);
|
||||
}
|
||||
|
||||
|
@ -455,6 +487,56 @@ public void testListRemote_BadName() throws IOException, URISyntaxException {
|
|||
info.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFetchBySHA1() throws Exception {
|
||||
Repository dst = createBareRepository();
|
||||
assertFalse(dst.hasObject(A_txt));
|
||||
|
||||
try (Transport t = Transport.open(dst, remoteURI)) {
|
||||
t.fetch(NullProgressMonitor.INSTANCE,
|
||||
Collections.singletonList(new RefSpec(B.name())));
|
||||
}
|
||||
|
||||
assertTrue(dst.hasObject(A_txt));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFetchBySHA1Unreachable() throws Exception {
|
||||
Repository dst = createBareRepository();
|
||||
assertFalse(dst.hasObject(A_txt));
|
||||
|
||||
try (Transport t = Transport.open(dst, remoteURI)) {
|
||||
thrown.expect(TransportException.class);
|
||||
thrown.expectMessage(Matchers.containsString(
|
||||
"want " + unreachableCommit.name() + " not valid"));
|
||||
t.fetch(NullProgressMonitor.INSTANCE, Collections
|
||||
.singletonList(new RefSpec(unreachableCommit.name())));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFetchBySHA1UnreachableByAdvertiseRefsHook()
|
||||
throws Exception {
|
||||
Repository dst = createBareRepository();
|
||||
assertFalse(dst.hasObject(A_txt));
|
||||
|
||||
advertiseRefsHook = new AbstractAdvertiseRefsHook() {
|
||||
@Override
|
||||
protected Map<String, Ref> getAdvertisedRefs(Repository repository,
|
||||
RevWalk revWalk) {
|
||||
return Collections.emptyMap();
|
||||
}
|
||||
};
|
||||
|
||||
try (Transport t = Transport.open(dst, remoteURI)) {
|
||||
thrown.expect(TransportException.class);
|
||||
thrown.expectMessage(Matchers.containsString(
|
||||
"want " + A.name() + " not valid"));
|
||||
t.fetch(NullProgressMonitor.INSTANCE, Collections
|
||||
.singletonList(new RefSpec(A.name())));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_Small() throws Exception {
|
||||
Repository dst = createBareRepository();
|
||||
|
|
Loading…
Reference in New Issue