Don't shade org.eclipse.jgit.transport package
Since JGit 3.4 release TransportHttp supports setting additional HTTP
headers. We can use this extension API to avoid subclassing that class
and set header for bearer token authentication.
Given that the ctor of TransportHttp is package-private the eclipse
transport package was shaded. This introduced access violation problem
when different classloaders were involved. JGit library was loaded by
gerrit core classloader, but the pull-replication plugin was loaded by
plugin's own classloader.
Change-Id: I5fefcfd0e4317e5a082747aa0c8e71a8e64d4b75
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
index 816d101..1845230 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/JGitFetch.java
@@ -18,6 +18,7 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.replication.pull.transport.TransportProvider;
import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;
diff --git a/src/main/java/org/eclipse/jgit/transport/TransportProvider.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProvider.java
similarity index 67%
rename from src/main/java/org/eclipse/jgit/transport/TransportProvider.java
rename to src/main/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProvider.java
index ca15b6c..c52db82 100644
--- a/src/main/java/org/eclipse/jgit/transport/TransportProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProvider.java
@@ -12,8 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package org.eclipse.jgit.transport;
+package com.googlesource.gerrit.plugins.replication.pull.transport;
+import static org.eclipse.jgit.util.HttpSupport.HDR_AUTHORIZATION;
+
+import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.CredentialsFactory;
@@ -23,10 +26,15 @@
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.errors.TransportException;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.RemoteConfig;
+import org.eclipse.jgit.transport.Transport;
+import org.eclipse.jgit.transport.TransportHttp;
+import org.eclipse.jgit.transport.URIish;
/**
- * This class is responsible for providing a Custom Git HTTP Transport with Bearer Token
- * Authentication or a concrete implementation of {@link org.eclipse.jgit.transport.Transport}.
+ * This class is responsible for setting bearer token header for Bearer Token Authentication using
+ * {@link org.eclipse.jgit.transport.TransportHttp#setAdditionalHeaders(java.util.Map)} method.
*/
@Singleton
public class TransportProvider {
@@ -46,23 +54,14 @@
public Transport open(Repository local, URIish uri)
throws NotSupportedException, TransportException {
- return (bearerToken.isPresent() && TransportHttpWithBearerToken.canHandle(uri))
- ? provideTransportHttpWithBearerToken(local, uri)
- : provideNativeTransport(local, uri);
- }
-
- private Transport provideTransportHttpWithBearerToken(Repository local, URIish uri)
- throws NotSupportedException {
- Transport tn = new TransportHttpWithBearerToken(local, uri, bearerToken.get());
- tn.applyConfig(remoteConfig);
- return tn;
- }
-
- private Transport provideNativeTransport(Repository local, URIish uri)
- throws NotSupportedException, TransportException {
Transport tn = Transport.open(local, uri);
tn.applyConfig(remoteConfig);
- tn.setCredentialsProvider(credentialsProvider);
+ if (tn instanceof TransportHttp && bearerToken.isPresent()) {
+ ((TransportHttp) tn)
+ .setAdditionalHeaders(ImmutableMap.of(HDR_AUTHORIZATION, "Bearer " + bearerToken.get()));
+ } else {
+ tn.setCredentialsProvider(credentialsProvider);
+ }
return tn;
}
}
diff --git a/src/main/java/org/eclipse/jgit/transport/TransportHttpWithBearerToken.java b/src/main/java/org/eclipse/jgit/transport/TransportHttpWithBearerToken.java
deleted file mode 100644
index 68ff6f8..0000000
--- a/src/main/java/org/eclipse/jgit/transport/TransportHttpWithBearerToken.java
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (C) 2022 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package org.eclipse.jgit.transport;
-
-import static org.eclipse.jgit.util.HttpSupport.HDR_AUTHORIZATION;
-
-import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSet;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.net.URL;
-import java.util.Collection;
-import java.util.Set;
-import org.eclipse.jgit.errors.NotSupportedException;
-import org.eclipse.jgit.lib.ProgressMonitor;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.transport.http.HttpConnection;
-
-/**
- * This is a hack in order to allow git over http/https with Bearer Token Authentication.
- *
- * <p>Currently {@link org.eclipse.jgit.transport.TransportHttp} does NOT provide Bearer Token
- * Authentication and unfortunately it is not possible to extend the functionality because some
- * classes, methods and instance variables are private or protected. This package in the pull
- * replication plugin provides the visibility needed and allows to extend the functionality.
- *
- * <p>It is important to mention that in the case of git push operation, this class cannot be used
- * because the push operation needs to initialise a push hook: {@link
- * org.eclipse.jgit.transport.Transport#push(ProgressMonitor, Collection, OutputStream)} and it is
- * defined as a private in the code.
- */
-public class TransportHttpWithBearerToken extends TransportHttp {
-
- private static final String SCHEME_HTTP = "http";
- private static final String SCHEME_HTTPS = "https";
- private static final Set<String> SCHEMES_ALLOWED = ImmutableSet.of(SCHEME_HTTP, SCHEME_HTTPS);
- private final String bearerToken;
-
- public TransportHttpWithBearerToken(Repository local, URIish uri, String bearerToken)
- throws NotSupportedException {
- super(local, uri);
- this.bearerToken = bearerToken;
- }
-
- protected HttpConnection httpOpen(String method, URL u, AcceptEncoding acceptEncoding)
- throws IOException {
- HttpConnection conn = super.httpOpen(method, u, acceptEncoding);
- conn.setRequestProperty(HDR_AUTHORIZATION, "Bearer " + bearerToken); // $NON-NLS-1$
- return conn;
- }
-
- /**
- * This method copies the behaviour of {@link
- * org.eclipse.jgit.transport.TransportProtocol#canHandle(URIish, Repository, String)} in the case
- * of {@link org.eclipse.jgit.transport.TransportHttp} where scheme, host and path are compulsory.
- */
- public static boolean canHandle(URIish uri) {
- return SCHEMES_ALLOWED.contains(uri.getScheme())
- && !Strings.isNullOrEmpty(uri.getHost())
- && !Strings.isNullOrEmpty(uri.getPath());
- }
-}
diff --git a/src/test/java/org/eclipse/jgit/transport/TransportProviderTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProviderTest.java
similarity index 86%
rename from src/test/java/org/eclipse/jgit/transport/TransportProviderTest.java
rename to src/test/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProviderTest.java
index ea70c40..0b28c03 100644
--- a/src/test/java/org/eclipse/jgit/transport/TransportProviderTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/transport/TransportProviderTest.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package org.eclipse.jgit.transport;
+package com.googlesource.gerrit.plugins.replication.pull.transport;
import static com.google.common.truth.Truth.assertThat;
import static org.eclipse.jgit.transport.HttpConfig.EXTRA_HEADER;
@@ -28,6 +28,11 @@
import java.util.Optional;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.transport.RemoteConfig;
+import org.eclipse.jgit.transport.TransferConfig;
+import org.eclipse.jgit.transport.Transport;
+import org.eclipse.jgit.transport.TransportHttp;
+import org.eclipse.jgit.transport.URIish;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -69,7 +74,9 @@
URIish urIish = new URIish("http://some-host/some-path");
Transport transport = transportProvider.open(repository, urIish);
- assertThat(transport).isInstanceOf(TransportHttpWithBearerToken.class);
+
+ // TODO(davido): We cannot access headers to check that the bearer token is set.
+ assertThat(transport).isInstanceOf(TransportHttp.class);
}
@Test
@@ -84,7 +91,8 @@
URIish urIish = new URIish("ssh://some-host/some-path");
Transport transport = transportProvider.open(repository, urIish);
- assertThat(transport).isNotInstanceOf(TransportHttpWithBearerToken.class);
+
+ assertThat(transport).isNotInstanceOf(TransportHttp.class);
}
@Test
@@ -98,6 +106,6 @@
URIish urIish = new URIish("ssh://some-host/some-path");
Transport transport = transportProvider.open(repository, urIish);
- assertThat(transport).isNotInstanceOf(TransportHttpWithBearerToken.class);
+ assertThat(transport).isNotInstanceOf(TransportHttp.class);
}
}
diff --git a/src/test/java/org/eclipse/jgit/transport/TransportHttpWithBearerTokenTest.java b/src/test/java/org/eclipse/jgit/transport/TransportHttpWithBearerTokenTest.java
deleted file mode 100644
index 5bf1047..0000000
--- a/src/test/java/org/eclipse/jgit/transport/TransportHttpWithBearerTokenTest.java
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (C) 2022 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package org.eclipse.jgit.transport;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import java.net.URISyntaxException;
-import org.junit.Test;
-
-public class TransportHttpWithBearerTokenTest {
-
- @Test
- public void cannotHandleURIWhenSchemaIsNeitherHttpNorHttps() throws URISyntaxException {
- URIish uriUnderTest = new URIish("some-uri").setScheme(null);
- boolean result = TransportHttpWithBearerToken.canHandle(uriUnderTest);
- assertThat(result).isFalse();
- }
-
- @Test
- public void cannotHandleURIWhenHostIsNotPresent() throws URISyntaxException {
- URIish uriUnderTest = new URIish("some-uri").setScheme("http").setHost(null);
- boolean result = TransportHttpWithBearerToken.canHandle(uriUnderTest);
- assertThat(result).isFalse();
- }
-
- @Test
- public void cannotHandleURIWhenPathIsNotPresent() throws URISyntaxException {
- URIish uriUnderTest =
- new URIish("some-uri").setScheme("http").setHost("some-host").setPath(null);
- boolean result = TransportHttpWithBearerToken.canHandle(uriUnderTest);
- assertThat(result).isFalse();
- }
-
- @Test
- public void canHandleURIWhenIsWellFormed() throws URISyntaxException {
- URIish uriUnderTest = new URIish("http://some-host/some-path");
- boolean result = TransportHttpWithBearerToken.canHandle(uriUnderTest);
- assertThat(result).isTrue();
- }
-}
