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 4c9e5f034d..0000000000 --- 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 6c729d9073..e7f454056b 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 @@ public class SysExecutorModule extends AbstractModule { } 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 a010e30e8d..7dcac1a996 100644 --- a/java/com/google/gerrit/server/mail/SignedToken.java +++ b/java/com/google/gerrit/server/mail/SignedToken.java @@ -22,6 +22,7 @@ import java.util.Arrays; 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 class SignedToken { 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 class SignedToken { */ 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 @@ public class SignedToken { 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 @@ public class SignedToken { } 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 fa9685e7fd..abb5955ecc 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.common.base.Preconditions.checkState; 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.AnonymousCowardName; 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 @@ public class InMemoryModule extends FactoryModule { 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 cfb551fcfa..41d8d69852 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.common.truth.Truth.assertThat; 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 @@ public class SignedTokenTest { 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 @@ public class SignedTokenTest { /** 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: /");