From 2f22aa04aa7e83db32466dd88d887acc291784d8 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 2 Jun 2019 19:49:46 +0200 Subject: [PATCH] Bazel: Stop unsigning jgit libraries This change deletes the last GWT related hack in build tool chain: unsigning of JGit libraries to prevent security exception after adding new class to the existing org.eclipse.jgit.diff package. Instead of adding new ReplaceEdit class to this package, move the class to its own gerrit top level package. My understanding was, that we have done unsigning acrobatic to support deserialization and JSON deserialization for GWT UI. Particularly these two classes: * EditDeserializer.java * Edit_JsonSerializer.java Right now I do not see why this couldn't be done in gerrit own package. In any event, we do not intend to backport this change to other stable branches but only apply it to branches where GWT UI was removed. Change-Id: I3be17767ed495bb1b8c5c2ce4ca1574c865b2fba --- java/{org/eclipse => com/google/gerrit}/jgit/BUILD | 0 .../google/gerrit}/jgit/diff/ReplaceEdit.java | 3 ++- java/com/google/gerrit/server/BUILD | 2 +- .../google/gerrit/server/patch/IntraLineDiff.java | 2 +- .../google/gerrit/server/patch/IntraLineLoader.java | 2 +- java/com/google/gerrit/server/restapi/BUILD | 2 +- .../gerrit/server/restapi/change/GetDiff.java | 2 +- javatests/com/google/gerrit/server/BUILD | 2 +- .../gerrit/server/patch/IntraLineLoaderTest.java | 2 +- lib/jgit/jgit.bzl | 3 --- tools/bzl/maven_jar.bzl | 2 -- tools/download_file.py | 13 ------------- 12 files changed, 9 insertions(+), 26 deletions(-) rename java/{org/eclipse => com/google/gerrit}/jgit/BUILD (100%) rename java/{org/eclipse => com/google/gerrit}/jgit/diff/ReplaceEdit.java (93%) diff --git a/java/org/eclipse/jgit/BUILD b/java/com/google/gerrit/jgit/BUILD similarity index 100% rename from java/org/eclipse/jgit/BUILD rename to java/com/google/gerrit/jgit/BUILD diff --git a/java/org/eclipse/jgit/diff/ReplaceEdit.java b/java/com/google/gerrit/jgit/diff/ReplaceEdit.java similarity index 93% rename from java/org/eclipse/jgit/diff/ReplaceEdit.java rename to java/com/google/gerrit/jgit/diff/ReplaceEdit.java index 46681c6473..45bfad2852 100644 --- a/java/org/eclipse/jgit/diff/ReplaceEdit.java +++ b/java/com/google/gerrit/jgit/diff/ReplaceEdit.java @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package org.eclipse.jgit.diff; +package com.google.gerrit.jgit.diff; import java.util.List; +import org.eclipse.jgit.diff.Edit; public class ReplaceEdit extends Edit { private List internalEdit; diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD index e96396926c..a3dc725dae 100644 --- a/java/com/google/gerrit/server/BUILD +++ b/java/com/google/gerrit/server/BUILD @@ -39,6 +39,7 @@ java_library( "//java/com/google/gerrit/index", "//java/com/google/gerrit/index:query_exception", "//java/com/google/gerrit/index/project", + "//java/com/google/gerrit/jgit", "//java/com/google/gerrit/json", "//java/com/google/gerrit/lifecycle", "//java/com/google/gerrit/mail", @@ -55,7 +56,6 @@ java_library( "//java/com/google/gerrit/util/ssl", "//java/com/google/gwtorm", "//java/org/apache/commons/net", - "//java/org/eclipse/jgit", "//lib:args4j", "//lib:autolink", "//lib:automaton", diff --git a/java/com/google/gerrit/server/patch/IntraLineDiff.java b/java/com/google/gerrit/server/patch/IntraLineDiff.java index a1823359e5..1c3d78a91b 100644 --- a/java/com/google/gerrit/server/patch/IntraLineDiff.java +++ b/java/com/google/gerrit/server/patch/IntraLineDiff.java @@ -21,6 +21,7 @@ import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32; import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; +import com.google.gerrit.jgit.diff.ReplaceEdit; import com.google.gerrit.reviewdb.client.CodedEnum; import java.io.IOException; import java.io.InputStream; @@ -32,7 +33,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import org.eclipse.jgit.diff.Edit; -import org.eclipse.jgit.diff.ReplaceEdit; public class IntraLineDiff implements Serializable { static final long serialVersionUID = IntraLineDiffKey.serialVersionUID; diff --git a/java/com/google/gerrit/server/patch/IntraLineLoader.java b/java/com/google/gerrit/server/patch/IntraLineLoader.java index 022fd9e0ac..34ac3d8c83 100644 --- a/java/com/google/gerrit/server/patch/IntraLineLoader.java +++ b/java/com/google/gerrit/server/patch/IntraLineLoader.java @@ -19,6 +19,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.jgit.diff.ReplaceEdit; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; @@ -34,7 +35,6 @@ import java.util.concurrent.TimeoutException; import java.util.regex.Pattern; import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.diff.MyersDiff; -import org.eclipse.jgit.diff.ReplaceEdit; import org.eclipse.jgit.lib.Config; class IntraLineLoader implements Callable { diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD index 58e7be279d..e083402ded 100644 --- a/java/com/google/gerrit/server/restapi/BUILD +++ b/java/com/google/gerrit/server/restapi/BUILD @@ -13,6 +13,7 @@ java_library( "//java/com/google/gerrit/index", "//java/com/google/gerrit/index:query_exception", "//java/com/google/gerrit/index/project", + "//java/com/google/gerrit/jgit", "//java/com/google/gerrit/json", "//java/com/google/gerrit/mail", "//java/com/google/gerrit/metrics", @@ -23,7 +24,6 @@ java_library( "//java/com/google/gerrit/server/logging", "//java/com/google/gerrit/server/util/time", "//java/com/google/gerrit/util/cli", - "//java/org/eclipse/jgit", "//lib:args4j", "//lib:blame-cache", "//lib:gson", diff --git a/java/com/google/gerrit/server/restapi/change/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java index 761500a2c8..d384f736fe 100644 --- a/java/com/google/gerrit/server/restapi/change/GetDiff.java +++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java @@ -40,6 +40,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.jgit.diff.ReplaceEdit; import com.google.gerrit.prettify.common.SparseFileContent; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; @@ -62,7 +63,6 @@ import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.diff.Edit; -import org.eclipse.jgit.diff.ReplaceEdit; import org.kohsuke.args4j.CmdLineException; import org.kohsuke.args4j.CmdLineParser; import org.kohsuke.args4j.NamedOptionDef; diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD index c1df5758a3..a19a48b122 100644 --- a/javatests/com/google/gerrit/server/BUILD +++ b/javatests/com/google/gerrit/server/BUILD @@ -42,6 +42,7 @@ junit_tests( "//java/com/google/gerrit/git", "//java/com/google/gerrit/index", "//java/com/google/gerrit/index:query_exception", + "//java/com/google/gerrit/jgit", "//java/com/google/gerrit/lifecycle", "//java/com/google/gerrit/mail", "//java/com/google/gerrit/metrics", @@ -61,7 +62,6 @@ junit_tests( "//java/com/google/gerrit/server/util/time", "//java/com/google/gerrit/testing:gerrit-test-util", "//java/com/google/gerrit/truth", - "//java/org/eclipse/jgit", "//lib:gson", "//lib:guava", "//lib:guava-retrying", diff --git a/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java b/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java index 0cc4b00786..6c63c5f6a0 100644 --- a/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java +++ b/javatests/com/google/gerrit/server/patch/IntraLineLoaderTest.java @@ -19,11 +19,11 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.gerrit.jgit.diff.ReplaceEdit; import com.google.gerrit.testing.GerritBaseTests; import java.util.List; import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.diff.EditList; -import org.eclipse.jgit.diff.ReplaceEdit; import org.junit.Test; public class IntraLineLoaderTest extends GerritBaseTests { diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl index dfbb291c26..0f52913d16 100644 --- a/lib/jgit/jgit.bzl +++ b/lib/jgit/jgit.bzl @@ -41,14 +41,12 @@ def jgit_maven_repos(): artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS, repository = _JGIT_REPO, sha1 = "dba85014483315fa426259bc1b8ccda9373a624b", - unsign = True, ) maven_jar( name = "jgit-servlet", artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS, repository = _JGIT_REPO, sha1 = "3287341fca859340a00b51cb5dd3b78b8e532b39", - unsign = True, ) maven_jar( name = "jgit-archive", @@ -61,7 +59,6 @@ def jgit_maven_repos(): artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS, repository = _JGIT_REPO, sha1 = "3d9ba7e610d6ab5d08dcb1e4ba448b592a34de77", - unsign = True, ) def jgit_dep(name): diff --git a/tools/bzl/maven_jar.bzl b/tools/bzl/maven_jar.bzl index 821e037687..0ff460b96a 100644 --- a/tools/bzl/maven_jar.bzl +++ b/tools/bzl/maven_jar.bzl @@ -139,8 +139,6 @@ def _maven_jar_impl(ctx): args = [python, script, "-o", binjar_path, "-u", binurl] if ctx.attr.sha1: args.extend(["-v", sha1]) - if ctx.attr.unsign: - args.append("--unsign") for x in ctx.attr.exclude: args.extend(["-x", x]) diff --git a/tools/download_file.py b/tools/download_file.py index 29398e6082..d0fe96d36c 100755 --- a/tools/download_file.py +++ b/tools/download_file.py @@ -81,7 +81,6 @@ opts.add_option('-u', help='URL to download') opts.add_option('-v', help='expected content SHA-1') opts.add_option('-x', action='append', help='file to delete from ZIP') opts.add_option('--exclude_java_sources', action='store_true') -opts.add_option('--unsign', action='store_true') args, _ = opts.parse_args() root_dir = args.o @@ -140,18 +139,6 @@ if args.exclude_java_sources: print('error opening %s: %s' % (cache_ent, err), file=stderr) exit(1) -if args.unsign: - try: - with ZipFile(cache_ent, 'r') as zf: - for n in zf.namelist(): - if (n.endswith('.RSA') - or n.endswith('.SF') - or n.endswith('.LIST')): - exclude.append(n) - except (BadZipfile, LargeZipFile) as err: - print('error opening %s: %s' % (cache_ent, err), file=stderr) - exit(1) - safe_mkdirs(path.dirname(args.o)) if exclude: try: