You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by an...@apache.org on 2016/04/19 23:11:04 UTC

[2/3] lucene-solr:branch_5x: SOLR-8683: Always consume the full request on the server, not just in the case of an error.

SOLR-8683: Always consume the full request on the server, not just in the case of an error.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/64c85394
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/64c85394
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/64c85394

Branch: refs/heads/branch_5x
Commit: 64c85394cb174669f8cf809d0a13a96bc5a92001
Parents: dab4c81
Author: markrmiller <ma...@apache.org>
Authored: Tue Feb 16 15:48:11 2016 -0500
Committer: anshum <an...@apache.org>
Committed: Tue Apr 19 12:24:32 2016 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |   3 +
 .../org/apache/solr/servlet/HttpSolrCall.java   |  29 +----
 .../apache/solr/servlet/SolrDispatchFilter.java | 107 +++++++++++--------
 3 files changed, 69 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/64c85394/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 792cef1..ccf396e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -68,6 +68,9 @@ Bug Fixes
 * SOLR-8578: Successful or not, requests are not always fully consumed by Solrj clients and we
   count on HttpClient or the JVM. (Mark Miller)
 
+* SOLR-8683: Always consume the full request on the server, not just in the case of an error.
+ (Mark Miller)
+
 ======================= 5.5.0 =======================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/64c85394/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index c50e5be..4288396 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -623,13 +623,9 @@ public class HttpSolrCall {
     } finally {
       try {
         if (exp != null) {
-          try {
-            SimpleOrderedMap info = new SimpleOrderedMap();
-            int code = ResponseUtils.getErrorInfo(ex, info, log);
-            sendError(code, info.toString());
-          } finally {
-            consumeInput(req);
-          }
+          SimpleOrderedMap info = new SimpleOrderedMap();
+          int code = ResponseUtils.getErrorInfo(ex, info, log);
+          sendError(code, info.toString());
         }
       } finally {
         if (core == null && localCore != null) {
@@ -644,21 +640,6 @@ public class HttpSolrCall {
       response.sendError(code, message);
     } catch (EOFException e) {
       log.info("Unable to write error response, client closed connection or we are shutting down", e);
-    } finally {
-      consumeInput(req);
-    }
-  }
-
-  // when we send back an error, we make sure we read
-  // the full client request so that the client does
-  // not hit a connection reset and we can reuse the 
-  // connection - see SOLR-8453
-  private void consumeInput(HttpServletRequest req) {
-    try {
-      ServletInputStream is = req.getInputStream();
-      while (!is.isFinished() && is.read() != -1) {}
-    } catch (IOException e) {
-      log.info("Could not consume full client request", e);
     }
   }
 
@@ -745,10 +726,6 @@ public class HttpSolrCall {
       //else http HEAD request, nothing to write out, waited this long just to get ContentType
     } catch (EOFException e) {
       log.info("Unable to write response, client closed connection or we are shutting down", e);
-    } finally {
-      if (solrRsp.getException() != null) {
-        consumeInput(req);
-      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/64c85394/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index baab3eb..7a0e4ef 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -19,6 +19,7 @@ package org.apache.solr.servlet;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
+import javax.servlet.ServletInputStream;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
@@ -185,58 +186,76 @@ public class SolrDispatchFilter extends BaseSolrFilter {
   
   public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain, boolean retry) throws IOException, ServletException {
     if (!(request instanceof HttpServletRequest)) return;
+    try {
 
-    if (cores == null || cores.isShutDown()) {
-      log.error("Error processing the request. CoreContainer is either not initialized or shutting down.");
-      throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
-          "Error processing the request. CoreContainer is either not initialized or shutting down.");
-    }
+      if (cores == null || cores.isShutDown()) {
+        log.error("Error processing the request. CoreContainer is either not initialized or shutting down.");
+        throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
+            "Error processing the request. CoreContainer is either not initialized or shutting down.");
+      }
 
-    AtomicReference<ServletRequest> wrappedRequest = new AtomicReference<>();
-    if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent
-      return;
-    }
-    if (wrappedRequest.get() != null) {
-      request = wrappedRequest.get();
-    }
-    if (cores.getAuthenticationPlugin() != null) {
-      log.debug("User principal: {}", ((HttpServletRequest)request).getUserPrincipal());
-    }
+      AtomicReference<ServletRequest> wrappedRequest = new AtomicReference<>();
+      if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been
+                                                                     // sent
+        return;
+      }
+      if (wrappedRequest.get() != null) {
+        request = wrappedRequest.get();
+      }
+      if (cores.getAuthenticationPlugin() != null) {
+        log.debug("User principal: {}", ((HttpServletRequest) request).getUserPrincipal());
+      }
 
-    // No need to even create the HttpSolrCall object if this path is excluded.
-    if(excludePatterns != null) {
-      String requestPath = ((HttpServletRequest) request).getServletPath();
-      String extraPath = ((HttpServletRequest)request).getPathInfo();
-      if (extraPath != null) { // In embedded mode, servlet path is empty - include all post-context path here for testing 
-        requestPath += extraPath;
+      // No need to even create the HttpSolrCall object if this path is excluded.
+      if (excludePatterns != null) {
+        String requestPath = ((HttpServletRequest) request).getServletPath();
+        String extraPath = ((HttpServletRequest) request).getPathInfo();
+        if (extraPath != null) { // In embedded mode, servlet path is empty - include all post-context path here for
+                                 // testing
+          requestPath += extraPath;
+        }
+        for (Pattern p : excludePatterns) {
+          Matcher matcher = p.matcher(requestPath);
+          if (matcher.lookingAt()) {
+            chain.doFilter(request, response);
+            return;
+          }
+        }
       }
-      for (Pattern p : excludePatterns) {
-        Matcher matcher = p.matcher(requestPath);
-        if (matcher.lookingAt()) {
-          chain.doFilter(request, response);
-          return;
+
+      HttpSolrCall call = getHttpSolrCall((HttpServletRequest) request, (HttpServletResponse) response, retry);
+      ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
+      try {
+        Action result = call.call();
+        switch (result) {
+          case PASSTHROUGH:
+            chain.doFilter(request, response);
+            break;
+          case RETRY:
+            doFilter(request, response, chain, true);
+            break;
+          case FORWARD:
+            request.getRequestDispatcher(call.getPath()).forward(request, response);
+            break;
         }
+      } finally {
+        call.destroy();
+        ExecutorUtil.setServerThreadFlag(null);
       }
+    } finally {
+      consumeInputFully((HttpServletRequest) request);
     }
-
-    HttpSolrCall call = getHttpSolrCall((HttpServletRequest) request, (HttpServletResponse) response, retry);
-    ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
+  }
+  
+  // we make sure we read the full client request so that the client does
+  // not hit a connection reset and we can reuse the 
+  // connection - see SOLR-8453 and SOLR-8683
+  private void consumeInputFully(HttpServletRequest req) {
     try {
-      Action result = call.call();
-      switch (result) {
-        case PASSTHROUGH:
-          chain.doFilter(request, response);
-          break;
-        case RETRY:
-          doFilter(request, response, chain, true);
-          break;
-        case FORWARD:
-          request.getRequestDispatcher(call.getPath()).forward(request, response);
-          break;
-      }  
-    } finally {
-      call.destroy();
-      ExecutorUtil.setServerThreadFlag(null);
+      ServletInputStream is = req.getInputStream();
+      while (!is.isFinished() && is.read() != -1) {}
+    } catch (IOException e) {
+      log.info("Could not consume full client request", e);
     }
   }