This is an automated email from the ASF dual-hosted git repository.

gerlowskija pushed a commit to branch branch_9_7
in repository https://gitbox.apache.org/repos/asf/solr.git

commit 88ecadd2f546c3855c96e83bacc6ad61db71d9c2
Author: Jason Gerlowski <[email protected]>
AuthorDate: Tue Oct 29 11:15:22 2024 -0700

    SOLR-17515: Always init Http2SolrClient authStore (#2802)
    
    Prior to this commit, Http2SolrClient only initialized its
    'authenticationStore' instance var when creating a fresh Jetty
    HttpClient.  This causes NPE's later on when client code (e.g.
    PreemptiveBasicAuthClientBuilderFactory) would attempt to access the
    object.
    
    This commit fixes this by making sure that 'authenticationStore' is
    initialized when using an existing Jetty client (typically by reusing
    the AuthenticationStore already used by the client).
---
 solr/CHANGES.txt                                               |  4 ++--
 .../src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java | 10 ++++++++++
 .../org/apache/solr/client/solrj/impl/Http2SolrClient.java     | 10 ++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 81f5884a95b..01c70c17409 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -7,10 +7,10 @@ 
https://github.com/apache/solr/blob/main/solr/solr-ref-guide/modules/upgrade-not
 ==================  9.7.1 ==================
 Bug Fixes
 ---------------------
-(No changes)
-
 * SOLR-17464: Fixed Http2SolrClient bug in that 'requestAsync' triggered NPE 
when using a shared Jetty client (Jason Gerlowski, James Dyer)
 
+* SOLR-17515: Http2SolrClient now sets its 'AuthenticationStore' in all 
circumstances, preventing NPE's when used with a HttpClientBuilderFactory 
(Jason Gerlowski)
+
 Dependency Upgrades
 ---------------------
 (No changes)
diff --git 
a/solr/core/src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java 
b/solr/core/src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java
index 1609f34bdf2..7edaa4090a2 100644
--- a/solr/core/src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java
+++ b/solr/core/src/test/org/apache/solr/cloud/RecoveryZkTestWithAuth.java
@@ -27,6 +27,7 @@ import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrResponse;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.QueryRequest;
@@ -60,6 +61,15 @@ public class RecoveryZkTestWithAuth extends 
SolrCloudTestCase {
 
   @BeforeClass
   public static void setupCluster() throws Exception {
+    // Periodically mimic the sysprops set by bin/solr when it knows auth is 
active (see SOLR-17515
+    // for context)
+    if (rarely()) {
+      System.setProperty(
+          HttpClientUtil.SYS_PROP_HTTP_CLIENT_BUILDER_FACTORY,
+          
"org.apache.solr.client.solrj.impl.PreemptiveBasicAuthClientBuilderFactory");
+      System.setProperty("basicauth", user + ":" + pass);
+    }
+
     cluster =
         configureCluster(1)
             .addConfig("conf", configset("cloud-minimal"))
diff --git 
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java 
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index 0def45620e4..1418ba9da64 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -135,6 +135,8 @@ public class Http2SolrClient extends HttpSolrClientBase {
       if (this.executor == null) {
         this.executor = builder.executor;
       }
+
+      initAuthStoreFromExistingClient(httpClient);
       this.closeClient = false;
     } else {
       this.httpClient = createHttpClient(builder);
@@ -150,6 +152,14 @@ public class Http2SolrClient extends HttpSolrClientBase {
     assert ObjectReleaseTracker.track(this);
   }
 
+  private void initAuthStoreFromExistingClient(HttpClient httpClient) {
+    // Since we don't allow users to provide arbitrary Jetty clients, all 
parameters to this method
+    // must originate from the 'createHttpClient' method, which uses 
AuthenticationStoreHolder.
+    // Verify this assumption and copy the existing instance to avoid 
unnecessary wrapping.
+    assert httpClient.getAuthenticationStore() instanceof 
AuthenticationStoreHolder;
+    this.authenticationStore = (AuthenticationStoreHolder) 
httpClient.getAuthenticationStore();
+  }
+
   @Deprecated(since = "9.7")
   public void addListenerFactory(HttpListenerFactory factory) {
     this.listenerFactory.add(factory);

Reply via email to