From 328afb242bd869ebe0b79b22c5f885629c2f8656 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 5 Nov 2018 10:00:01 +0900 Subject: [PATCH 01/17] Set version to 2.14.17-SNAPSHOT Change-Id: I837dd515ca2a7e2dc8684ced53c31e7cf0e78d90 --- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- version.bzl | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index 17acd5ff94..95915f0008 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.14.16 + 2.14.17-SNAPSHOT jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 90526de715..2b4e9969fd 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.14.16 + 2.14.17-SNAPSHOT jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index 85dbb34852..06bd0c287e 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.14.16 + 2.14.17-SNAPSHOT jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index 02fa41cdab..a61255911d 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.14.16 + 2.14.17-SNAPSHOT jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index fd3c29f86d..939833dfe4 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.14.16 + 2.14.17-SNAPSHOT war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index 468af16b30..244486cac9 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "2.14.16" +GERRIT_VERSION = "2.14.17-SNAPSHOT" From 79b7ca94761e0f4c60b0fd51d786743960d396b9 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 1 Nov 2018 15:02:38 +0900 Subject: [PATCH 02/17] [CVE-2015-1832] Upgrade Apache Derby to 10.12.1.1 This upgrade fixes CVE-2015-1832 [1]: XML external entity (XXE) vulnerability in the SqlXmlUtil code in Apache Derby before 10.12.1.1, when a Java Security Manager is not in place, allows context-dependent attackers to read arbitrary files or cause a denial of service (resource consumption) via vectors involving XmlVTI and the XML datatype. [1] https://nvd.nist.gov/vuln/detail/CVE-2015-1832 Bug: Issue 9952 Change-Id: I632d3048c21baece089affdd01e2e7782dbaebc6 --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index d5a8c25502..51105628d4 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -750,9 +750,9 @@ maven_jar( maven_jar( name = "derby", - artifact = "org.apache.derby:derby:10.11.1.1", + artifact = "org.apache.derby:derby:10.12.1.1", attach_source = False, - sha1 = "df4b50061e8e4c348ce243b921f53ee63ba9bbe1", + sha1 = "75070c744a8e52a7d17b8b476468580309d5cd09", ) JETTY_VERS = "9.3.17.v20170317" From bb37eef53aa56af392a01a343888628eab0b49ee Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 1 Nov 2018 15:09:26 +0900 Subject: [PATCH 03/17] [CVE-2018-10936] Upgrade postgresql to 42.2.5 This upgrade fixes CVE-2018-10936 [1]: A weakness was found in postgresql-jdbc before version 42.2.5. It was possible to provide an SSL Factory and not check the host name if a host name verifier was not provided to the driver. This could lead to a condition where a man-in-the-middle attacker could masquerade as a trusted server by providing a certificate for the wrong host, as long as it was signed by a trusted CA. [1] https://nvd.nist.gov/vuln/detail/CVE-2018-10936 Bug: Issue 9952 Change-Id: I32972ae466a7876c221e6b678ffddcf3ca5a5a10 --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 51105628d4..70c686b8a8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -832,8 +832,8 @@ maven_jar( maven_jar( name = "postgresql", - artifact = "org.postgresql:postgresql:42.2.4", - sha1 = "dff98730c28a4b3a3263f0cf4abb9a3392f815a7", + artifact = "org.postgresql:postgresql:42.2.5", + sha1 = "951b7eda125f3137538a94e2cbdcf744088ad4c2", ) maven_jar( From fb11ccf5072380899591c05416f557a11cabf6da Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 1 Nov 2018 15:21:49 +0900 Subject: [PATCH 04/17] [CVE-2017-12629] Upgrade Lucene to 5.5.5 This upgrade fixes CVE-2017-12629 [1]. Although this issue only affects Solr, according to the description, it's probably worth upgrading anyway since there have been several bug fixes in the intermediate versions. See the release notes for 5.5.2 [2], 5.5.4 [3] and 5.5.5 [4] for details. Note: there are no bug fixes listed for 5.5.3. The only reason we didn't upgrade to the latest version before is because we had a dependency on Elasticsearch which had a tight coupling with a specific Lucene version. [1] https://nvd.nist.gov/vuln/detail/CVE-2017-12629 [2] https://lucene.apache.org/core/5_5_2/changes/Changes.html#v5.5.2.bug_fixes [3] https://lucene.apache.org/core/5_5_4/changes/Changes.html#v5.5.4.bug_fixes [4] https://lucene.apache.org/core/5_5_5/changes/Changes.html#v5.5.5.bug_fixes Bug: Issue 9952 Change-Id: I776e2dc10c86dc6761a0a1ce6644ce5ac384509c --- WORKSPACE | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 70c686b8a8..7de37b6afb 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -459,36 +459,36 @@ maven_jar( sha1 = "18a9a2ce6abf32ea1b5fd31dae5210ad93f4e5e3", ) -LUCENE_VERS = "5.5.2" +LUCENE_VERS = "5.5.5" maven_jar( name = "lucene-core", artifact = "org.apache.lucene:lucene-core:" + LUCENE_VERS, - sha1 = "de5e5c3161ea01e89f2a09a14391f9b7ed66cdbb", + sha1 = "c34bcd9274859dc07cfed2a935aaca90c4f4b861", ) maven_jar( name = "lucene-analyzers-common", artifact = "org.apache.lucene:lucene-analyzers-common:" + LUCENE_VERS, - sha1 = "f0bc3114a6b43f8e64a33c471d5b9e8ddc51564d", + sha1 = "e6b3f5d1b33ed24da7eef0a72f8062bd4652700c", ) maven_jar( name = "backward-codecs", artifact = "org.apache.lucene:lucene-backward-codecs:" + LUCENE_VERS, - sha1 = "c5cfcd7a8cf48a0144b61fb991c8e50a0bf868d5", + sha1 = "d1dee5c7676a313758adb30d7b0bd4c69a4cd214", ) maven_jar( name = "lucene-misc", artifact = "org.apache.lucene:lucene-misc:" + LUCENE_VERS, - sha1 = "37bbe5a2fb429499dfbe75d750d1778881fff45d", + sha1 = "bc0eb46ba0377594cac7b0cdaab35562d7877521", ) maven_jar( name = "lucene-queryparser", artifact = "org.apache.lucene:lucene-queryparser:" + LUCENE_VERS, - sha1 = "8ac921563e744463605284c6d9d2d95e1be5b87c", + sha1 = "6c965eb5838a2ba58b0de0fd860a420dcda11937", ) maven_jar( From ef976a47fb563bf504ff2e253f9b0e20327a06ff Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Mon, 4 Sep 2017 08:33:26 +0200 Subject: [PATCH 05/17] project/Index: Assign and ignore unused future At Google we have tooling that points out unused futures and demands explicit 'unused' statements to indicate that the return value is ignored by intention. In addition to that, this commit adds a comment about why it is safe to ignore this future. Change-Id: I7651011eef43c02fbf51e86720661ee4928e70af --- .../main/java/com/google/gerrit/server/project/Index.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/Index.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/Index.java index 9ce72fe4c8..ceace1f870 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/Index.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/Index.java @@ -33,6 +33,7 @@ import com.google.gerrit.server.index.change.ChangeIndexer; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.PrintWriter; +import java.util.concurrent.Future; @RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER) @Singleton @@ -59,7 +60,11 @@ public class Index implements RestModifyView { new MultiProgressMonitor(ByteStreams.nullOutputStream(), "Reindexing project") .beginSubTask("", MultiProgressMonitor.UNKNOWN); PrintWriter pw = new PrintWriter(CharStreams.nullWriter()); - executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt, pw)); + // The REST call is just a trigger for async reindexing, so it is safe to ignore the future's + // return value. + @SuppressWarnings("unused") + Future ignored = + executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt, pw)); return Response.accepted("Project " + project + " submitted for reindexing"); } } From ec46d92a772ebdbf4f08d19182d4d7f42300fc3e Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 4 Dec 2017 14:45:35 +0900 Subject: [PATCH 06/17] Stop using CharMatcher.javaLetterOrDigit CharMatcher.javaLetterOrDigit is deprecated in Guava 23.2 [1]. Replace it with regular expression for matching. [1] https://github.com/google/guava/releases/tag/v23.2 Change-Id: Icaf7504e7250beaa92b9c86ad592649bd6fafcba --- .../gerrit/server/config/ListCapabilities.java | 15 +++++++-------- .../google/gerrit/server/project/PutConfig.java | 17 +++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ListCapabilities.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ListCapabilities.java index 52f2e69cbe..b8d1888d3e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ListCapabilities.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ListCapabilities.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.config; -import com.google.common.base.CharMatcher; import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.extensions.config.CapabilityDefinition; import com.google.gerrit.extensions.registration.DynamicMap; @@ -24,6 +23,7 @@ import com.google.inject.Provider; import com.google.inject.Singleton; import java.util.Map; import java.util.TreeMap; +import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +31,8 @@ import org.slf4j.LoggerFactory; @Singleton public class ListCapabilities implements RestReadView { private static final Logger log = LoggerFactory.getLogger(ListCapabilities.class); + private static final Pattern PLUGIN_NAME_PATTERN = Pattern.compile("^[a-zA-Z0-9-]+$"); + private final DynamicMap pluginCapabilities; @Inject @@ -59,10 +61,11 @@ public class ListCapabilities implements RestReadView { private void collectPluginCapabilities(Map output) { for (String pluginName : pluginCapabilities.plugins()) { - if (!isPluginNameSane(pluginName)) { + if (!PLUGIN_NAME_PATTERN.matcher(pluginName).matches()) { log.warn( - "Plugin name {} must match [A-Za-z0-9-]+ to use capabilities;" + " rename the plugin", - pluginName); + "Plugin name '{}' must match '{}' to use capabilities; rename the plugin", + pluginName, + PLUGIN_NAME_PATTERN.pattern()); continue; } for (Map.Entry> entry : @@ -73,10 +76,6 @@ public class ListCapabilities implements RestReadView { } } - private static boolean isPluginNameSane(String pluginName) { - return CharMatcher.javaLetterOrDigit().or(CharMatcher.is('-')).matchesAllOf(pluginName); - } - public static class CapabilityInfo { public String id; public String name; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java index 85dcfbc931..5521316eee 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.project; -import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.gerrit.extensions.api.projects.ConfigInfo; @@ -44,6 +43,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.regex.Pattern; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.slf4j.Logger; @@ -52,6 +52,8 @@ import org.slf4j.LoggerFactory; @Singleton public class PutConfig implements RestModifyView { private static final Logger log = LoggerFactory.getLogger(PutConfig.class); + private static final Pattern PARAMETER_NAME_PATTERN = + Pattern.compile("^[a-zA-Z0-9]+[a-zA-Z0-9-]*$"); private final boolean serverEnableSignedPush; private final Provider metaDataUpdateFactory; @@ -196,8 +198,12 @@ public class PutConfig implements RestModifyView { for (Entry v : e.getValue().entrySet()) { ProjectConfigEntry projectConfigEntry = pluginConfigEntries.get(pluginName, v.getKey()); if (projectConfigEntry != null) { - if (!isValidParameterName(v.getKey())) { - log.warn("Parameter name '{}' must match '^[a-zA-Z0-9]+[a-zA-Z0-9-]*$'", v.getKey()); + if (!PARAMETER_NAME_PATTERN.matcher(v.getKey()).matches()) { + // TODO check why we have this restriction + log.warn( + "Parameter name '{}' must match '{}'", + v.getKey(), + PARAMETER_NAME_PATTERN.pattern()); continue; } String oldValue = cfg.getString(v.getKey()); @@ -287,9 +293,4 @@ public class PutConfig implements RestModifyView { parameterName, pluginName, projectState.getProject().getName())); } } - - private static boolean isValidParameterName(String name) { - return CharMatcher.javaLetterOrDigit().or(CharMatcher.is('-')).matchesAllOf(name) - && !name.startsWith("-"); - } } From b0618f9b9459e7d9fff0e21a8bcc09d86e2c40b4 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 1 Nov 2018 15:46:39 +0900 Subject: [PATCH 07/17] [CVE-2018-10237]: Upgrade guava to 24.1.1-jre This upgrade fixes CVE-2018-10237 [1]: Unbounded memory allocation in Google Guava 11.0 through 24.x before 24.1.1 allows remote attackers to conduct denial of service attacks against servers that depend on this library and deserialize attacker- provided data, because the AtomicDoubleArray class (when serialized with Java serialization) and the CompoundOrdering class (when serialized with GWT serialization) perform eager allocation without appropriate checks on what a client has sent and whether the data size is reasonable. [1] https://nvd.nist.gov/vuln/detail/CVE-2018-10237 This also adds dependency on j2objc-annotations to prevent the following warning during the build: INFO: From Building java/com/google/gerrit/lucene/liblucene.jar (12 source files): warning: unknown enum constant ReflectionSupport$Level.FULL reason: class file for com.google.j2objc.annotations.ReflectionSupport$Level not found Bug: Issue 9952 Change-Id: Iea79ee7d93c4b7c85479b5ec01ee07e19beed611 --- WORKSPACE | 6 ++++++ lib/BUILD | 12 +++++++++++- lib/guava.bzl | 4 ++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 7de37b6afb..ad6b0720f9 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -219,6 +219,12 @@ maven_jar( sha1 = GUAVA_BIN_SHA1, ) +maven_jar( + name = "j2objc", + artifact = "com.google.j2objc:j2objc-annotations:1.1", + sha1 = "ed28ded51a8b1c6b112568def5f4b455e6809019", +) + maven_jar( name = "velocity", artifact = "org.apache.velocity:velocity:1.7", diff --git a/lib/BUILD b/lib/BUILD index 032addcb1e..7fe6e10d7b 100644 --- a/lib/BUILD +++ b/lib/BUILD @@ -75,11 +75,21 @@ java_library( runtime_deps = [":protobuf"], ) +java_library( + name = "j2objc", + data = ["//lib:LICENSE-Apache2.0"], + visibility = ["//visibility:public"], + exports = ["@j2objc//jar"], +) + java_library( name = "guava", data = ["//lib:LICENSE-Apache2.0"], visibility = ["//visibility:public"], - exports = ["@guava//jar"], + exports = [ + ":j2objc", + "@guava//jar", + ], ) java_library( diff --git a/lib/guava.bzl b/lib/guava.bzl index 768b99ea93..0cd3f38629 100644 --- a/lib/guava.bzl +++ b/lib/guava.bzl @@ -1,5 +1,5 @@ -GUAVA_VERSION = "22.0" +GUAVA_VERSION = "24.1.1-jre" -GUAVA_BIN_SHA1 = "3564ef3803de51fb0530a8377ec6100b33b0d073" +GUAVA_BIN_SHA1 = "2e3014320a8005e3f3c1800cb246ed42db8cab81" GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/" From 5980f9d70757e3f8b33a82912d2dd18009a42c07 Mon Sep 17 00:00:00 2001 From: Brandon Weeks Date: Thu, 26 Jul 2018 13:07:12 -0700 Subject: [PATCH 08/17] Adapt PublicKeyStoreTest to work with BouncyCastle 1.60 removePublicKey() operates by removing the chosen key and creating a new PGPPublicKeyRing with the remainder. However, in BC 1.60[1], there is new validation logic that requires the key in slot 0 to be a master key. In the test case, the master key is removed and reinserted, which throws with the new validation. Fix it by removing the subkey before attempting to remove the master key, and then adding it back. [1] https://github.com/bcgit/bc-java/commit/1c3e644933b9e3b394bc7d89ff93badee448652c Change-Id: Iee072294488bfaaa9ff60b5a1a199a01f4c9800d --- .../test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java index c9c0b188d5..e7189da7ad 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.common.collect.Iterators; import com.google.gerrit.gpg.testutil.TestKey; import java.util.ArrayList; import java.util.Arrays; @@ -163,6 +164,8 @@ public class PublicKeyStoreTest { TestKey key5 = validKeyWithSecondUserId(); PGPPublicKeyRing keyRing = key5.getPublicKeyRing(); PGPPublicKey key = keyRing.getPublicKey(); + PGPPublicKey subKey = + keyRing.getPublicKey(Iterators.get(keyRing.getPublicKeys(), 1).getKeyID()); store.add(keyRing); assertEquals(RefUpdate.Result.NEW, store.save(newCommitBuilder())); @@ -171,9 +174,11 @@ public class PublicKeyStoreTest { "Testuser Five ", "foo:myId"); + keyRing = PGPPublicKeyRing.removePublicKey(keyRing, subKey); keyRing = PGPPublicKeyRing.removePublicKey(keyRing, key); key = PGPPublicKey.removeCertification(key, "foo:myId"); keyRing = PGPPublicKeyRing.insertPublicKey(keyRing, key); + keyRing = PGPPublicKeyRing.insertPublicKey(keyRing, subKey); store.add(keyRing); assertEquals(RefUpdate.Result.FAST_FORWARD, store.save(newCommitBuilder())); From 6f97ef16b34c1fd1d22bfcc8e2720c22a209600c Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 2 Nov 2018 09:21:59 +0900 Subject: [PATCH 09/17] [CVE-2018-1000180, CVE-2018-1000613] Upgrade Bouncycastle to 1.60 This upgrade fixes CVE-2018-1000180 [1]: Bouncy Castle BC 1.54 - 1.59, BC-FJA 1.0.0, BC-FJA 1.0.1 and earlier have a flaw in the Low-level interface to RSA key pair generator, specifically RSA Key Pairs generated in low-level API with added certainty may have less M-R tests than expected. This appears to be fixed in versions BC 1.60 beta 4 and later, BC-FJA 1.0.2 and later. and CVE-2018-1000613 [2]: Bouncy Castle Java Cryptography APIs version prior to version 1.60 contains a CWE-470: Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection') vulnerability in XMSS/XMSS^MT private key deserialization that can result in Deserializing an XMSS/XMSS^MT private key can result in the execution of unexpected code. This attack appear to be exploitable via a handcrafted private key can include references to unexpected classes which will be picked up from the class path for the executing application. This vulnerability appears to have been fixed in 1.60 and later. Bouncycastle 1.57 introduced generics in its APIs. Remove the casts and @SuppressWarnings("unchecked") annotations that are not necessary any more. [1] https://nvd.nist.gov/vuln/detail/CVE-2018-1000180 [2] https://nvd.nist.gov/vuln/detail/CVE-2018-1000613 Bug: Issue 9952 Change-Id: I9b25b1568ac7da555de96d82c597b3dff47966c6 --- WORKSPACE | 9 +++++---- .../google/gerrit/acceptance/api/accounts/AccountIT.java | 1 - .../com/google/gerrit/gpg/GerritPublicKeyChecker.java | 1 - .../java/com/google/gerrit/gpg/PublicKeyChecker.java | 1 - .../main/java/com/google/gerrit/gpg/PublicKeyStore.java | 1 - .../main/java/com/google/gerrit/gpg/server/GpgKeys.java | 1 - .../google/gerrit/gpg/GerritPublicKeyCheckerTest.java | 5 ++--- .../java/com/google/gerrit/gpg/PublicKeyStoreTest.java | 1 - .../java/com/google/gerrit/gpg/testutil/TestKey.java | 2 +- 9 files changed, 8 insertions(+), 14 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index ad6b0720f9..e6a53f6c5c 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -585,24 +585,25 @@ maven_jar( sha1 = "bb562ee73f740bb6b2bf7955f97be6b870d9e9f0", ) -BC_VERS = "1.56" +# When updating Bouncy Castle, also update it in bazlets. +BC_VERS = "1.60" maven_jar( name = "bcprov", artifact = "org.bouncycastle:bcprov-jdk15on:" + BC_VERS, - sha1 = "a153c6f9744a3e9dd6feab5e210e1c9861362ec7", + sha1 = "bd47ad3bd14b8e82595c7adaa143501e60842a84", ) maven_jar( name = "bcpg", artifact = "org.bouncycastle:bcpg-jdk15on:" + BC_VERS, - sha1 = "9c3f2e7072c8cc1152079b5c25291a9f462631f1", + sha1 = "13c7a199c484127daad298996e95818478431a2c", ) maven_jar( name = "bcpkix", artifact = "org.bouncycastle:bcpkix-jdk15on:" + BC_VERS, - sha1 = "4648af70268b6fdb24674fb1fd7c1fcc73db1231", + sha1 = "d0c46320fbc07be3a24eb13a56cee4e3d38e0c75", ) # TODO(davido): Remove exlusion of file system provider, when this issue is fixed: diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index e6fd60ddc1..83bb7be88e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -963,7 +963,6 @@ public class AccountIT extends AbstractDaemonTest { assertThat(actual.fingerprint) .named(id) .isEqualTo(Fingerprint.toString(expected.getPublicKey().getFingerprint())); - @SuppressWarnings("unchecked") List userIds = ImmutableList.copyOf(expected.getPublicKey().getUserIDs()); assertThat(actual.userIds).named(id).containsExactlyElementsIn(userIds); assertThat(actual.key).named(id).startsWith("-----BEGIN PGP PUBLIC KEY BLOCK-----\n"); diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java index 8e503ee230..30c6f84706 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java @@ -176,7 +176,6 @@ public class GerritPublicKeyChecker extends PublicKeyChecker { private boolean hasAllowedUserId(PGPPublicKey key, Set allowedUserIds) throws PGPException { - @SuppressWarnings("unchecked") Iterator userIds = key.getUserIDs(); while (userIds.hasNext()) { String userId = userIds.next(); diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java index c0ab26c2ac..70e9a243cb 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java @@ -381,7 +381,6 @@ public class PublicKeyChecker { } List signerResults = new ArrayList<>(); - @SuppressWarnings("unchecked") Iterator userIds = key.getUserIDs(); while (userIds.hasNext()) { String userId = userIds.next(); diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java index 144606a5f0..8ab5fbd33e 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java @@ -398,7 +398,6 @@ public class PublicKeyStore implements AutoCloseable { } public static String keyToString(PGPPublicKey key) { - @SuppressWarnings("unchecked") Iterator it = key.getUserIDs(); return String.format( "%s %s(%s)", diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java index 9f2acd2464..efbec808ba 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java @@ -230,7 +230,6 @@ public class GpgKeys implements ChildCollection { if (key != null) { info.id = PublicKeyStore.keyIdToString(key.getKeyID()); info.fingerprint = Fingerprint.toString(key.getFingerprint()); - @SuppressWarnings("unchecked") Iterator userIds = key.getUserIDs(); info.userIds = ImmutableList.copyOf(userIds); diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index d1bbc5ec37..886fdcd410 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -373,7 +373,7 @@ public class GerritPublicKeyCheckerTest { PGPPublicKeyRing keyRingB = keyB().getPublicKeyRing(); PGPPublicKey keyB = keyRingB.getPublicKey(); - keyB = PGPPublicKey.removeCertification(keyB, (String) keyB.getUserIDs().next()); + keyB = PGPPublicKey.removeCertification(keyB, keyB.getUserIDs().next()); keyRingB = PGPPublicKeyRing.insertPublicKey(keyRingB, keyB); add(keyRingB, addUser("userB")); @@ -391,8 +391,7 @@ public class GerritPublicKeyCheckerTest { List newExtIds = new ArrayList<>(2); newExtIds.add(ExternalId.create(toExtIdKey(kr.getPublicKey()), id)); - @SuppressWarnings("unchecked") - String userId = (String) Iterators.getOnlyElement(kr.getPublicKey().getUserIDs(), null); + String userId = Iterators.getOnlyElement(kr.getPublicKey().getUserIDs(), null); if (userId != null) { String email = PushCertificateIdent.parse(userId).getEmailAddress(); assertThat(email).contains("@"); diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java index e7189da7ad..9a2acff7b0 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java @@ -236,7 +236,6 @@ public class PublicKeyStoreTest { private void assertUserIds(PGPPublicKeyRing keyRing, String... expected) throws Exception { List actual = new ArrayList<>(); - @SuppressWarnings("unchecked") Iterator userIds = store.get(keyRing.getPublicKey().getKeyID()).iterator().next().getPublicKey().getUserIDs(); while (userIds.hasNext()) { diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKey.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKey.java index 420dedfa74..b2ef65d86f 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKey.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKey.java @@ -77,7 +77,7 @@ public class TestKey { } public String getFirstUserId() { - return (String) getPublicKey().getUserIDs().next(); + return getPublicKey().getUserIDs().next(); } public PGPPrivateKey getPrivateKey() throws PGPException { From f904ab9ef318f2bbc35b945463564a2a0499592a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 2 Nov 2018 09:56:50 +0900 Subject: [PATCH 10/17] Upgrade Jetty to 9.3.24.v20180605 to fix several CVEs This upgrade fixes the following CVEs: - CVE-2017-7656 [1]: In Eclipse Jetty, versions 9.2.x and older, 9.3.x (all configurations), and 9.4.x (non-default configuration with RFC2616 compliance enabled), HTTP/0.9 is handled poorly. An HTTP/1 style request line (i.e. method space URI space version) that declares a version of HTTP/0.9 was accepted and treated as a 0.9 request. If deployed behind an intermediary that also accepted and passed through the 0.9 version (but did not act on it), then the response sent could be interpreted by the intermediary as HTTP/1 headers. This could be used to poison the cache if the server allowed the origin client to generate arbitrary content in the response. - CVE-2017-7657 [2]: In Eclipse Jetty, versions 9.2.x and older, 9.3.x (all configurations), and 9.4.x (non-default configuration with RFC2616 compliance enabled), transfer- encoding chunks are handled poorly. The chunk length parsing was vulnerable to an integer overflow. Thus a large chunk size could be interpreted as a smaller chunk size and content sent as chunk body could be interpreted as a pipelined request. If Jetty was deployed behind an intermediary that imposed some authorization and that intermediary allowed arbitrarily large chunks to be passed on unchanged, then this flaw could be used to bypass the authorization imposed by the intermediary as the fake pipelined request would not be interpreted by the intermediary as a request. - CVE-2017-7658 [3]: In Eclipse Jetty Server, versions 9.2.x and older, 9.3.x (all non HTTP/1.x configurations), and 9.4.x (all HTTP/1.x configurations), when presented with two content-length headers, Jetty ignored the second. When presented with a content-length and a chunked encoding header, the content-length was ignored (as per RFC 2616). If an intermediary decided on the shorter length, but still passed on the longer body, then body content could be interpreted by Jetty as a pipelined request. If the intermediary was imposing authorization, the fake pipelined request would bypass that authorization. - CVE-2017-9735 [4]: Jetty through 9.4.x is prone to a timing channel in util/security/Password.java, which makes it easier for remote attackers to obtain access by observing elapsed times before rejection of incorrect passwords. - CVE-2018-12536 [5]: In Eclipse Jetty Server, all 9.x versions, on webapps deployed using default Error Handling, when an intentionally bad query arrives that doesn't match a dynamic url-pattern, and is eventually handled by the DefaultServlet's static file serving, the bad characters can trigger a java.nio.file.InvalidPathException which includes the full path to the base resource directory that the DefaultServlet and/or webapp is using. If this InvalidPathException is then handled by the default Error Handler, the InvalidPathException message is included in the error response, revealing the full server path to the requesting system. [1] https://nvd.nist.gov/vuln/detail/CVE-2017-7656 [2] https://nvd.nist.gov/vuln/detail/CVE-2017-7657 [3] https://nvd.nist.gov/vuln/detail/CVE-2017-7658 [4] https://nvd.nist.gov/vuln/detail/CVE-2017-9735 [5] https://nvd.nist.gov/vuln/detail/CVE-2018-12536 Bug: Issue 9952 Change-Id: I1ebb91406b88289e3803ffb1d5049ea7352c695c --- WORKSPACE | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index e6a53f6c5c..a0cb82c787 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -762,60 +762,60 @@ maven_jar( sha1 = "75070c744a8e52a7d17b8b476468580309d5cd09", ) -JETTY_VERS = "9.3.17.v20170317" +JETTY_VERS = "9.3.24.v20180605" maven_jar( name = "jetty-servlet", artifact = "org.eclipse.jetty:jetty-servlet:" + JETTY_VERS, - sha1 = "ed6986b0d0ca7b9b0f9015c9efb80442e3043a8e", + sha1 = "db09c8e226c07c46dc3d84626fc97955ec6bf8bf", ) maven_jar( name = "jetty-security", artifact = "org.eclipse.jetty:jetty-security:" + JETTY_VERS, - sha1 = "ca52535569445682d42aaa97c7039442719a0507", + sha1 = "dfc4e2169f3dd91954804e7fdff9c4f67c63f385", ) maven_jar( name = "jetty-servlets", artifact = "org.eclipse.jetty:jetty-servlets:" + JETTY_VERS, - sha1 = "6369e945c7da441ac042002e31dbe3ca2068db8f", + sha1 = "189db52691aacab9e13546429583765d143faf81", ) maven_jar( name = "jetty-server", artifact = "org.eclipse.jetty:jetty-server:" + JETTY_VERS, - sha1 = "194e9a02e6ba249ef4a3f4bd56b4993087992299", + sha1 = "0e629740cf0a08b353ec07c35eeab8fd06590041", ) maven_jar( name = "jetty-jmx", artifact = "org.eclipse.jetty:jetty-jmx:" + JETTY_VERS, - sha1 = "2ba3219f6ee2617ca7f1ec7ae87e4b2128a0c1ce", + sha1 = "aaeda444192a42389d2ac17a786329a1b6f4cf68", ) maven_jar( name = "jetty-continuation", artifact = "org.eclipse.jetty:jetty-continuation:" + JETTY_VERS, - sha1 = "63ff8e2716e20b72787a1dbc666022ef6c1f7b1e", + sha1 = "44d7b4a9aef498abef268f3aade92daa459050f6", ) maven_jar( name = "jetty-http", artifact = "org.eclipse.jetty:jetty-http:" + JETTY_VERS, - sha1 = "6c02d728e15d4868486254039c867a1ac3e4a52e", + sha1 = "f3d614a7c82b5ee028df78bdb3cdadb6c3be89bc", ) maven_jar( name = "jetty-io", artifact = "org.eclipse.jetty:jetty-io:" + JETTY_VERS, - sha1 = "756a8cd2a1cbfb84a94973b6332dd3eccd47c0cd", + sha1 = "f12a02ab2cb79eb9c3fa01daf28a58e8ea7cbea9", ) maven_jar( name = "jetty-util", artifact = "org.eclipse.jetty:jetty-util:" + JETTY_VERS, - sha1 = "b8512ab02819de01f0f5a5c6026163041f579beb", + sha1 = "f74fb3f999e658a2ddea397155e20da5b9126b5d", ) maven_jar( From cafa53b95e6d92e8d90e5c7b0c1054c5f47d0007 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 2 Nov 2018 13:33:26 +0900 Subject: [PATCH 11/17] Fix Elasticsearch dependency on httpcore-nio The elasticsearch-rest-client library has an explicit dependency on version 4.4.5 of httpcore-nio [1], but the version provided by Gerrit is tied to the same version 4.4.1 as all the other httpcomponents. Since httpcore-nio is only used by elasticsearch-rest-client, we can safely provide the required version. We can also restrict visibility to the elasticsearch package to prevent accidental usage elsewhere. The same is true for the httpasyncclient component, which we already provide at the correct version as used by elasticsearch-rest-client, so also restrict its visibility in the same way. At the same time, move httpcore-nio and httpasyncclient declarations up the WORKSPACE file adjacent to the other httpcomponents, and add a comment to clarify that they are set at explicit versions for ES. [1] https://search.maven.org/artifact/org.elasticsearch.client/elasticsearch-rest-client/6.4.2/jar Bug: Issue 9969 Change-Id: Id80f92768667541604df0c53235ea81ecdda9c0e --- WORKSPACE | 28 ++++++++++++++++------------ lib/httpcomponents/BUILD | 2 ++ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index a0cb82c787..0b49f1cba1 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -659,6 +659,20 @@ maven_jar( sha1 = "2f8757f5ac5e38f46c794e5229d1f3c522e9b1df", ) +# elasticsearch-rest-client explicitly depends on this version +maven_jar( + name = "httpasyncclient", + artifact = "org.apache.httpcomponents:httpasyncclient:4.1.2", + sha1 = "95aa3e6fb520191a0970a73cf09f62948ee614be", +) + +# elasticsearch-rest-client explicitly depends on this version +maven_jar( + name = "httpcore-nio", + artifact = "org.apache.httpcomponents:httpcore-nio:4.4.5", + sha1 = "f4be009e7505f6ceddf21e7960c759f413f15056", +) + # Test-only dependencies below. maven_jar( @@ -880,6 +894,8 @@ maven_jar( sha1 = "76716d529710fc03d1d429b43e3cedd4419f78d4", ) +# When upgrading elasticsearch-rest-client, also upgrade http-niocore +# and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.4.2", @@ -894,18 +910,6 @@ maven_jar( sha1 = "4b7f0e0dc527fab032e9800ed231080fdc3ac015", ) -maven_jar( - name = "httpasyncclient", - artifact = "org.apache.httpcomponents:httpasyncclient:4.1.2", - sha1 = "95aa3e6fb520191a0970a73cf09f62948ee614be", -) - -maven_jar( - name = "httpcore-nio", - artifact = "org.apache.httpcomponents:httpcore-nio:" + HTTPCOMP_VERS, - sha1 = "a8c5e3c3bfea5ce23fb647c335897e415eb442e3", -) - maven_jar( name = "testcontainers", artifact = "org.testcontainers:testcontainers:1.8.0", diff --git a/lib/httpcomponents/BUILD b/lib/httpcomponents/BUILD index 8e9fbc57ce..295a15d106 100644 --- a/lib/httpcomponents/BUILD +++ b/lib/httpcomponents/BUILD @@ -37,11 +37,13 @@ java_library( java_library( name = "httpasyncclient", data = ["//lib:LICENSE-Apache2.0"], + visibility = ["//gerrit-elasticsearch:__pkg__"], exports = ["@httpasyncclient//jar"], ) java_library( name = "httpcore-nio", data = ["//lib:LICENSE-Apache2.0"], + visibility = ["//gerrit-elasticsearch:__pkg__"], exports = ["@httpcore-nio//jar"], ) From 0a154a41bb0199a3742988eb200a71ffa435fe8b Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 2 Nov 2018 13:53:30 +0900 Subject: [PATCH 12/17] Remove unused dependency on httpcomponents:httpmime The only consumer of httpmime was Apache Solr, which was removed in change Ic62ae3103 more than 3 years ago. Change-Id: I6204dc74cc99a878ac4cac7c777a235544f9f80e --- WORKSPACE | 6 ------ lib/httpcomponents/BUILD | 7 ------- 2 files changed, 13 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 0b49f1cba1..8d02d2363d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -653,12 +653,6 @@ maven_jar( sha1 = "f5aa318bda4c6c8d688c9d00b90681dcd82ce636", ) -maven_jar( - name = "httpmime", - artifact = "org.apache.httpcomponents:httpmime:" + HTTPCOMP_VERS, - sha1 = "2f8757f5ac5e38f46c794e5229d1f3c522e9b1df", -) - # elasticsearch-rest-client explicitly depends on this version maven_jar( name = "httpasyncclient", diff --git a/lib/httpcomponents/BUILD b/lib/httpcomponents/BUILD index 295a15d106..fd34214e6a 100644 --- a/lib/httpcomponents/BUILD +++ b/lib/httpcomponents/BUILD @@ -27,13 +27,6 @@ java_library( exports = ["@httpcore//jar"], ) -java_library( - name = "httpmime", - data = ["//lib:LICENSE-Apache2.0"], - visibility = ["//visibility:public"], - exports = ["@httpmime//jar"], -) - java_library( name = "httpasyncclient", data = ["//lib:LICENSE-Apache2.0"], From 3c7f7e67b59caa4a1c858fb4d53a2cf94a1e8f15 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 6 Nov 2018 17:18:07 +0900 Subject: [PATCH 13/17] dev-contributing: Specify buildifier version 0.17.2 Change-Id: I40c1c54d024f646881c4e2534837c6f189e1debd --- Documentation/dev-contributing.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt index a6fdee8f8e..eab3618e38 100644 --- a/Documentation/dev-contributing.txt +++ b/Documentation/dev-contributing.txt @@ -155,7 +155,7 @@ To format Java source code, Gerrit uses the link:https://github.com/google/google-java-format[`google-java-format`] tool (version 1.5), and to format Bazel BUILD, WORKSPACE and .bzl files the link:https://github.com/bazelbuild/buildtools/tree/master/buildifier[`buildifier`] -tool (version 0.15.0). +tool (version 0.17.2). These tools automatically apply format according to the style guides; this streamlines code review by reducing the need for time-consuming, tedious, and contentious discussions about trivial issues like whitespace. From c893c02b32a9be96eb5f5d2665fd273fa8853a26 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 20 Dec 2017 08:14:27 -0500 Subject: [PATCH 14/17] AbstractChangeNotes: Never open repo when NoteDb is off This line has seen some action before in I1867f633, and before that in I265ef862. Rereading those commit messages, I see no reason why we should be opening the repo and possibly triggering auto-rebuilding when NoteDb writes and reads are both disabled. This was causing NoSuchChangeExceptions in the case where the Change contained a noteDbState field but the ref was absent. That logic is also questionable, for reasons now mentioned in a TODO. But really we should never be reaching the openHandle method when NoteDb is completely off. Add a regression test that would have caught this. Change-Id: If0970e1cf61e1d98ccbb3ce27549186f5771466a --- .../server/notedb/ChangeRebuilderIT.java | 22 +++++++++++++++++++ .../server/notedb/AbstractChangeNotes.java | 5 ++++- .../gerrit/server/notedb/ChangeNotes.java | 3 +++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index dc6a933a54..ed9cd90886 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -1275,6 +1275,28 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { assertThat(newPs3.getCreatedOn()).isGreaterThan(ps1.getCreatedOn()); } + @Test + public void ignoreNoteDbStateWithNoCorrespondingRefWhenWritesAndReadsDisabled() throws Exception { + PushOneCommit.Result r = createChange(); + Change.Id id = r.getChange().getId(); + ReviewDb db = getUnwrappedDb(); + Change c = db.changes().get(id); + c.setNoteDbState("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + db.changes().update(Collections.singleton(c)); + c = db.changes().get(id); + + String refName = RefNames.changeMetaRef(id); + assertThat(getMetaRef(project, refName)).isNull(); + + ChangeNotes notes = notesFactory.create(dbProvider.get(), project, id); + assertThat(notes.getChange().getRowVersion()).isEqualTo(c.getRowVersion()); + + notes = notesFactory.createChecked(dbProvider.get(), project, id); + assertThat(notes.getChange().getRowVersion()).isEqualTo(c.getRowVersion()); + + assertThat(getMetaRef(project, refName)).isNull(); + } + private void assertChangesReadOnly(RestApiException e) throws Exception { Throwable cause = e.getCause(); assertThat(cause).isInstanceOf(UpdateException.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java index 4cb570a422..7b19b39427 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java @@ -144,7 +144,10 @@ public abstract class AbstractChangeNotes { throw new OrmException("NoteDb is required to read change " + changeId); } boolean readOrWrite = read || args.migration.writeChanges(); - if (!readOrWrite && !autoRebuild) { + if (!readOrWrite) { + // Don't even open the repo if we neither write to nor read from NoteDb. It's possible that + // there is some garbage in the noteDbState field and/or the repo, but at this point NoteDb is + // completely off so it's none of our business. loadDefaults(); return self(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index d821a78661..9582fb372f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -604,6 +604,9 @@ public class ChangeNotes extends AbstractChangeNotes { if (state == null) { return super.openHandle(repo, id); } else if (shouldExist) { + // TODO(dborowitz): This means we have a state recorded in noteDbState but the ref doesn't + // exist for whatever reason. Doesn't this mean we should trigger an auto-rebuild, rather + // than throwing? throw new NoSuchChangeException(getChangeId()); } } From efcb9927a2a4cbcc5d65fec2355e620f9fc0a0ed Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 6 Nov 2018 21:12:58 +0100 Subject: [PATCH 15/17] Bazel: Consume versions directly from lib:versions.bzl in skylib Avoid loading versions from @bazel_skylib//:lib.bzl, because it is now deprecated and is going to be removed in future skylib versions: https://github.com/bazelbuild/bazel-skylib/blob/master/lib.bzl#L17 Change-Id: Idfd131cb1d773887bbc07faadd168da53524fdc0 --- WORKSPACE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WORKSPACE b/WORKSPACE index 8d02d2363d..d03c23d9ad 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -29,7 +29,7 @@ http_file( urls = ["https://raw.githubusercontent.com/google/closure-compiler/775609aad61e14aef289ebec4bfc09ad88877f9e/contrib/externs/polymer-1.0.js"], ) -load("@bazel_skylib//:lib.bzl", "versions") +load("@bazel_skylib//lib:versions.bzl", "versions") versions.check(minimum_bazel_version = "0.17.1") From 481bf8500f634c6927200e17b387889e485d9e03 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 7 Nov 2018 11:43:24 +0900 Subject: [PATCH 16/17] Upgrade elasticsearch-rest-client to 6.4.3 Change-Id: Iac9306a0cfd19e13226a58437536d0297642aca8 --- WORKSPACE | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 8d02d2363d..01800c21a5 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -892,8 +892,8 @@ maven_jar( # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.4.2", - sha1 = "a2baf2d4fdf03f31fbd39351a32bee25fcdfa1cf", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.4.3", + sha1 = "5c24325430971ba2fa4769eb446f026b7680d5e7", ) JACKSON_VERSION = "2.9.7" From fceff73b6f567383738bc6950b35b07c0a315f2a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 7 Nov 2018 11:55:52 +0900 Subject: [PATCH 17/17] ElasticContainer: Test with versions 5.6.13 and 6.4.3 Change-Id: I242972a06800798b6ad449109d765ac21f27d8b5 --- .../google/gerrit/elasticsearch/testing/ElasticContainer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticContainer.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticContainer.java index e347f75a03..0b6e3eeaae 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticContainer.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/testing/ElasticContainer.java @@ -46,13 +46,13 @@ public class ElasticContainer> extends Gener case V2_4: return "elasticsearch:2.4.6-alpine"; case V5_6: - return "docker.elastic.co/elasticsearch/elasticsearch:5.6.12"; + return "docker.elastic.co/elasticsearch/elasticsearch:5.6.13"; case V6_2: return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.2.4"; case V6_3: return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.2"; case V6_4: - return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.2"; + return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.3"; } throw new IllegalStateException("No tests for version: " + version.name()); }