You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2021/05/24 17:11:25 UTC

[lucene-solr] branch branch_8x updated: SOLR-15418: V2 API: Fix GET to /select and others (#134)

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

dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 0669811  SOLR-15418: V2 API: Fix GET to /select and others (#134)
0669811 is described below

commit 06698116a6fd07be9886b7f8c74eb436672e1921
Author: David Smiley <ds...@apache.org>
AuthorDate: Mon May 24 12:07:54 2021 -0400

    SOLR-15418: V2 API: Fix GET to /select and others (#134)
    
    Indirectly fixed by not adding ContentStreams that are empty.
---
 solr/CHANGES.txt                                   |  3 ++
 .../apache/solr/servlet/SolrRequestParsers.java    | 33 ++++++++++++++++------
 .../apache/solr/handler/V2ApiIntegrationTest.java  | 12 ++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d6d10ef..ace9ae6 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -103,6 +103,9 @@ Bug Fixes
 * SOLR-11904: Mark ReplicationHandler's polling thread as a Solr server thread so the PKI Interceptor is activated to
   allow PULL replicas to replicate from security-enabled leaders (Timothy Potter, Torsten Bøgh Köster)
 
+* SOLR-15418: The V2 API threw errors for GET requests to collection handlers like /select.
+  (David Smiley)
+
 * SOLR-15424: Solr replication UI wraps ETA time on top of next line (janhoy)
 
 * SOLR-15399: IndexFetcher should not issue a local commit for PULL replicas when leader's version is zero (Timothy Potter)
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
index 2592950..3388569 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -17,12 +17,14 @@
 package org.apache.solr.servlet;
 
 import javax.servlet.MultipartConfigElement;
+import javax.servlet.ServletInputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.Part;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.PushbackInputStream;
 import java.lang.invoke.MethodHandles;
 import java.net.URL;
 import java.nio.ByteBuffer;
@@ -512,12 +514,12 @@ public class SolrRequestParsers {
    */
   static class HttpRequestContentStream extends ContentStreamBase
   {
-    private final HttpServletRequest req;
-    
-    public HttpRequestContentStream( HttpServletRequest req ) {
-      this.req = req;
-      
-      contentType = req.getContentType();
+    private final InputStream inputStream;
+
+    public HttpRequestContentStream(HttpServletRequest req, InputStream inputStream) {
+      this.inputStream = inputStream;
+
+      this.contentType = req.getContentType();
       // name = ???
       // sourceInfo = ???
       
@@ -533,7 +535,7 @@ public class SolrRequestParsers {
       // so that it does not trip our test assert in our close shield
       // in SolrDispatchFilter - we must allow closes from getStream
       // due to the other impls of ContentStream
-      return new CloseShieldInputStream(req.getInputStream());
+      return new CloseShieldInputStream(inputStream);
     }
   }
 
@@ -546,7 +548,22 @@ public class SolrRequestParsers {
     public SolrParams parseParamsAndFillStreams( 
         final HttpServletRequest req, ArrayList<ContentStream> streams ) throws Exception
     {
-      streams.add( new HttpRequestContentStream( req ) );
+      // If we wrongly add a stream that actually has no content, then it can confuse
+      //  some of our code that sees a stream but has no content-type.
+      // If we wrongly don't add a stream, then obviously we'll miss data.
+      final ServletInputStream inputStream = req.getInputStream(); // don't close it
+      if (req.getContentLengthLong() >= 0 || req.getHeader("Transfer-Encoding") != null
+            || inputStream.available() > 0) {
+        streams.add(new HttpRequestContentStream(req, inputStream));
+      } else if (!req.getMethod().equals("GET")) { // GET shouldn't have data
+        // We're not 100% sure there is no data, so check by reading a byte (and put back).
+        PushbackInputStream pbInputStream = new PushbackInputStream(inputStream);
+        int b = pbInputStream.read();
+        if (b != -1) {
+          pbInputStream.unread(b); // put back
+          streams.add(new HttpRequestContentStream(req, pbInputStream));
+        }
+      }
       return parseQueryString( req.getQueryString() );
     }
   }
diff --git a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
index 5db5883..36377c6 100644
--- a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java
@@ -36,6 +36,7 @@ import org.apache.solr.client.solrj.request.V2Request;
 import org.apache.solr.client.solrj.response.DelegationTokenResponse;
 import org.apache.solr.client.solrj.response.V2Response;
 import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.Utils;
@@ -183,6 +184,17 @@ public class V2ApiIntegrationTest extends SolrCloudTestCase {
         .build());
   }
 
+  @Test
+  public void testSelect() throws Exception {
+    CloudSolrClient cloudClient = cluster.getSolrClient();
+    final V2Response v2Response = new V2Request.Builder("/c/" + COLL_NAME + "/select")
+        .withMethod(SolrRequest.METHOD.GET)
+        .withParams(params("q", "-*:*"))
+        .build()
+        .process(cloudClient);
+    assertEquals(0, ((SolrDocumentList)v2Response.getResponse().get("response")).getNumFound());
+  }
+  
   @SuppressWarnings({"rawtypes"})
   private Map resAsMap(CloudSolrClient client, V2Request request) throws SolrServerException, IOException {
     NamedList<Object> rsp = client.request(request);