TransportHttp: make the connection factory configurable

Previously, TransportHttp always used the globally set connection
factory. This is problematic if that global factory is changed in
the middle of a fetch or push operation. Initialize the factory to
use in the constructor, then use that factory for all HTTP requests
made through this transport. Provide a setter and a getter for it
so that client code can customize the factory, if needed, in a
TransportConfigCallback.

Once a factory has been used on a TransportHttp instance it cannot
be changed anymore.

Make the global static factory reference volatile.

Change-Id: I7c6ee16680407d3724e901c426db174a3125ba1c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
Thomas Wolf 2020-11-29 16:22:39 +01:00
parent 312ab4f7f6
commit 224aaa0be7
6 changed files with 114 additions and 2 deletions

View File

@ -28,6 +28,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
org.eclipse.jetty.util.log;version="[9.4.5,10.0.0)",
org.eclipse.jetty.util.security;version="[9.4.5,10.0.0)",
org.eclipse.jetty.util.thread;version="[9.4.5,10.0.0)",
org.eclipse.jgit.api;version="[5.11.0,5.12.0)",
org.eclipse.jgit.errors;version="[5.11.0,5.12.0)",
org.eclipse.jgit.http.server;version="[5.11.0,5.12.0)",
org.eclipse.jgit.http.server.glue;version="[5.11.0,5.12.0)",

View File

@ -23,12 +23,15 @@
import static org.junit.Assert.fail;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Writer;
import java.net.Proxy;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.text.MessageFormat;
import java.util.Collections;
@ -52,6 +55,8 @@
import org.eclipse.jetty.servlet.FilterHolder;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.TransportConfigCallback;
import org.eclipse.jgit.errors.RemoteRepositoryException;
import org.eclipse.jgit.errors.TransportException;
import org.eclipse.jgit.errors.UnsupportedCredentialItem;
@ -91,6 +96,8 @@
import org.eclipse.jgit.transport.URIish;
import org.eclipse.jgit.transport.UploadPack;
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
import org.eclipse.jgit.transport.http.HttpConnection;
import org.eclipse.jgit.transport.http.HttpConnectionFactory;
import org.eclipse.jgit.util.HttpSupport;
import org.eclipse.jgit.util.SystemReader;
import org.junit.Before;
@ -638,6 +645,63 @@ public void testInitialClone_Small() throws Exception {
.getResponseHeader(HDR_CONTENT_TYPE));
}
@Test
public void test_CloneWithCustomFactory() throws Exception {
HttpConnectionFactory globalFactory = HttpTransport
.getConnectionFactory();
HttpConnectionFactory failingConnectionFactory = new HttpConnectionFactory() {
@Override
public HttpConnection create(URL url) throws IOException {
throw new IOException("Should not be reached");
}
@Override
public HttpConnection create(URL url, Proxy proxy)
throws IOException {
throw new IOException("Should not be reached");
}
};
HttpTransport.setConnectionFactory(failingConnectionFactory);
try {
File tmp = createTempDirectory("cloneViaApi");
boolean[] localFactoryUsed = { false };
TransportConfigCallback callback = new TransportConfigCallback() {
@Override
public void configure(Transport transport) {
if (transport instanceof TransportHttp) {
((TransportHttp) transport).setHttpConnectionFactory(
new HttpConnectionFactory() {
@Override
public HttpConnection create(URL url)
throws IOException {
localFactoryUsed[0] = true;
return globalFactory.create(url);
}
@Override
public HttpConnection create(URL url,
Proxy proxy) throws IOException {
localFactoryUsed[0] = true;
return globalFactory.create(url, proxy);
}
});
}
}
};
try (Git git = Git.cloneRepository().setDirectory(tmp)
.setTransportConfigCallback(callback)
.setURI(remoteURI.toPrivateString()).call()) {
assertTrue("Should have used the local HttpConnectionFactory",
localFactoryUsed[0]);
}
} finally {
HttpTransport.setConnectionFactory(globalFactory);
}
}
private void initialClone_Redirect(int nofRedirects, int code)
throws Exception {
initialClone_Redirect(nofRedirects, code, false);

View File

@ -311,6 +311,7 @@ headRequiredToStash=HEAD required to stash local changes
hoursAgo={0} hours ago
httpConfigCannotNormalizeURL=Cannot normalize URL path {0}: too many .. segments
httpConfigInvalidURL=Cannot parse URL from subsection http.{0} in git config; ignored.
httpFactoryInUse=Changing the HTTP connection factory after an HTTP connection has already been opened is not allowed.
hugeIndexesAreNotSupportedByJgitYet=Huge indexes are not supported by jgit, yet
hunkBelongsToAnotherFile=Hunk belongs to another file
hunkDisconnectedFromFile=Hunk disconnected from file

View File

@ -339,6 +339,7 @@ public static JGitText get() {
/***/ public String hoursAgo;
/***/ public String httpConfigCannotNormalizeURL;
/***/ public String httpConfigInvalidURL;
/***/ public String httpFactoryInUse;
/***/ public String hugeIndexesAreNotSupportedByJgitYet;
/***/ public String hunkBelongsToAnotherFile;
/***/ public String hunkDisconnectedFromFile;

View File

@ -26,7 +26,7 @@ public abstract class HttpTransport extends Transport {
*
* @since 3.3
*/
protected static HttpConnectionFactory connectionFactory = new JDKHttpConnectionFactory();
protected static volatile HttpConnectionFactory connectionFactory = new JDKHttpConnectionFactory();
/**
* Get the {@link org.eclipse.jgit.transport.http.HttpConnectionFactory}

View File

@ -75,6 +75,7 @@
import javax.net.ssl.SSLHandshakeException;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.NoRemoteRepositoryException;
import org.eclipse.jgit.errors.NotSupportedException;
@ -95,6 +96,7 @@
import org.eclipse.jgit.transport.HttpAuthMethod.Type;
import org.eclipse.jgit.transport.HttpConfig.HttpRedirectMode;
import org.eclipse.jgit.transport.http.HttpConnection;
import org.eclipse.jgit.transport.http.HttpConnectionFactory;
import org.eclipse.jgit.util.HttpSupport;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
@ -260,6 +262,10 @@ public Transport open(URIish uri, Repository local, String remoteName)
private boolean sslFailure = false;
private HttpConnectionFactory factory;
private boolean factoryUsed;
/**
* All stored cookies bound to this repo (independent of the baseUrl)
*/
@ -282,6 +288,7 @@ public Transport open(URIish uri, Repository local, String remoteName)
sslVerify = http.isSslVerify();
cookieFile = getCookieFileFromConfig(http);
relevantCookies = filterCookies(cookieFile, baseUrl);
factory = HttpTransport.getConnectionFactory();
}
private URL toURL(URIish urish) throws MalformedURLException {
@ -324,6 +331,7 @@ protected void setURI(URIish uri) throws NotSupportedException {
sslVerify = http.isSslVerify();
cookieFile = getCookieFileFromConfig(http);
relevantCookies = filterCookies(cookieFile, baseUrl);
factory = HttpTransport.getConnectionFactory();
}
/**
@ -360,6 +368,42 @@ private FetchConnection getConnection(HttpConnection c, InputStream in,
return (FetchConnection) f;
}
/**
* Sets the {@link HttpConnectionFactory} to be used by this
* {@link TransportHttp} instance.
* <p>
* If no factory is set explicitly, the {@link TransportHttp} instance uses
* the {@link HttpTransport#getConnectionFactory() globally defined
* factory}.
* </p>
*
* @param customFactory
* the {@link HttpConnectionFactory} to use
* @throws IllegalStateException
* if an HTTP/HTTPS connection has already been opened on this
* {@link TransportHttp} instance
* @since 5.11
*/
public void setHttpConnectionFactory(
@NonNull HttpConnectionFactory customFactory) {
if (factoryUsed) {
throw new IllegalStateException(JGitText.get().httpFactoryInUse);
}
factory = customFactory;
}
/**
* Retrieves the {@link HttpConnectionFactory} used by this
* {@link TransportHttp} instance.
*
* @return the {@link HttpConnectionFactory}
* @since 5.11
*/
@NonNull
public HttpConnectionFactory getHttpConnectionFactory() {
return factory;
}
/** {@inheritDoc} */
@Override
public FetchConnection openFetch() throws TransportException,
@ -916,7 +960,8 @@ protected HttpConnection httpOpen(String method, URL u,
}
final Proxy proxy = HttpSupport.proxyFor(proxySelector, u);
HttpConnection conn = connectionFactory.create(u, proxy);
factoryUsed = true;
HttpConnection conn = factory.create(u, proxy);
if (!sslVerify && "https".equals(u.getProtocol())) { //$NON-NLS-1$
HttpSupport.disableSslVerify(conn);