SOLR-5700: Improve error handling of remote queries (proxied requests).

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1566174 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -383,6 +383,9 @@ Other Changes
 * SOLR-5659: Add test for compositeId ending with an '!'.
   (Markus Jelsma, Anshum Gupta via shalin)
 
+* SOLR-5700: Improve error handling of remote queries (proxied requests).
+  (Greg Chanan via Mark Miller)
+
 ==================  4.6.1  ==================
 
 Versions of Major Components

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

@@ -19,6 +19,20 @@ package org.apache.solr.servlet;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpOptions;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.entity.InputStreamEntity;
+import org.apache.http.Header;
+import org.apache.http.HeaderIterator;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpEntityEnclosingRequest;
+import org.apache.http.HttpResponse;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.Aliases;
@@ -42,6 +56,7 @@ import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrConfig;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
 import org.apache.solr.handler.ContentStreamHandlerBase;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrQueryRequestBase;
@@ -72,7 +87,6 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
-import java.net.HttpURLConnection;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
@@ -96,6 +110,7 @@ public class SolrDispatchFilter implements Filter
 {
   private static final String CONNECTION_HEADER = "Connection";
   private static final String TRANSFER_ENCODING_HEADER = "Transfer-Encoding";
+  private static final String CONTENT_LENGTH_HEADER = "Content-Length";
 
   final Logger log;
 
@@ -103,6 +118,7 @@ public class SolrDispatchFilter implements Filter
 
   protected String pathPrefix = null; // strip this from the beginning of a path
   protected String abortErrorMessage = null;
+  protected final HttpClient httpClient = HttpClientUtil.createClient(new ModifiableSolrParams());
   
   private static final Charset UTF8 = Charset.forName("UTF-8");
 
@@ -495,6 +511,8 @@ public class SolrDispatchFilter implements Filter
 
   private void remoteQuery(String coreUrl, HttpServletRequest req,
       SolrQueryRequest solrReq, HttpServletResponse resp) throws IOException {
+    HttpRequestBase method = null;
+    boolean success = false;
     try {
       String urlstr = coreUrl;
       
@@ -503,58 +521,58 @@ public class SolrDispatchFilter implements Filter
       urlstr += queryString == null ? "" : "?" + queryString;
       
       URL url = new URL(urlstr);
-      HttpURLConnection con = (HttpURLConnection) url.openConnection();
-      con.setRequestMethod(req.getMethod());
-      con.setUseCaches(false);
-      
       boolean isPostOrPutRequest = "POST".equals(req.getMethod()) || "PUT".equals(req.getMethod());
 
-      if (isPostOrPutRequest) {
-        con.setDoOutput(true);
+      if ("GET".equals(req.getMethod())) {
+        method = new HttpGet(urlstr);
       }
-      con.setDoInput(true);
+      else if ("HEAD".equals(req.getMethod())) {
+        method = new HttpHead(urlstr);
+      }
+      else if (isPostOrPutRequest) {
+        HttpEntityEnclosingRequestBase entityRequest =
+          "POST".equals(req.getMethod()) ? new HttpPost(urlstr) : new HttpPut(urlstr);
+        HttpEntity entity = new InputStreamEntity(req.getInputStream(), req.getContentLength());
+        entityRequest.setEntity(entity);
+        method = entityRequest;
+      }
+      else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+          "Unexpected method type: " + req.getMethod());
+      }
+
       for (Enumeration<String> e = req.getHeaderNames(); e.hasMoreElements();) {
         String headerName = e.nextElement();
-        con.setRequestProperty(headerName, req.getHeader(headerName));
+        method.addHeader(headerName, req.getHeader(headerName));
+      }
+      // These headers not supported for HttpEntityEnclosingRequests
+      if (method instanceof HttpEntityEnclosingRequest) {
+        method.removeHeaders(TRANSFER_ENCODING_HEADER);
+        method.removeHeaders(CONTENT_LENGTH_HEADER);
       }
-      try {
-        con.connect();
 
-        InputStream is;
-        OutputStream os;
-        if (isPostOrPutRequest) {
-          is = req.getInputStream();
-          os = con.getOutputStream(); // side effect: method is switched to POST
-          try {
-            IOUtils.copyLarge(is, os);
-            os.flush();
-          } finally {
-            IOUtils.closeQuietly(os);
-            IOUtils.closeQuietly(is);  // TODO: I thought we weren't supposed to explicitly close servlet streams
-          }
+      final HttpResponse response = httpClient.execute(method);
+      int httpStatus = response.getStatusLine().getStatusCode();
+      HttpEntity httpEntity = response.getEntity();
+
+      resp.setStatus(httpStatus);
+      for (HeaderIterator responseHeaders = response.headerIterator(); responseHeaders.hasNext();) {
+        Header header = responseHeaders.nextHeader();
+
+        // We pull out these two headers below because they can cause chunked
+        // encoding issues with Tomcat
+        if (header != null && !header.getName().equals(TRANSFER_ENCODING_HEADER)
+          && !header.getName().equals(CONNECTION_HEADER)) {
+            resp.addHeader(header.getName(), header.getValue());
         }
+      }
 
-        resp.setStatus(con.getResponseCode());
+      if (httpEntity != null) {
+        if (httpEntity.getContentEncoding() != null) resp.setCharacterEncoding(httpEntity.getContentEncoding().getValue());
+        if (httpEntity.getContentType() != null) resp.setContentType(httpEntity.getContentType().getValue());
 
-        for (Iterator<Entry<String,List<String>>> i = con.getHeaderFields().entrySet().iterator(); i.hasNext();) {
-          Map.Entry<String,List<String>> mapEntry = i.next();
-          String header = mapEntry.getKey();
-          
-          // We pull out these two headers below because they can cause chunked
-          // encoding issues with Tomcat and certain clients
-          if (header != null && !header.equals(TRANSFER_ENCODING_HEADER)
-              && !header.equals(CONNECTION_HEADER)) {
-            for (String value : mapEntry.getValue()) {
-              resp.addHeader(mapEntry.getKey(), value);
-            }
-          }
-        }
-        
-        resp.setCharacterEncoding(con.getContentEncoding());
-        resp.setContentType(con.getContentType());
-        
-        is = con.getInputStream();
-        os = resp.getOutputStream();
+        InputStream is = httpEntity.getContent();
+        OutputStream os = resp.getOutputStream();
         try {
           IOUtils.copyLarge(is, os);
           os.flush();
@@ -562,13 +580,16 @@ public class SolrDispatchFilter implements Filter
           IOUtils.closeQuietly(os);   // TODO: I thought we weren't supposed to explicitly close servlet streams
           IOUtils.closeQuietly(is);
         }
-      } finally {
-        con.disconnect();
       }
+      success = true;
     } catch (IOException e) {
       sendError(null, solrReq, req, resp, new SolrException(
           SolrException.ErrorCode.SERVER_ERROR,
           "Error trying to proxy request for url: " + coreUrl, e));
+    } finally {
+      if (method != null && !success) {
+        method.abort();
+      }
     }
 
   }

solr/core/src/test/org/apache/solr/cloud/RemoteQueryErrorTest.java

@@ -0,0 +1,101 @@
+package org.apache.solr.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.lucene.util.LuceneTestCase.Slow;
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.impl.HttpSolrServer;
+import org.apache.solr.common.SolrInputDocument;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+
+/**
+ * Verify that remote (proxied) queries return proper error messages
+ */
+
+@Slow
+public class RemoteQueryErrorTest extends AbstractFullDistribZkTestBase {
+  @BeforeClass
+  public static void beforeSuperClass() throws Exception {
+  }
+  
+  @AfterClass
+  public static void afterSuperClass() {
+    
+  }
+  
+  @Before
+  @Override
+  public void setUp() throws Exception {
+    super.setUp();
+    System.setProperty("numShards", Integer.toString(sliceCount));
+  }
+  
+  @Override
+  @After
+  public void tearDown() throws Exception {
+    super.tearDown();
+    resetExceptionIgnores();
+  }
+
+  public RemoteQueryErrorTest() {
+    super();
+    sliceCount = 1;
+    shardCount = random().nextBoolean() ? 3 : 4;
+  }
+
+  @Override
+  public void doTest() throws Exception {
+    handle.clear();
+    handle.put("QTime", SKIPVAL);
+    handle.put("timestamp", SKIPVAL);
+    
+    waitForThingsToLevelOut(15);
+
+    del("*:*");
+    
+    createCollection("collection2", 2, 1, 10);
+    
+    List<Integer> numShardsNumReplicaList = new ArrayList<Integer>(2);
+    numShardsNumReplicaList.add(2);
+    numShardsNumReplicaList.add(1);
+    checkForCollection("collection2", numShardsNumReplicaList, null);
+    waitForRecoveriesToFinish("collection2", true);
+
+    HttpSolrServer solrServer = null;
+    for (JettySolrRunner jetty : jettys) {
+      int port = port = jetty.getLocalPort();
+      solrServer = new HttpSolrServer("http://127.0.0.1:" + port + context + "/collection2");
+      try {
+        SolrInputDocument emptyDoc = new SolrInputDocument();
+        solrServer.add(emptyDoc);
+        fail("Expected unique key exceptoin");
+      } catch (Exception ex) {
+        assert(ex.getMessage().contains("Document is missing mandatory uniqueKey field: id"));
+      } finally {
+        solrServer.shutdown();
+      }
+    }
+  }
+}