Merge branch 'stable-3.1' into stable-3.2
* stable-3.1:
Restore existing Base64 transcoder for private key
Remove unused annotation @ChangeUpdateExecutor
Change-Id: I55f0668b1131b945ae3c7325a9b6b27b3ad9b51c
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index ef3f415..1c46ed6 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -107,6 +107,7 @@
"//lib/auto:auto-value-annotations",
"//lib/bouncycastle:bcpkix-neverlink",
"//lib/bouncycastle:bcprov-neverlink",
+ "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:dbcp",
"//lib/commons:lang",
diff --git a/java/com/google/gerrit/server/config/ChangeUpdateExecutor.java b/java/com/google/gerrit/server/config/ChangeUpdateExecutor.java
deleted file mode 100644
index 4c9e5f0..0000000
--- a/java/com/google/gerrit/server/config/ChangeUpdateExecutor.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (C) 2012 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 com.google.gerrit.server.config;
-
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.gerrit.server.update.BatchUpdate;
-import com.google.inject.BindingAnnotation;
-import java.lang.annotation.Retention;
-
-/**
- * Marker on the global {@link ListeningExecutorService} used by asynchronous {@link BatchUpdate}s.
- */
-@Retention(RUNTIME)
-@BindingAnnotation
-public @interface ChangeUpdateExecutor {}
diff --git a/java/com/google/gerrit/server/config/SysExecutorModule.java b/java/com/google/gerrit/server/config/SysExecutorModule.java
index 6c729d9..e7f4540 100644
--- a/java/com/google/gerrit/server/config/SysExecutorModule.java
+++ b/java/com/google/gerrit/server/config/SysExecutorModule.java
@@ -14,19 +14,13 @@
package com.google.gerrit.server.config;
-import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.git.WorkQueue;
-import com.google.gerrit.server.logging.LoggingContextAwareExecutorService;
import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.Singleton;
-import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
/**
@@ -73,28 +67,4 @@
}
return queues.createQueue(poolSize, "FanOut");
}
-
- @Provides
- @Singleton
- @ChangeUpdateExecutor
- public ListeningExecutorService createChangeUpdateExecutor(@GerritServerConfig Config config) {
- int poolSize = config.getInt("receive", null, "changeUpdateThreads", 1);
- if (poolSize <= 1) {
- return MoreExecutors.newDirectExecutorService();
- }
- return MoreExecutors.listeningDecorator(
- new LoggingContextAwareExecutorService(
- MoreExecutors.getExitingExecutorService(
- new ThreadPoolExecutor(
- 1,
- poolSize,
- 10,
- TimeUnit.MINUTES,
- new ArrayBlockingQueue<>(poolSize),
- new ThreadFactoryBuilder()
- .setNameFormat("ChangeUpdate-%d")
- .setDaemon(true)
- .build(),
- new ThreadPoolExecutor.CallerRunsPolicy()))));
- }
}
diff --git a/java/com/google/gerrit/server/mail/SignedToken.java b/java/com/google/gerrit/server/mail/SignedToken.java
index a010e30..7dcac1a 100644
--- a/java/com/google/gerrit/server/mail/SignedToken.java
+++ b/java/com/google/gerrit/server/mail/SignedToken.java
@@ -22,6 +22,7 @@
import javax.crypto.Mac;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.SecretKeySpec;
+import org.apache.commons.codec.binary.Base64;
/**
* Utility function to compute and verify XSRF tokens.
@@ -46,7 +47,7 @@
public static String generateRandomKey() {
final byte[] r = new byte[26];
new SecureRandom().nextBytes(r);
- return encodeBase64(r);
+ return encodeBase64PrivateKey(r);
}
private final int maxAge;
@@ -63,7 +64,7 @@
*/
public SignedToken(final int age, final String keyBase64) throws XsrfException {
maxAge = age > 5 ? age / 5 : age;
- key = new SecretKeySpec(decodeBase64(keyBase64), MAC_ALG);
+ key = new SecretKeySpec(decodeBase64PrivateKey(keyBase64), MAC_ALG);
rng = new SecureRandom();
tokenLength = 2 * INT_SZ + newMac().getMacLength();
}
@@ -169,6 +170,14 @@
return (int) (System.currentTimeMillis() / 5000L);
}
+ private static byte[] decodeBase64PrivateKey(final String privateKeyBase64String) {
+ return Base64.decodeBase64(toBytes(privateKeyBase64String));
+ }
+
+ private static String encodeBase64PrivateKey(final byte[] buf) {
+ return toString(Base64.encodeBase64(buf));
+ }
+
private static byte[] decodeBase64(final String s) {
return BaseEncoding.base64Url().decode(s);
}
@@ -208,4 +217,12 @@
}
return r;
}
+
+ private static String toString(final byte[] b) {
+ final StringBuilder r = new StringBuilder(b.length);
+ for (int i = 0; i < b.length; i++) {
+ r.append((char) b[i]);
+ }
+ return r.toString();
+ }
}
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index af9d8c3..baccbf9 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -18,7 +18,6 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
-import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
@@ -48,7 +47,6 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
-import com.google.gerrit.server.config.ChangeUpdateExecutor;
import com.google.gerrit.server.config.DefaultUrlFormatter;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
@@ -186,9 +184,6 @@
bind(GitRepositoryManager.class).to(InMemoryRepositoryManager.class);
bind(InMemoryRepositoryManager.class).in(SINGLETON);
bind(TrackingFooters.class).toProvider(TrackingFootersProvider.class).in(SINGLETON);
- bind(ListeningExecutorService.class)
- .annotatedWith(ChangeUpdateExecutor.class)
- .toInstance(MoreExecutors.newDirectExecutorService());
bind(SecureStore.class).to(DefaultSecureStore.class);
install(new InMemorySchemaModule());
diff --git a/javatests/com/google/gerrit/server/mail/SignedTokenTest.java b/javatests/com/google/gerrit/server/mail/SignedTokenTest.java
index cfb551f..41d8d69 100644
--- a/javatests/com/google/gerrit/server/mail/SignedTokenTest.java
+++ b/javatests/com/google/gerrit/server/mail/SignedTokenTest.java
@@ -18,20 +18,14 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import java.util.Random;
+import java.util.regex.Pattern;
import org.junit.Before;
import org.junit.Test;
public class SignedTokenTest {
- private static final String REGISTER_EMAIL_PRIVATE_KEY =
- "R2Vycml0JTIwcmVnaXN0ZXJFbWFpbFByaXZhdGVLZXk=";
- private static final String URL_SAFE_REGISTER_EMAIL_PRIVATE_KEY =
- REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R2", "_-");
- private static final String URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_PLUS =
- REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R", "+");
- private static final String URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_SLASH =
- REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R", "/");
-
+ private static final Pattern URL_UNSAFE_CHARS = Pattern.compile("(\\+|/)");
+ private static final String REGISTER_EMAIL_PRIVATE_KEY = "TGMv3/bTC42jUKQndTQrXyHhHYMP0t69i/4=";
private static final int maxAge = 5;
private static final String TEXT = "This is a text";
private static final String FORGED_TEXT = "This is a forged text";
@@ -44,29 +38,23 @@
signedToken = new SignedToken(maxAge, REGISTER_EMAIL_PRIVATE_KEY);
}
- /**
- * Test new token: the key is a normal BASE64 string without index of '62'(+ or _) or '63'(/ or -)
- */
+ /** Test new token: the key is a normal BASE64 string that can be used for URL safely */
@Test
public void newTokenKeyDoesNotContainUnsafeChar() throws Exception {
- new SignedToken(maxAge, REGISTER_EMAIL_PRIVATE_KEY);
- }
-
- /** Test new token: the key is an URL safe BASE64 string with indexes of '62'(_) and '63'(-) */
- @Test
- public void newTokenWithUrlSafeBase64() throws Exception {
- new SignedToken(maxAge, URL_SAFE_REGISTER_EMAIL_PRIVATE_KEY);
+ assertThat(signedToken.newToken(TEXT)).doesNotContainMatch(URL_UNSAFE_CHARS);
}
/** Test new token: the key is an URL unsafe BASE64 string with index of '62'(+) */
@Test
public void newTokenWithUrlUnsafeBase64Plus() throws Exception {
- IllegalArgumentException thrown =
- assertThrows(
- IllegalArgumentException.class,
- () -> new SignedToken(maxAge, URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_PLUS));
+ String token = "+" + signedToken.newToken(TEXT);
+ CheckTokenException thrown =
+ assertThrows(CheckTokenException.class, () -> signedToken.checkToken(token, TEXT));
+
+ assertThat(thrown).hasMessageThat().contains("decoding failed");
assertThat(thrown)
+ .hasCauseThat()
.hasMessageThat()
.isEqualTo(
"com.google.common.io.BaseEncoding$DecodingException: Unrecognized character: +");
@@ -75,12 +63,14 @@
/** Test new token: the key is an URL unsafe BASE64 string with '63'(/) */
@Test
public void newTokenWithUrlUnsafeBase64Slash() throws Exception {
- IllegalArgumentException thrown =
- assertThrows(
- IllegalArgumentException.class,
- () -> new SignedToken(maxAge, URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_SLASH));
+ String token = "/" + signedToken.newToken(TEXT);
+ CheckTokenException thrown =
+ assertThrows(CheckTokenException.class, () -> signedToken.checkToken(token, TEXT));
+
+ assertThat(thrown).hasMessageThat().contains("decoding failed");
assertThat(thrown)
+ .hasCauseThat()
.hasMessageThat()
.isEqualTo(
"com.google.common.io.BaseEncoding$DecodingException: Unrecognized character: /");
