This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit ebf8b0d0f7b6fb57134c4be424906d8ac891356d Author: sarkaramrit1993 <[email protected]> AuthorDate: Wed Oct 21 03:56:02 2026 -0700 SOLR-18010: Fix security.json corruption from parallel edits (#5065) (cherry picked from commit 1afeee27c48416615cf26b13488a0aee976424dd) (cherry picked from commit 9a0837ad0c6b189e8a7c86b9b5b6e44243236f92) Test adapted to branch_9x's Apache HttpClient-based test setup. --- .../SOLR-18010-security-json-corruption.yml | 7 +++ .../handler/admin/SecurityConfHandlerLocal.java | 13 +++++ .../solr/security/BasicAuthStandaloneTest.java | 62 ++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/changelog/unreleased/SOLR-18010-security-json-corruption.yml b/changelog/unreleased/SOLR-18010-security-json-corruption.yml new file mode 100644 index 00000000000..25121863215 --- /dev/null +++ b/changelog/unreleased/SOLR-18010-security-json-corruption.yml @@ -0,0 +1,7 @@ +title: Concurrent security edits in standalone mode, such as creating a role with permissions in the Admin UI, no longer corrupt security.json. +type: fixed +authors: + - name: Amrit Sarkar +links: + - name: SOLR-18010 + url: https://issues.apache.org/jira/browse/SOLR-18010 diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 356f355942a..11f66afd84c 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -29,6 +29,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; +import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,12 +38,24 @@ import org.slf4j.LoggerFactory; public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected Path securityJsonPath; + private final Object securityJsonLock = new Object(); public SecurityConfHandlerLocal(CoreContainer coreContainer) { super(coreContainer); securityJsonPath = Paths.get(coreContainer.getSolrHome()).resolve("security.json"); } + /** + * Serializes requests, since a local file has no versioned write to detect concurrent edits the + * way ZooKeeper does. Reads are included so they never see a partially written file. + */ + @Override + public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { + synchronized (securityJsonLock) { + super.handleRequestBody(req, rsp); + } + } + /** * Fetches security props from SOLR_HOME * diff --git a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java index ad4a671d259..edf96ab8d26 100644 --- a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java +++ b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java @@ -26,9 +26,16 @@ import java.lang.invoke.MethodHandles; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Base64; import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Properties; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; @@ -40,6 +47,8 @@ import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.HttpClientUtil; import org.apache.solr.common.params.MapSolrParams; +import org.apache.solr.common.util.ExecutorUtil; +import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.common.util.Utils; import org.apache.solr.embedded.JettyConfig; import org.apache.solr.embedded.JettySolrRunner; @@ -153,6 +162,59 @@ public class BasicAuthStandaloneTest extends SolrTestCaseJ4 { } } + @Test + public void testConcurrentEdits() throws Exception { + String authzPrefix = "/admin/authorization"; + HttpClient httpClient = HttpClientUtil.createClient(null); + try { + concurrentEdits(httpClient, buildUrl(jetty.getLocalPort(), "/solr") + authzPrefix); + } finally { + HttpClientUtil.close(httpClient); + } + } + + private void concurrentEdits(HttpClient httpClient, String url) throws Exception { + securityConfHandler.persistConf( + new SecurityConfHandler.SecurityConfig() + .setData(Utils.fromJSONString(STD_CONF.replace("'", "\"")))); + securityConfHandler.securityConfEdited(); + verifySecurityStatus(httpClient, url, "authorization.enabled", "true", 20); + + int numEdits = 20; + // Release all edits at once so they overlap, like the Admin UI's per-permission POSTs. The + // await timeout fails the test instead of hanging if the pool ever has fewer threads than + // numEdits. + CyclicBarrier barrier = new CyclicBarrier(numEdits); + ExecutorService executor = + ExecutorUtil.newMDCAwareFixedThreadPool( + numEdits, new SolrNamedThreadFactory("securityEdit")); + try { + List<Future<?>> futures = new ArrayList<>(); + for (int i = 0; i < numEdits; i++) { + String command = "{'set-user-role': {'user" + i + "': ['dev']}}"; + futures.add( + executor.submit( + () -> { + barrier.await(30, TimeUnit.SECONDS); + doHttpPost(httpClient, url, command, "solr", "SolrRocks"); + return null; + })); + } + for (Future<?> future : futures) { + future.get(); + } + } finally { + ExecutorUtil.shutdownAndAwaitTermination(executor); + } + + Map<?, ?> authz = + (Map<?, ?>) securityConfHandler.getSecurityConfig(false).getData().get("authorization"); + Map<?, ?> userRoles = (Map<?, ?>) authz.get("user-role"); + for (int i = 0; i < numEdits; i++) { + assertTrue("missing user" + i + " in " + userRoles, userRoles.containsKey("user" + i)); + } + } + static void doHttpPost( HttpClient cl, String url, String jsonCommand, String basicUser, String basicPass) throws IOException {
