Mention explicitly that general regex aren't supported
The general regex were neither explicitly allowed nor forbidden;
add a specific note in the documentation and checks in the code
and tests for making sure that the only general regex allowed
is ^.*
Change-Id: I61d51430e5586f805a4f7691ac6c865e0afc78b8
diff --git a/README.md b/README.md
index e94a03e..a51efd3 100644
--- a/README.md
+++ b/README.md
@@ -63,3 +63,9 @@
[default]
projects = ^.*
```
+
+> **NOTE**: The `^.*` is the only regular expression supported by the virtualhost
+> module because of the potential performance implication of a generic regular expression
+> evaluation during the ACLs. Bear in mind that any possible action perform in Gerrit will
+> go through the virtualhost module filtering and therefore it is paramount to minimize the
+> potential performance impact.
diff --git a/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfig.java b/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfig.java
index d2d1cc9..3b80f7c 100644
--- a/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfig.java
+++ b/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfig.java
@@ -15,6 +15,7 @@
package com.gerritforge.gerrit.modules.virtualhost;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.project.RefPattern;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
@@ -31,6 +32,7 @@
private final Config config;
private final boolean enabled;
+ public static final String ALL_PROJECTS_REGEX = "^.*";
public final String[] defaultProjects;
@Inject
@@ -46,7 +48,7 @@
defaultProjects = new String[0];
return;
}
- defaultProjects = config.getStringList("default", null, "projects");
+ defaultProjects = validateProjects(config.getStringList("default", null, "projects"));
enabled = !config.getSubsections("server").isEmpty() || defaultProjects.length > 0;
}
@@ -55,7 +57,7 @@
return EMPTY_PROJECTS_ARRAY;
}
- String[] projects = config.getStringList("server", hostname, "projects");
+ String[] projects = validateProjects(config.getStringList("server", hostname, "projects"));
if (projects.length > 0) {
return projects;
}
@@ -63,6 +65,16 @@
return defaultProjects;
}
+ private String[] validateProjects(String[] projects) {
+ for (String project : projects) {
+ if (RefPattern.isRE(project) && !project.equals(ALL_PROJECTS_REGEX)) {
+ throw new IllegalStateException(
+ "Project regex '" + project + "' is not allowed in virtualhost.config");
+ }
+ }
+ return projects;
+ }
+
public boolean isEnabled() {
return enabled;
}
diff --git a/src/test/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfigTest.java b/src/test/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfigTest.java
new file mode 100644
index 0000000..d9ecbf2
--- /dev/null
+++ b/src/test/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostConfigTest.java
@@ -0,0 +1,80 @@
+// Copyright (C) 2024 GerritForge Ltd.
+//
+// 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.gerritforge.gerrit.modules.virtualhost;
+
+import static com.gerritforge.gerrit.modules.virtualhost.VirtualHostConfig.ALL_PROJECTS_REGEX;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.server.config.SitePaths;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.Before;
+import org.junit.Test;
+
+public class VirtualHostConfigTest {
+
+ public static final String TEST_VIRTUAL_HOST = "testhost";
+ private SitePaths testSitePaths;
+ private FileBasedConfig virtualHostFileConfig;
+
+ @Before
+ public void setup() throws IOException {
+ Path tempPath = Files.createTempDirectory("virtualhost-test-" + System.nanoTime());
+ testSitePaths = new SitePaths(tempPath);
+ virtualHostFileConfig =
+ new FileBasedConfig(
+ testSitePaths.etc_dir.resolve("virtualhost.config").toFile(), FS.DETECTED);
+ }
+
+ @Test
+ public void projectsAllowsCatchAllRegExOnDefaults() throws IOException {
+ setVirtualHostConfig("default", null, ALL_PROJECTS_REGEX);
+ assertThat(new VirtualHostConfig(testSitePaths).getProjects(TEST_VIRTUAL_HOST))
+ .isEqualTo(new String[] {ALL_PROJECTS_REGEX});
+ }
+
+ @Test
+ public void projectsAllowsCatchAllRegExOnSpecificHost() throws IOException {
+ setVirtualHostConfig("server", TEST_VIRTUAL_HOST, ALL_PROJECTS_REGEX);
+ assertThat(new VirtualHostConfig(testSitePaths).getProjects(TEST_VIRTUAL_HOST))
+ .isEqualTo(new String[] {ALL_PROJECTS_REGEX});
+ }
+
+ @Test
+ public void projectsGeneralRegExOnDefaultsShouldBeForbidden() throws IOException {
+ setVirtualHostConfig("default", null, "^ageneralregex.*$");
+ assertThrows(
+ IllegalStateException.class,
+ () -> new VirtualHostConfig(testSitePaths).getProjects(TEST_VIRTUAL_HOST));
+ }
+
+ @Test
+ public void projectsGeneralRegExOnSpecificHostShouldBeForbidden() throws IOException {
+ setVirtualHostConfig("server", TEST_VIRTUAL_HOST, "^ageneralregex.*$");
+ assertThrows(
+ IllegalStateException.class,
+ () -> new VirtualHostConfig(testSitePaths).getProjects(TEST_VIRTUAL_HOST));
+ }
+
+ private void setVirtualHostConfig(String section, String subsection, String projectsValue)
+ throws IOException {
+ virtualHostFileConfig.setString(section, subsection, "projects", projectsValue);
+ virtualHostFileConfig.save();
+ }
+}
