SOLR-17069: HttpServletRequest; use eagerly to avoid errors (#3324)

HttpServletRequest should be used only by the thread dispatched receiving it, or else it _might_ produce errors (particularly in Jetty 12).  Thus aspects of the request that are needed should be eagerly consumed instead of deferred, to avoid risks.
* Cache Span in HttpSolrCall to avoid NPE from Jetty 12 async request loss
* javadocs: warning for HttpSolrCall.getReq
* Cache the security UserPrincipal in SolrRequestInfo, and not the request
* CancellableQueryTracker: don't use HttpServletRequest
* HttpSolrCall: eager parse the user agent
* SolrRequestParsers: eagerly get the principal from the request

---------

Co-authored-by: Sanjay Dutt <sanjaydutt@apache.org>

Author: dsmiley

solr/core/src/java/org/apache/solr/core/CancellableQueryTracker.java

@@ -56,7 +56,7 @@ public String generateQueryID(SolrQueryRequest req) {
       }
     }
 
-    activeQueriesGenerated.put(queryID, req.getHttpSolrCall().getReq().getQueryString());
+    activeQueriesGenerated.put(queryID, req.getParamString());
 
     return queryID;
   }

solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java

@@ -209,7 +209,7 @@ private LBSolrClient.Req prepareLBRequest(
     req.setMethod(SolrRequest.METHOD.POST);
     SolrRequestInfo requestInfo = SolrRequestInfo.getRequestInfo();
     if (requestInfo != null) {
-      req.setUserPrincipal(requestInfo.getReq().getUserPrincipal());
+      req.setUserPrincipal(requestInfo.getUserPrincipal());
     }
 
     return httpShardHandlerFactory.newLBHttpSolrClientReq(req, urls);

solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java

@@ -51,12 +51,12 @@ public class SolrRequestInfo {
   private SolrQueryRequest req;
   private SolrQueryResponse rsp;
   private Date now;
-  public HttpServletRequest httpRequest;
   private TimeZone tz;
   private ResponseBuilder rb;
   private List<AutoCloseable> closeHooks;
   private SolrDispatchFilter.Action action;
   private boolean useServerToken = false;
+  private Principal principal;
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
@@ -151,6 +151,7 @@ private synchronized void close() {
   public SolrRequestInfo(SolrQueryRequest req, SolrQueryResponse rsp) {
     this.req = req;
     this.rsp = rsp;
+    this.principal = req != null ? req.getUserPrincipal() : null;
   }
 
   public SolrRequestInfo(
@@ -160,8 +161,8 @@ public SolrRequestInfo(
   }
 
   public SolrRequestInfo(HttpServletRequest httpReq, SolrQueryResponse rsp) {
-    this.httpRequest = httpReq;
     this.rsp = rsp;
+    this.principal = httpReq != null ? httpReq.getUserPrincipal() : null;
   }
 
   public SolrRequestInfo(
@@ -171,9 +172,7 @@ public SolrRequestInfo(
   }
 
   public Principal getUserPrincipal() {
-    if (req != null) return req.getUserPrincipal();
-    if (httpRequest != null) return httpRequest.getUserPrincipal();
-    return null;
+    return principal;
   }
 
   public Date getNOW() {

solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java

@@ -50,6 +50,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -145,6 +146,8 @@ public class HttpSolrCall {
   protected final HttpServletRequest req;
   protected final HttpServletResponse response;
   protected final boolean retry;
+  private final SolrVersion userAgentSolrVersion; // of the client
+  private final Span span;
   protected SolrCore core = null;
   protected SolrQueryRequest solrReq = null;
   private boolean mustClearSolrRequestInfo = false;
@@ -176,18 +179,30 @@ public HttpSolrCall(
     this.response = response;
     this.retry = retry;
     this.requestType = RequestType.UNKNOWN;
+    this.userAgentSolrVersion = parseUserAgentSolrVersion();
+    this.span = Optional.ofNullable(TraceUtils.getSpan(req)).orElse(Span.getInvalid());
+    this.path = ServletUtils.getPathAfterContext(req);
+
     req.setAttribute(HttpSolrCall.class.getName(), this);
     // set a request timer which can be reused by requests if needed
     req.setAttribute(SolrRequestParsers.REQUEST_TIMER_SERVLET_ATTRIBUTE, new RTimerTree());
     // put the core container in request attribute
     req.setAttribute("org.apache.solr.CoreContainer", cores);
-    path = ServletUtils.getPathAfterContext(req);
   }
 
   public String getPath() {
     return path;
   }
 
+  /**
+   * WARNING: This method returns a non-null {@link HttpServletRequest}, but calling certain methods
+   * on it — such as {@link HttpServletRequest#getAttribute(String)} — may throw {@link
+   * NullPointerException} if accessed outside the original servlet thread (e.g., in asynchronous
+   * tasks).
+   *
+   * <p>Always cache required request data early during request handling if it needs to be accessed
+   * later.
+   */
   public HttpServletRequest getReq() {
     return req;
   }
@@ -639,12 +654,7 @@ protected void handleAdminOrRemoteRequest() throws IOException {
 
   /** Get the Span for this request. Not null. */
   public Span getSpan() {
-    var s = TraceUtils.getSpan(req);
-    if (s != null) {
-      return s;
-    } else {
-      return Span.getInvalid();
-    }
+    return span;
   }
 
   // called after init().
@@ -1174,6 +1184,10 @@ protected Object _getHandler() {
    * clients prior to 9.9 present themselves as 1.0 or 2.0.
    */
   public SolrVersion getUserAgentSolrVersion() {
+    return userAgentSolrVersion;
+  }
+
+  private SolrVersion parseUserAgentSolrVersion() {
     String header = req.getHeader("User-Agent");
     if (header == null || !header.startsWith("Solr")) {
       return null;

solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java

@@ -169,7 +169,7 @@ public SolrQueryRequest parse(SolrCore core, String path, HttpServletRequest req
     SolrParams params = parser.parseParamsAndFillStreams(req, streams);
 
     SolrQueryRequest sreq =
-        buildRequestFrom(core, params, streams, getRequestTimer(req), req, null);
+        buildRequestFrom(core, params, streams, getRequestTimer(req), req, req.getUserPrincipal());
 
     // Handlers and login will want to know the path. If it contains a ':'
     // the handler could use it for RESTful URLs
@@ -200,7 +200,7 @@ private SolrQueryRequest buildRequestFrom(
       Collection<ContentStream> streams, // might be added to but caller shouldn't depend on it
       RTimerTree requestTimer,
       final HttpServletRequest req,
-      final Principal principal)
+      final Principal principal) // from req, if req was provided, otherwise from elsewhere
       throws Exception {
     // ensure streams is non-null and mutable so we can easily add to it
     if (streams == null) {
@@ -267,11 +267,7 @@ private SolrQueryRequest buildRequestFrom(
         new SolrQueryRequestBase(core, params, requestTimer) {
           @Override
           public Principal getUserPrincipal() {
-            if (principal != null) {
-              return principal;
-            } else {
-              return req == null ? null : req.getUserPrincipal();
-            }
+            return principal;
           }
 
           @Override

solr/core/src/java/org/apache/solr/update/SolrCmdDistributor.java

@@ -308,7 +308,7 @@ private void submit(final Req req, boolean isCommit) throws IOException {
     // Copy user principal from the original request to the new update request, for later
     // authentication interceptor use
     if (SolrRequestInfo.getRequestInfo() != null) {
-      req.uReq.setUserPrincipal(SolrRequestInfo.getRequestInfo().getReq().getUserPrincipal());
+      req.uReq.setUserPrincipal(SolrRequestInfo.getRequestInfo().getUserPrincipal());
     }
 
     if (req.synchronous) {