Merge "Refuse convert-ref-storage to reftable when usePerRequestRefCache is set"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 6efa1db..6521e3b 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -60,8 +60,15 @@
---
[accountPatchReviewDb]
- url = jdbc:h2:file:/path/to/db
+ url = jdbc:h2:async:/path/to/db
---
++
+For legacy configurations, Gerrit also accepts the previous
+`jdbc:h2:file:/path/to/db` form as an alias for `jdbc:h2:async:/path/to/db`.
+H2 documents the `async:` file system as a workaround for applications that
+may interrupt embedded H2 operations, but not as a complete safety guarantee.
+Use a client/server database for the strongest isolation from interrupted
+request threads.
[[accountPatchReviewDb.poolLimit]]accountPatchReviewDb.poolLimit::
@@ -3645,7 +3652,8 @@
`X-Forwarded-Server`. These headers are typically set by Apache's
link:https://httpd.apache.org/docs/2.4/mod/mod_proxy.html#x-headers[mod_proxy,role=external,window=_blank].
+
-[NOTE]
+[[httpd.listenUrl.proxyHttpSecurity]]
+[WARNING]
--
For security reasons, make sure to only allow connections from a
trusted reverse proxy in your network, as clients could otherwise
@@ -3718,6 +3726,27 @@
By default, `\http://*:8080`.
--
+[[httpd.enableProxyProtocol]]httpd.enableProxyProtocol::
++
+If `true`, enables parsing PROXY protocol headers on incoming HTTP connections
+configured with `proxy-http://` or `proxy-https://`. This allows Gerrit to
+access the original client address and port instead of the reverse proxy
+address. A PROXY protocol header is optional; connections without one are
+handled normally.
+If both a PROXY protocol header and `X-Forwarded-For` are present, the
+`X-Forwarded-For` header takes precedence for the client address.
++
+[WARNING]
+This should only be enabled when *all* HTTP traffic on the listener comes
+from trusted proxies. A client that can connect directly to the HTTP
+listener can send a forged PROXY protocol header and spoof its source
+address.
+
+See also <<httpd.listenUrl.proxyHttpSecurity, the proxy-http security warning>>
+for related reverse-proxy security considerations.
++
+By default, `false`.
+
[[httpd.reuseAddress]]httpd.reuseAddress::
+
If `true`, permits the daemon to bind to the port even if the port
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 31a0584..2364f05 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -123,6 +123,11 @@
@BindingAnnotation
public @interface TestSshServerAddress {}
+ /** Marker on {@link InetSocketAddress} for test HTTP server. */
+ @Retention(RUNTIME)
+ @BindingAnnotation
+ public @interface TestHttpServerAddress {}
+
@AutoValue
public abstract static class Description {
public static Description forTestClass(
@@ -655,6 +660,14 @@
? SocketUtil.resolve(cfg.getString("sshd", null, "listenAddress"), 0)
: null;
}
+
+ @Provides
+ @Singleton
+ @TestHttpServerAddress
+ InetSocketAddress getHttpAddress(@GerritServerConfig Config cfg) {
+ URI uri = URI.create(cfg.getString("httpd", null, "listenUrl"));
+ return new InetSocketAddress(uri.getHost(), uri.getPort());
+ }
};
return sysInjector.createChildInjector(module);
}
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index c212f0c..885edc6 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -60,7 +60,6 @@
public static final String SUBMIT = "submit";
public static final String SUBMIT_AS = "submitAs";
public static final String TOGGLE_WORK_IN_PROGRESS_STATE = "toggleWipState";
- public static final String POST_REVIEW_COMMENT = "postReviewComment";
public static final String VIEW_PRIVATE_CHANGES = "viewPrivateChanges";
public static final String AI_REVIEW = "aiReview";
@@ -100,7 +99,6 @@
NAMES_LC.add(SUBMIT.toLowerCase(Locale.US));
NAMES_LC.add(SUBMIT_AS.toLowerCase(Locale.US));
NAMES_LC.add(TOGGLE_WORK_IN_PROGRESS_STATE.toLowerCase(Locale.US));
- NAMES_LC.add(POST_REVIEW_COMMENT.toLowerCase(Locale.US));
NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase(Locale.US));
NAMES_LC.add(AI_REVIEW.toLowerCase(Locale.US));
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
index 09cad57..faf9f16 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -70,6 +70,8 @@
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.ProxyConnectionFactory;
+import org.eclipse.jetty.server.ProxyCustomizer;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
@@ -104,17 +106,24 @@
* }
* </code>
*
- * What we want to achieve here is to remember what it was the original proxy address before
- * calling super.customize() and give the possibility to fetch it later down the chain.
+ * What we want to achieve here is to remember the original proxy address before calling
+ * super.customize() and give the possibility to fetch it later down the chain. When
+ * PROXY protocol is enabled, ProxyCustomizer has already exposed the underlying socket
+ * peer address through its request attribute.
*/
- request.setAttribute(
- RemoteUserUtil.PROXY_REMOTE_ADDRESS_ATTR,
- ((InetSocketAddress) request.getConnectionMetaData().getRemoteSocketAddress())
- .getAddress()
- .getHostAddress());
+ String proxyRemoteAddress =
+ (String) request.getAttribute(ProxyCustomizer.REMOTE_ADDRESS_ATTRIBUTE_NAME);
+ if (proxyRemoteAddress == null) {
+ proxyRemoteAddress =
+ ((InetSocketAddress) request.getConnectionMetaData().getRemoteSocketAddress())
+ .getAddress()
+ .getHostAddress();
+ }
+ request.setAttribute(RemoteUserUtil.PROXY_REMOTE_ADDRESS_ATTR, proxyRemoteAddress);
return super.customize(request, responseHeaders);
}
};
+ private static final ProxyCustomizer PROXY_CUSTOMIZER = new ProxyCustomizer();
static class Lifecycle implements LifecycleListener {
private final JettyServer server;
@@ -335,11 +344,13 @@
final int acceptors = cfg.getInt("httpd", "acceptorThreads", 0);
final int selectors = cfg.getInt("httpd", "selectorThreads", 2);
final AuthType authType = cfg.getEnum("auth", null, "type", AuthType.OPENID);
+ final boolean enableProxyProtocol = cfg.getBoolean("httpd", null, "enableProxyProtocol", false);
- reverseProxy = isReverseProxied(listenUrls);
+ boolean allReverseProxy = true;
final Connector[] connectors = new Connector[listenUrls.length];
for (int idx = 0; idx < listenUrls.length; idx++) {
final URI u = listenUrls[idx];
+ final String scheme = u.getScheme();
final int defaultPort;
final ServerConnector c;
HttpConfiguration config = defaultConfig(requestHeaderSize);
@@ -371,10 +382,10 @@
UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING,
UriCompliance.Violation.SUSPICIOUS_PATH_CHARACTERS)));
- if (AuthType.CLIENT_SSL_CERT_LDAP.equals(authType) && !"https".equals(u.getScheme())) {
+ if (AuthType.CLIENT_SSL_CERT_LDAP.equals(authType) && !"https".equals(scheme)) {
throw new IllegalArgumentException(
"Protocol '"
- + u.getScheme()
+ + scheme
+ "' "
+ " not supported in httpd.listenurl '"
+ u
@@ -383,11 +394,13 @@
+ "'; only 'https' is supported");
}
- if ("http".equals(u.getScheme())) {
+ if ("http".equals(scheme)) {
+ allReverseProxy = false;
defaultPort = 80;
- c = newServerConnector(server, acceptors, selectors, config);
+ c = newServerConnector(server, acceptors, selectors, config, false);
- } else if ("https".equals(u.getScheme())) {
+ } else if ("https".equals(scheme)) {
+ allReverseProxy = false;
SslContextFactory.Server ssl = new SslContextFactory.Server();
final Path keystore = getFile(cfg, "sslkeystore", "etc/keystore");
String password = cfg.getString("httpd", null, "sslkeypassword");
@@ -423,41 +436,43 @@
new SslConnectionFactory(ssl, "http/1.1"),
new HttpConnectionFactory(config));
- } else if ("proxy-http".equals(u.getScheme())) {
+ } else if ("proxy-http".equals(scheme) || "proxy-https".equals(scheme)) {
defaultPort = 8080;
+ // Add the proxy customizer first so the forwarding customizer can record the underlying
+ // peer address used for authentication before applying X-Forwarded-For to the request.
+ if (enableProxyProtocol) {
+ config.addCustomizer(PROXY_CUSTOMIZER);
+ }
config.addCustomizer(FORWARDED_REQUEST_CUSTOMIZER);
- c = newServerConnector(server, acceptors, selectors, config);
+ if ("proxy-https".equals(scheme)) {
+ // For a proxy that terminates TLS, mark every request as HTTPS
+ // unconditionally. ForwardedRequestCustomizer alone only sets
+ // isSecure() when the proxy sends X-Forwarded-Proto=https or
+ // X-Proxied-Https=on; this wrapper covers proxies that don't.
+ // Jetty 12's HttpConfiguration.Customizer returns a (possibly
+ // wrapped) Request, so wrap the URI's scheme and override isSecure().
+ config.addCustomizer(
+ (request, responseHeaders) ->
+ new Request.Wrapper(request) {
+ @Override
+ public HttpURI getHttpURI() {
+ return HttpURI.build(super.getHttpURI())
+ .scheme(HttpScheme.HTTPS.asString())
+ .asImmutable();
+ }
- } else if ("proxy-https".equals(u.getScheme())) {
- defaultPort = 8080;
- config.addCustomizer(FORWARDED_REQUEST_CUSTOMIZER);
- // For a proxy that terminates TLS, mark every request as HTTPS
- // unconditionally. ForwardedRequestCustomizer alone only sets
- // isSecure() when the proxy sends X-Forwarded-Proto=https or
- // X-Proxied-Https=on; this wrapper covers proxies that don't.
- // Jetty 12's HttpConfiguration.Customizer returns a (possibly
- // wrapped) Request, so wrap the URI's scheme and override isSecure().
- config.addCustomizer(
- (request, responseHeaders) ->
- new Request.Wrapper(request) {
- @Override
- public HttpURI getHttpURI() {
- return HttpURI.build(super.getHttpURI())
- .scheme(HttpScheme.HTTPS.asString())
- .asImmutable();
- }
-
- @Override
- public boolean isSecure() {
- return true;
- }
- });
- c = newServerConnector(server, acceptors, selectors, config);
+ @Override
+ public boolean isSecure() {
+ return true;
+ }
+ });
+ }
+ c = newServerConnector(server, acceptors, selectors, config, enableProxyProtocol);
} else {
throw new IllegalArgumentException(
"Protocol '"
- + u.getScheme()
+ + scheme
+ "' "
+ " not supported in httpd.listenurl '"
+ u
@@ -490,13 +505,30 @@
c.setIdleTimeout(cfg.getTimeUnit("httpd", null, "idleTimeout", 30000L, MILLISECONDS));
connectors[idx] = c;
}
+ reverseProxy = allReverseProxy;
return connectors;
}
- private static ServerConnector newServerConnector(
- Server server, int acceptors, int selectors, HttpConfiguration config) {
- return new ServerConnector(
- server, null, null, null, acceptors, selectors, new HttpConnectionFactory(config));
+ @VisibleForTesting
+ static ServerConnector newServerConnector(
+ Server server,
+ int acceptors,
+ int selectors,
+ HttpConfiguration config,
+ boolean enableProxyProtocol) {
+ HttpConnectionFactory http = new HttpConnectionFactory(config);
+ if (enableProxyProtocol) {
+ return new ServerConnector(
+ server,
+ null,
+ null,
+ null,
+ acceptors,
+ selectors,
+ new ProxyConnectionFactory(http.getProtocol()),
+ http);
+ }
+ return new ServerConnector(server, null, null, null, acceptors, selectors, http);
}
private HttpConfiguration defaultConfig(int requestHeaderSize) {
@@ -520,15 +552,6 @@
return config;
}
- static boolean isReverseProxied(URI[] listenUrls) {
- for (URI u : listenUrls) {
- if ("http".equals(u.getScheme()) || "https".equals(u.getScheme())) {
- return false;
- }
- }
- return true;
- }
-
static URI[] listenURLs(Config cfg) {
String[] urls = cfg.getStringList("httpd", null, "listenurl");
if (urls.length == 0) {
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index 83878fe..a12a2f6 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.ScheduleConfig;
import com.google.gerrit.server.config.ScheduleConfig.Schedule;
+import com.google.gerrit.server.util.H2Url;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -229,7 +230,7 @@
private <V, K> SqlStore<K, V> newSqlStore(PersistentCacheDef<K, V> def, long maxSize) {
String cacheName = def.name() + "-v" + COMPATIBILITY_VERSION;
StringBuilder url = new StringBuilder();
- url.append("jdbc:h2:file:")
+ url.append(H2Url.ASYNC_PREFIX)
.append(cacheDir.resolve(cacheName).toAbsolutePath().toString().replace(";", "\\;"));
if (h2CacheSize >= 0) {
url.append(";CACHE_SIZE=");
diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index b721717..0ea2e5c 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -227,10 +227,6 @@
grant(config, refsFor, Permission.PUSH, defaultUsersGroup);
grant(config, refsFor, Permission.PUSH_MERGE, defaultUsersGroup);
});
-
- config.upsertAccessSection(
- AccessSection.ALL,
- all -> grant(config, all, Permission.POST_REVIEW_COMMENT, defaultUsersGroup));
}
private void initDefaultAclsForServiceUsers(
diff --git a/java/com/google/gerrit/server/schema/GrantPostReviewCommentPermission.java b/java/com/google/gerrit/server/schema/GrantPostReviewCommentPermission.java
deleted file mode 100644
index e9f34fd..0000000
--- a/java/com/google/gerrit/server/schema/GrantPostReviewCommentPermission.java
+++ /dev/null
@@ -1,97 +0,0 @@
-// Copyright (C) 2025 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.schema;
-
-import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.schema.AclUtil.grant;
-
-import com.google.gerrit.entities.AccessSection;
-import com.google.gerrit.entities.GroupReference;
-import com.google.gerrit.entities.Permission;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
-import com.google.gerrit.server.group.SystemGroupBackend;
-import com.google.gerrit.server.project.ProjectConfig;
-import com.google.inject.Inject;
-import java.io.IOException;
-import java.util.concurrent.atomic.AtomicBoolean;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Repository;
-
-/**
- * This class adds the "postReviewComment" permission to all hosts that call this method with the
- * relevant projectName. This class should be called with AllProjects as the project, by all hosts
- * before enabling the "postReviewComment" permission.
- */
-public class GrantPostReviewCommentPermission {
-
- private final GitRepositoryManager repoManager;
- private final ProjectConfig.Factory projectConfigFactory;
- private final SystemGroupBackend systemGroupBackend;
- private final PersonIdent serverUser;
-
- @Inject
- public GrantPostReviewCommentPermission(
- GitRepositoryManager repoManager,
- ProjectConfig.Factory projectConfigFactory,
- SystemGroupBackend systemGroupBackend,
- @GerritPersonIdent PersonIdent serverUser) {
- this.repoManager = repoManager;
- this.projectConfigFactory = projectConfigFactory;
- this.systemGroupBackend = systemGroupBackend;
- this.serverUser = serverUser;
- }
-
- public void execute(Project.NameKey projectName) throws IOException, ConfigInvalidException {
- GroupReference registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS);
- try (Repository repo = repoManager.openRepository(projectName)) {
- MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, repo);
- ProjectConfig projectConfig = projectConfigFactory.read(md);
-
- AtomicBoolean shouldExit = new AtomicBoolean(false);
- projectConfig.upsertAccessSection(
- AccessSection.ALL,
- all -> {
- Permission permissionOnRefsStar =
- all.build().getPermission(Permission.POST_REVIEW_COMMENT);
- if (permissionOnRefsStar != null) {
- if (permissionOnRefsStar.getRule(registeredUsers) == null) {
- // If admins already changed the permission, don't do anything.
- shouldExit.set(true);
- return;
- }
- // permission already exists on refs/* for Registered Users, don't do anything.
- return;
- }
- // If the permission doesn't exist on refs/* for Registered Users, grant it.
- grant(projectConfig, all, Permission.POST_REVIEW_COMMENT, registeredUsers);
- });
-
- if (shouldExit.get()) {
- return;
- }
-
- md.getCommitBuilder().setAuthor(serverUser);
- md.getCommitBuilder().setCommitter(serverUser);
- md.setMessage("Add Post Review Comment permission for all registered users\n");
-
- projectConfig.commit(md);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStore.java b/java/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStore.java
index c66b3b4..5975474 100644
--- a/java/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStore.java
@@ -20,6 +20,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.util.H2Url;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.File;
@@ -41,7 +42,6 @@
@Singleton
public class H2JGitLockAccountPatchReviewStore extends H2CustomLockAccountPatchReviewStore {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static final String H2_DB_URL_PREFIX = "jdbc:h2:file:";
private final File lockTarget;
@Inject
@@ -52,13 +52,15 @@
@VisibleForTesting
static File lockTargetFromUrl(String h2Url) {
- if (!h2Url.startsWith(H2_DB_URL_PREFIX)) {
+ String path;
+ if (h2Url.startsWith(H2Url.ASYNC_PREFIX)) {
+ path = h2Url.substring(H2Url.ASYNC_PREFIX.length());
+ } else if (h2Url.startsWith(H2Url.FILE_PREFIX)) {
+ path = h2Url.substring(H2Url.FILE_PREFIX.length());
+ } else {
throw new IllegalArgumentException("Not a valid H2 file URL: " + h2Url);
}
- // URL format: "jdbc:h2:file:/path/to/db" - where ";" in the path is escaped as "\;"
- String path = h2Url.substring(H2_DB_URL_PREFIX.length());
-
// Split on first unescaped ";" to drop options, then unescape "\;" in the path
return new File(
Iterables.get(Splitter.on(Pattern.compile("(?<!\\\\);")).split(path), 0)
diff --git a/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java b/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
index e1842f6..102d1ec 100644
--- a/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
+++ b/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
+import com.google.gerrit.server.util.H2Url;
import java.nio.file.Path;
import java.sql.Connection;
import java.sql.PreparedStatement;
@@ -153,6 +154,15 @@
if (url == null) {
return createH2Url(sitePaths.db_dir.resolve("account_patch_reviews"));
}
+ url = normalizeH2Url(url);
+ return url;
+ }
+
+ @VisibleForTesting
+ static String normalizeH2Url(String url) {
+ if (url.startsWith(H2Url.FILE_PREFIX)) {
+ return H2Url.ASYNC_PREFIX + url.substring(H2Url.FILE_PREFIX.length());
+ }
return url;
}
@@ -455,7 +465,7 @@
@VisibleForTesting
static String createH2Url(Path path) {
return new StringBuilder()
- .append("jdbc:h2:file:")
+ .append(H2Url.ASYNC_PREFIX)
.append(path.toAbsolutePath().toString())
.toString()
.replace(";", "\\;");
diff --git a/java/com/google/gerrit/server/schema/Schema_186.java b/java/com/google/gerrit/server/schema/Schema_186.java
index de1e315..7b68fb0 100644
--- a/java/com/google/gerrit/server/schema/Schema_186.java
+++ b/java/com/google/gerrit/server/schema/Schema_186.java
@@ -14,11 +14,8 @@
package com.google.gerrit.server.schema;
+/** Retains schema version 186 after reverting the Post Review Comment migration. */
public class Schema_186 implements NoteDbSchemaVersion {
@Override
- public void upgrade(NoteDbSchemaVersion.Arguments args, UpdateUI ui) throws Exception {
- new GrantPostReviewCommentPermission(
- args.repoManager, args.projectConfigFactory, args.systemGroupBackend, args.serverUser)
- .execute(args.allProjects);
- }
+ public void upgrade(NoteDbSchemaVersion.Arguments args, UpdateUI ui) throws Exception {}
}
diff --git a/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java b/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
index ea1e4e4..52c1267 100644
--- a/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
+++ b/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
@@ -56,7 +56,6 @@
return ImmutableList.of(
"[access \"refs/*\"]",
" read = group Administrators",
- " postReviewComment = group " + usersGroupName,
" read = block group Blocked Users",
"[access \"refs/for/*\"]",
" addPatchSet = group " + usersGroupName,
diff --git a/java/com/google/gerrit/server/util/H2Url.java b/java/com/google/gerrit/server/util/H2Url.java
new file mode 100644
index 0000000..575ca9c
--- /dev/null
+++ b/java/com/google/gerrit/server/util/H2Url.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2026 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.util;
+
+public final class H2Url {
+ public static final String ASYNC_PREFIX = "jdbc:h2:async:";
+ public static final String FILE_PREFIX = "jdbc:h2:file:";
+
+ private H2Url() {}
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
index eeeef99..5f29062 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
@@ -71,7 +71,6 @@
import com.google.gerrit.server.group.testing.TestGroupBackend;
import com.google.gerrit.server.permissions.RegexPermissionPolicy;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.schema.GrantPostReviewCommentPermission;
import com.google.gerrit.server.schema.GrantRevertPermission;
import com.google.inject.Inject;
import java.util.Arrays;
@@ -103,7 +102,6 @@
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ExtensionRegistry extensionRegistry;
@Inject private GrantRevertPermission grantRevertPermission;
- @Inject private GrantPostReviewCommentPermission grantPostReviewCommentPermission;
private Project.NameKey newProjectName;
@@ -215,67 +213,6 @@
}
@Test
- public void grantPostReviewCommentPermission() throws Exception {
- String ref = "refs/*";
- String groupId = "global:Registered-Users";
-
- grantPostReviewCommentPermission.execute(newProjectName);
-
- ProjectAccessInfo info = pApi().access();
- assertThat(info.local.containsKey(ref)).isTrue();
- AccessSectionInfo accessSectionInfo = info.local.get(ref);
- assertThat(accessSectionInfo.permissions.containsKey(Permission.POST_REVIEW_COMMENT)).isTrue();
- PermissionInfo permissionInfo =
- accessSectionInfo.permissions.get(Permission.POST_REVIEW_COMMENT);
- assertThat(permissionInfo.rules.containsKey(groupId)).isTrue();
- PermissionRuleInfo permissionRuleInfo = permissionInfo.rules.get(groupId);
- assertThat(permissionRuleInfo.action).isEqualTo(PermissionRuleInfo.Action.ALLOW);
- }
-
- @Test
- public void grantPostReviewCommentPermissionOnlyWorksOnce() throws Exception {
- grantPostReviewCommentPermission.execute(newProjectName);
- grantPostReviewCommentPermission.execute(newProjectName);
-
- try (Repository repo = repoManager.openRepository(newProjectName)) {
- MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
- ProjectConfig projectConfig = projectConfigFactory.read(md);
- AccessSection all = projectConfig.getAccessSection(AccessSection.ALL);
-
- Permission permission = all.getPermission(Permission.POST_REVIEW_COMMENT);
- assertThat(permission.getRules()).hasSize(1);
- }
- }
-
- @Test
- public void grantPostReviewCommentPermissionDoesntOverrideAdminsPreferences() throws Exception {
- GroupReference otherGroup = systemGroupBackend.getGroup(ANONYMOUS_USERS);
-
- try (Repository repo = repoManager.openRepository(newProjectName)) {
- MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
- ProjectConfig projectConfig = projectConfigFactory.read(md);
- projectConfig.upsertAccessSection(
- AccessSection.ALL,
- all -> {
- grant(projectConfig, all, Permission.POST_REVIEW_COMMENT, otherGroup);
- });
- md.getCommitBuilder().setAuthor(admin.newIdent());
- md.getCommitBuilder().setCommitter(admin.newIdent());
- md.setMessage("Add Post Review Comment permission for anonymous users\n");
-
- projectConfig.commit(md);
- }
- projectCache.evict(newProjectName);
- ProjectAccessInfo expected = pApi().access();
-
- grantPostReviewCommentPermission.execute(newProjectName);
- projectCache.evict(newProjectName);
- ProjectAccessInfo actual = pApi().access();
- // Permissions don't change.
- assertThat(actual.local).isEqualTo(expected.local);
- }
-
- @Test
public void getDefaultInheritance() throws Exception {
String inheritedName = pApi().access().inheritsFrom.name;
assertThat(inheritedName).isEqualTo(AllProjectsNameProvider.DEFAULT);
diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/BUILD b/javatests/com/google/gerrit/acceptance/server/httpd/BUILD
index d1a64c0..f05651d 100644
--- a/javatests/com/google/gerrit/acceptance/server/httpd/BUILD
+++ b/javatests/com/google/gerrit/acceptance/server/httpd/BUILD
@@ -1,7 +1,17 @@
+load("@rules_java//java:defs.bzl", "java_library")
load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+java_library(
+ name = "http_test_util",
+ srcs = ["HttpTestUtil.java"],
+ deps = [
+ "//lib:servlet-api",
+ ],
+)
+
acceptance_tests(
srcs = glob(["*IT.java"]),
group = "server_httpd",
labels = ["server"],
+ deps = [":http_test_util"],
)
diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java b/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java
index 1dea800..62dac8f 100644
--- a/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/httpd/HttpLogoutServletIT.java
@@ -22,7 +22,6 @@
import com.google.inject.Inject;
import java.io.IOException;
import java.net.InetAddress;
-import java.net.ServerSocket;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.List;
@@ -47,7 +46,11 @@
Config cfg = new Config();
cfg.setString("auth", null, "logouturl", "/test-logout");
cfg.setString("gerrit", null, "canonicalWebUrl", "https://" + LOCALHOST + ":8443/");
- cfg.setString("httpd", null, "listenUrl", "proxy-https://" + LOCALHOST + ":" + getFreePort());
+ cfg.setString(
+ "httpd",
+ null,
+ "listenUrl",
+ "proxy-https://" + LOCALHOST + ":" + HttpTestUtil.getFreePort());
return cfg;
}
@@ -103,10 +106,4 @@
return Optional.empty();
}
}
-
- private static int getFreePort() throws IOException {
- try (ServerSocket s = new ServerSocket(0)) {
- return s.getLocalPort();
- }
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/HttpTestUtil.java b/javatests/com/google/gerrit/acceptance/server/httpd/HttpTestUtil.java
new file mode 100644
index 0000000..9200c37
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/httpd/HttpTestUtil.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2026 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.acceptance.server.httpd;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public final class HttpTestUtil {
+ static final String REMOTE_ADDRESS_HEADER = "X-Test-Remote-Address";
+
+ private HttpTestUtil() {}
+
+ static int getFreePort() throws IOException {
+ try (ServerSocket s = new ServerSocket(0)) {
+ return s.getLocalPort();
+ }
+ }
+
+ public static final class RemoteAddressFilter implements Filter {
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+ HttpServletResponse httpResponse = (HttpServletResponse) response;
+ httpResponse.setHeader(REMOTE_ADDRESS_HEADER, ((HttpServletRequest) request).getRemoteAddr());
+ chain.doFilter(request, response);
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/httpd/JettyProxyProtocolIT.java b/javatests/com/google/gerrit/acceptance/server/httpd/JettyProxyProtocolIT.java
new file mode 100644
index 0000000..5ebbc32
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/httpd/JettyProxyProtocolIT.java
@@ -0,0 +1,306 @@
+// Copyright (C) 2026 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.acceptance.server.httpd;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.GerritServer.TestHttpServerAddress;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.testing.ConfigSuite;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Assume;
+import org.junit.Test;
+
+public class JettyProxyProtocolIT extends StandaloneSiteTest {
+ private static final String SERVER_ADDRESS = "127.0.0.1";
+ private static final String TRUSTED_PROXY_ADDRESS = "127.0.0.2";
+ private static final String PROXY_SOURCE_ADDRESS = "127.0.0.3";
+ private static final String UNTRUSTED_PEER_ADDRESS = "127.0.0.99";
+ private static final String FORWARDED_CLIENT_ADDRESS = "192.0.2.1";
+ private static final int PROXY_SOURCE_PORT = 12345;
+ private static final byte[] PROXY_PROTOCOL_V2_SIGNATURE = {
+ 0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49, 0x54, 0x0a
+ };
+ private static final String PROXY_PROTOCOL_HEADER_PREFIX =
+ "PROXY TCP4 " + PROXY_SOURCE_ADDRESS + " " + SERVER_ADDRESS + " " + PROXY_SOURCE_PORT + " ";
+
+ @ConfigSuite.Default
+ public static Config config() throws IOException {
+ return config(true);
+ }
+
+ @ConfigSuite.Config
+ public static Config proxyProtocolDisabledConfig() throws IOException {
+ return config(false);
+ }
+
+ private static Config config(boolean enableProxyProtocol) throws IOException {
+ Config cfg = new Config();
+ cfg.setString("auth", null, "type", "HTTP");
+ cfg.setString("auth", null, "httpHeader", "X-Remote-User");
+ cfg.setStringList(
+ "auth", null, "httpTrustedProxyNetworks", List.of(TRUSTED_PROXY_ADDRESS + "/32"));
+ cfg.setBoolean("auth", null, "trustContainerAuth", true);
+ int serverPort = HttpTestUtil.getFreePort();
+ cfg.setString("httpd", null, "listenUrl", "proxy-http://" + SERVER_ADDRESS + ":" + serverPort);
+ cfg.setBoolean("httpd", null, "enableProxyProtocol", enableProxyProtocol);
+ cfg.setStringList(
+ "httpd", null, "filterClass", List.of(HttpTestUtil.RemoteAddressFilter.class.getName()));
+ return cfg;
+ }
+
+ @Inject @TestHttpServerAddress private InetSocketAddress httpAddress;
+
+ private enum ProxyProtocolFormat {
+ V1_PROXY,
+ V2_PROXY,
+ V2_LOCAL
+ }
+
+ private static final List<ProxyProtocolFormat> PROXY_FORMATS =
+ List.of(ProxyProtocolFormat.V1_PROXY, ProxyProtocolFormat.V2_PROXY);
+
+ @Test
+ public void trustedProxyAllowsHttpHeaderAuthenticationWithProxyProtocol() throws Exception {
+ Assume.assumeTrue(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+ for (ProxyProtocolFormat format : PROXY_FORMATS) {
+ byte[] proxyRequest = createProxyRequest(format, serverPort, request);
+
+ HttpResponse proxyResponse = sendRequest(serverPort, proxyRequest, TRUSTED_PROXY_ADDRESS);
+ assertThat(proxyResponse.statusCode()).isEqualTo(200);
+ assertThat(proxyResponse.body()).contains(admin.username());
+ assertThat(proxyResponse.headers())
+ .contains(HttpTestUtil.REMOTE_ADDRESS_HEADER + ": " + PROXY_SOURCE_ADDRESS);
+ }
+ }
+ }
+
+ @Test
+ public void directHttpFromUntrustedPeerCannotUseHttpHeaderAuthentication() throws Exception {
+ Assume.assumeTrue(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+
+ HttpResponse response = sendRequest(serverPort, request, UNTRUSTED_PEER_ADDRESS);
+ assertThat(response.statusCode()).isEqualTo(403);
+ }
+ }
+
+ @Test
+ public void untrustedProxyCannotUseProxyProtocolForHttpHeaderAuthentication() throws Exception {
+ Assume.assumeTrue(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+ for (ProxyProtocolFormat format : PROXY_FORMATS) {
+ byte[] proxyRequest = createProxyRequest(format, serverPort, request);
+
+ HttpResponse response = sendRequest(serverPort, proxyRequest, UNTRUSTED_PEER_ADDRESS);
+ assertThat(response.statusCode()).isEqualTo(403);
+ }
+ }
+ }
+
+ @Test
+ public void xForwardedForTakesPrecedenceOverProxyProtocolAddress() throws Exception {
+ Assume.assumeTrue(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "X-Forwarded-For: "
+ + FORWARDED_CLIENT_ADDRESS
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+ for (ProxyProtocolFormat format : PROXY_FORMATS) {
+ byte[] proxyRequest = createProxyRequest(format, serverPort, request);
+
+ HttpResponse response = sendRequest(serverPort, proxyRequest, TRUSTED_PROXY_ADDRESS);
+ assertThat(response.statusCode()).isEqualTo(200);
+ assertThat(response.body()).contains(admin.username());
+ assertThat(response.headers())
+ .contains(HttpTestUtil.REMOTE_ADDRESS_HEADER + ": " + FORWARDED_CLIENT_ADDRESS);
+ }
+ }
+ }
+
+ // A v2 LOCAL command describes a connection made by the proxy itself, not a relayed client.
+ // The receiver must ignore the address block and keep the actual socket peer address.
+ @Test
+ public void localCommandUsesActualPeerAddress() throws Exception {
+ Assume.assumeTrue(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+ byte[] localRequest = createProxyRequest(ProxyProtocolFormat.V2_LOCAL, serverPort, request);
+
+ HttpResponse response = sendRequest(serverPort, localRequest, TRUSTED_PROXY_ADDRESS);
+ assertThat(response.statusCode()).isEqualTo(200);
+ assertThat(response.body()).contains(admin.username());
+ assertThat(response.headers())
+ .contains(HttpTestUtil.REMOTE_ADDRESS_HEADER + ": " + TRUSTED_PROXY_ADDRESS);
+ assertThat(response.headers()).doesNotContain(PROXY_SOURCE_ADDRESS);
+ }
+ }
+
+ @Test
+ public void proxyProtocolHeaderIsRejectedWhenDisabled() throws Exception {
+ Assume.assumeFalse(baseConfig.getBoolean("httpd", null, "enableProxyProtocol", false));
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ int serverPort = httpAddress.getPort();
+ String request =
+ "GET /a/accounts/self HTTP/1.1\r\n"
+ + "Host: localhost\r\n"
+ + "X-Remote-User: "
+ + admin.username()
+ + "\r\n"
+ + "Connection: close\r\n\r\n";
+ for (ProxyProtocolFormat format : PROXY_FORMATS) {
+ byte[] proxyRequest = createProxyRequest(format, serverPort, request);
+
+ HttpResponse response = sendRequest(serverPort, proxyRequest, TRUSTED_PROXY_ADDRESS);
+ assertThat(response.statusCode()).isEqualTo(400);
+ }
+ }
+ }
+
+ private static HttpResponse sendRequest(int port, String request, String sourceAddress)
+ throws IOException {
+ return sendRequest(port, request.getBytes(StandardCharsets.US_ASCII), sourceAddress);
+ }
+
+ private static HttpResponse sendRequest(int port, byte[] request, String sourceAddress)
+ throws IOException {
+ try (Socket socket = new Socket()) {
+ if (sourceAddress != null) {
+ socket.bind(new InetSocketAddress(sourceAddress, 0));
+ }
+ socket.connect(new InetSocketAddress(SERVER_ADDRESS, port));
+ socket.getOutputStream().write(request);
+ socket.getOutputStream().flush();
+ return parseResponse(
+ new String(socket.getInputStream().readAllBytes(), StandardCharsets.UTF_8));
+ }
+ }
+
+ private static byte[] createProxyRequest(
+ ProxyProtocolFormat format, int targetPort, String request) throws IOException {
+ if (format == ProxyProtocolFormat.V1_PROXY) {
+ return (PROXY_PROTOCOL_HEADER_PREFIX + targetPort + "\r\n" + request)
+ .getBytes(StandardCharsets.US_ASCII);
+ }
+ byte command = format == ProxyProtocolFormat.V2_LOCAL ? (byte) 0x20 : (byte) 0x21;
+ return concat(createProxyProtocolV2Header(command, targetPort), request);
+ }
+
+ private static byte[] createProxyProtocolV2Header(byte command, int targetPort)
+ throws IOException {
+ ByteBuffer header = ByteBuffer.allocate(28);
+ header.put(PROXY_PROTOCOL_V2_SIGNATURE);
+ header.put(command); // Version 2, PROXY or LOCAL command.
+ header.put((byte) 0x11); // IPv4, stream transport.
+ header.putShort((short) 12); // IPv4 addresses and ports: 4 + 4 + 2 + 2 bytes.
+ header.put(InetAddress.getByName(PROXY_SOURCE_ADDRESS).getAddress());
+ header.put(InetAddress.getByName(SERVER_ADDRESS).getAddress());
+ header.putShort((short) PROXY_SOURCE_PORT);
+ header.putShort((short) targetPort);
+ return header.array();
+ }
+
+ private static byte[] concat(byte[] header, String request) {
+ byte[] requestBytes = request.getBytes(StandardCharsets.US_ASCII);
+ byte[] result = new byte[header.length + requestBytes.length];
+ System.arraycopy(header, 0, result, 0, header.length);
+ System.arraycopy(requestBytes, 0, result, header.length, requestBytes.length);
+ return result;
+ }
+
+ private static HttpResponse parseResponse(String response) throws IOException {
+ int headerEnd = response.indexOf("\r\n\r\n");
+ if (headerEnd < 0) {
+ throw new IOException("HTTP response did not contain a header terminator");
+ }
+ int statusLineEnd = response.indexOf("\r\n");
+ if (statusLineEnd < 0 || statusLineEnd > headerEnd) {
+ throw new IOException("HTTP response did not contain a status line");
+ }
+ String[] statusLineParts = response.substring(0, statusLineEnd).split("\\s+", 3);
+ if (statusLineParts.length < 2 || !statusLineParts[0].startsWith("HTTP/")) {
+ throw new IOException("Invalid HTTP status line: " + response.substring(0, statusLineEnd));
+ }
+ try {
+ return new HttpResponse(
+ Integer.parseInt(statusLineParts[1]),
+ response.substring(statusLineEnd + 2, headerEnd),
+ response.substring(headerEnd + 4));
+ } catch (NumberFormatException e) {
+ throw new IOException("Invalid HTTP status code: " + statusLineParts[1], e);
+ }
+ }
+
+ private record HttpResponse(int statusCode, String headers, String body) {}
+}
diff --git a/javatests/com/google/gerrit/pgm/http/jetty/JettyServerTest.java b/javatests/com/google/gerrit/pgm/http/jetty/JettyServerTest.java
new file mode 100644
index 0000000..c84096c
--- /dev/null
+++ b/javatests/com/google/gerrit/pgm/http/jetty/JettyServerTest.java
@@ -0,0 +1,53 @@
+// Copyright (C) 2026 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.pgm.http.jetty;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import org.eclipse.jetty.server.ConnectionFactory;
+import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.ProxyConnectionFactory;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.junit.Test;
+
+public class JettyServerTest {
+ @Test
+ public void proxyProtocolConnectorUsesProxyConnectionFactory() {
+ HttpConfiguration config = new HttpConfiguration();
+
+ ServerConnector connector = JettyServer.newServerConnector(new Server(), 0, 0, config, true);
+ ImmutableList<ConnectionFactory> factories =
+ ImmutableList.copyOf(connector.getConnectionFactories());
+
+ assertThat(factories).hasSize(2);
+ assertThat(factories.get(0)).isInstanceOf(ProxyConnectionFactory.class);
+ assertThat(factories.get(1)).isInstanceOf(HttpConnectionFactory.class);
+ }
+
+ @Test
+ public void regularConnectorDoesNotUseProxyProtocol() {
+ HttpConfiguration config = new HttpConfiguration();
+
+ ServerConnector connector = JettyServer.newServerConnector(new Server(), 0, 0, config, false);
+ ImmutableList<ConnectionFactory> factories =
+ ImmutableList.copyOf(connector.getConnectionFactories());
+
+ assertThat(factories).hasSize(1);
+ assertThat(factories.get(0)).isInstanceOf(HttpConnectionFactory.class);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/h2/H2CacheFactoryTest.java b/javatests/com/google/gerrit/server/cache/h2/H2CacheFactoryTest.java
index 9f55711..e06c564 100644
--- a/javatests/com/google/gerrit/server/cache/h2/H2CacheFactoryTest.java
+++ b/javatests/com/google/gerrit/server/cache/h2/H2CacheFactoryTest.java
@@ -66,7 +66,7 @@
assertThat(readField(store, "url"))
.isEqualTo(
- "jdbc:h2:file:"
+ "jdbc:h2:async:"
+ cacheDir.resolve("demo-cache-v2").toAbsolutePath()
+ ";DB_CLOSE_DELAY=-1");
}
diff --git a/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreIT.java b/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreIT.java
index bac1b31..ef4624a 100644
--- a/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreIT.java
+++ b/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreIT.java
@@ -82,6 +82,26 @@
}
@Test
+ public void legacyFileUrlStartsAndStoresReviewedFlag() throws Exception {
+ SitePaths sitePaths = new SitePaths(temporaryFolder.getRoot().toPath());
+ Config cfg = new Config();
+ cfg.setString(
+ JdbcAccountPatchReviewStore.ACCOUNT_PATCH_REVIEW_DB,
+ null,
+ "url",
+ "jdbc:h2:file:"
+ + temporaryFolder.newFolder("legacy-account-patch-reviews").toPath().resolve("db"));
+ H2JGitLockAccountPatchReviewStore legacyStore =
+ new H2JGitLockAccountPatchReviewStore(cfg, sitePaths);
+ legacyStore.start();
+
+ var unused = legacyStore.markReviewed(PS, ACCOUNT, FILE);
+
+ assertThat(legacyStore.findReviewed(PS, ACCOUNT)).isPresent();
+ assertThat(legacyStore.findReviewed(PS, ACCOUNT).get().files()).containsExactly(FILE);
+ }
+
+ @Test
public void concurrentMarksAreAllDurable() throws Exception {
int nThreads = 8;
int filesPerThread = 5;
diff --git a/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreTest.java b/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreTest.java
index c15d1c8..33b2ad6 100644
--- a/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreTest.java
+++ b/javatests/com/google/gerrit/server/schema/H2JGitLockAccountPatchReviewStoreTest.java
@@ -24,18 +24,23 @@
public class H2JGitLockAccountPatchReviewStoreTest {
@Test
public void lockTargetFromUrl_plainPath() {
+ assertThat(lockTargetFromUrl("jdbc:h2:async:/path/to/db")).isEqualTo(new File("/path/to/db"));
+ }
+
+ @Test
+ public void lockTargetFromUrl_legacyFileUrl() {
assertThat(lockTargetFromUrl("jdbc:h2:file:/path/to/db")).isEqualTo(new File("/path/to/db"));
}
@Test
public void lockTargetFromUrl_stripsOptions() {
- assertThat(lockTargetFromUrl("jdbc:h2:file:/path/to/db;FILE_LOCK=NO;DB_CLOSE_DELAY=0"))
+ assertThat(lockTargetFromUrl("jdbc:h2:async:/path/to/db;FILE_LOCK=NO;DB_CLOSE_DELAY=0"))
.isEqualTo(new File("/path/to/db"));
}
@Test
public void lockTargetFromUrl_unescapesSemicolonInPath() {
- assertThat(lockTargetFromUrl("jdbc:h2:file:/path/with\\;semi/db;FILE_LOCK=NO"))
+ assertThat(lockTargetFromUrl("jdbc:h2:async:/path/with\\;semi/db;FILE_LOCK=NO"))
.isEqualTo(new File("/path/with;semi/db"));
}
diff --git a/javatests/com/google/gerrit/server/schema/JdbcAccountPatchReviewStoreTest.java b/javatests/com/google/gerrit/server/schema/JdbcAccountPatchReviewStoreTest.java
index d8c876b..18b9479 100644
--- a/javatests/com/google/gerrit/server/schema/JdbcAccountPatchReviewStoreTest.java
+++ b/javatests/com/google/gerrit/server/schema/JdbcAccountPatchReviewStoreTest.java
@@ -16,53 +16,118 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.gerrit.server.config.SitePaths;
import java.nio.file.Path;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.Statement;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
public class JdbcAccountPatchReviewStoreTest {
+ @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
@Test
public void checkCreateH2Url() {
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test")))
- .isEqualTo("jdbc:h2:file:" + Path.of("test").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("test").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("test.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("test.db").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db.mv.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("test.db.mv.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("test.db.mv.db").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db.mv.db.trace.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("test.db.mv.db.trace.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("test.db.mv.db.trace.db").toAbsolutePath());
}
@Test
public void checkCreateH2UrlWithPath() {
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("path/to/test")))
- .isEqualTo("jdbc:h2:file:" + Path.of("path/to/test").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("path/to/test").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("path/to/test.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("path/to/test.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("path/to/test.db").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("path/to/test.db.mv.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("path/to/test.db.mv.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("path/to/test.db.mv.db").toAbsolutePath());
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("path/to/test.db.mv.db.trace.db")))
- .isEqualTo("jdbc:h2:file:" + Path.of("path/to/test.db.mv.db.trace.db").toAbsolutePath());
+ .isEqualTo("jdbc:h2:async:" + Path.of("path/to/test.db.mv.db.trace.db").toAbsolutePath());
}
@Test
public void checkCreateH2UrlWithSemicolon() {
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test;test")))
.isEqualTo(
- "jdbc:h2:file:" + Path.of("test;test").toAbsolutePath().toString().replace(";", "\\;"));
+ "jdbc:h2:async:"
+ + Path.of("test;test").toAbsolutePath().toString().replace(";", "\\;"));
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db;test")))
.isEqualTo(
- "jdbc:h2:file:"
+ "jdbc:h2:async:"
+ Path.of("test.db;test").toAbsolutePath().toString().replace(";", "\\;"));
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db.mv.db;test")))
.isEqualTo(
- "jdbc:h2:file:"
+ "jdbc:h2:async:"
+ Path.of("test.db.mv.db;test").toAbsolutePath().toString().replace(";", "\\;"));
assertThat(JdbcAccountPatchReviewStore.createH2Url(Path.of("test.db.mv.db.trace.db;test")))
.isEqualTo(
- "jdbc:h2:file:"
+ "jdbc:h2:async:"
+ Path.of("test.db.mv.db.trace.db;test")
.toAbsolutePath()
.toString()
.replace(";", "\\;"));
}
+
+ @Test
+ public void normalizeH2Url_preservesAsyncUrl() {
+ assertThat(JdbcAccountPatchReviewStore.normalizeH2Url("jdbc:h2:async:/path/to/db;FILE_LOCK=NO"))
+ .isEqualTo("jdbc:h2:async:/path/to/db;FILE_LOCK=NO");
+ }
+
+ @Test
+ public void normalizeH2Url_convertsLegacyFileUrlToAsyncUrl() {
+ assertThat(JdbcAccountPatchReviewStore.normalizeH2Url("jdbc:h2:file:/path/to/db;FILE_LOCK=NO"))
+ .isEqualTo("jdbc:h2:async:/path/to/db;FILE_LOCK=NO");
+ }
+
+ @Test
+ public void getUrl_normalizesLegacyFileUrlFromConfig() throws Exception {
+ Config cfg = new Config();
+ cfg.setString(
+ JdbcAccountPatchReviewStore.ACCOUNT_PATCH_REVIEW_DB,
+ null,
+ "url",
+ "jdbc:h2:file:/path/to/db;FILE_LOCK=NO");
+
+ assertThat(
+ JdbcAccountPatchReviewStore.getUrl(
+ cfg, new SitePaths(temporaryFolder.getRoot().toPath())))
+ .isEqualTo("jdbc:h2:async:/path/to/db;FILE_LOCK=NO");
+ }
+
+ @Test
+ public void normalizeH2Url_preservesNonFileH2Url() {
+ assertThat(JdbcAccountPatchReviewStore.normalizeH2Url("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1"))
+ .isEqualTo("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1");
+ }
+
+ @Test
+ public void createH2Url_canCreateAndReadAsyncH2Database() throws Exception {
+ String url =
+ JdbcAccountPatchReviewStore.createH2Url(
+ temporaryFolder.newFolder("db").toPath().resolve("account_patch_reviews"));
+
+ try (Connection con = DriverManager.getConnection(url);
+ Statement stmt = con.createStatement()) {
+ stmt.executeUpdate("CREATE TABLE account_patch_reviews (id INT PRIMARY KEY, path VARCHAR)");
+ stmt.executeUpdate("INSERT INTO account_patch_reviews VALUES (1, 'README.md')");
+ }
+
+ try (Connection con = DriverManager.getConnection(url);
+ Statement stmt = con.createStatement();
+ ResultSet rs = stmt.executeQuery("SELECT path FROM account_patch_reviews WHERE id = 1")) {
+ assertThat(rs.next()).isTrue();
+ assertThat(rs.getString(1)).isEqualTo("README.md");
+ assertThat(rs.next()).isFalse();
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/util/H2UrlTest.java b/javatests/com/google/gerrit/server/util/H2UrlTest.java
new file mode 100644
index 0000000..e2e0ac4
--- /dev/null
+++ b/javatests/com/google/gerrit/server/util/H2UrlTest.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2026 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.util;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class H2UrlTest {
+ @Test
+ public void prefixes() {
+ assertThat(H2Url.ASYNC_PREFIX).isEqualTo("jdbc:h2:async:");
+ assertThat(H2Url.FILE_PREFIX).isEqualTo("jdbc:h2:file:");
+ }
+}
