SOLR-8933: Solr should not close container streams.

lucene/replicator/src/java/org/apache/lucene/replicator/http/ReplicationService.java

@@ -148,6 +148,7 @@ public class ReplicationService {
       throw new ServletException("unrecognized shard ID " + pathElements[SHARD_IDX]);
     }
     
+    // SOLR-8933 Don't close this stream.
     ServletOutputStream resOut = resp.getOutputStream();
     try {
       switch (action) {

solr/CHANGES.txt

@@ -194,6 +194,8 @@ Other Changes
 
 * SOLR-8929: Add an idea module for solr/server to enable launching start.jar (Scott Blum, Steve Rowe)
 
+* SOLR-8933: Solr should not close container streams. (Mike Drob, Uwe Schindler, Mark Miller)
+
 * SOLR-9037: Replace multiple "/replication" strings with one static constant. (Christine Poerschke)
 
 * SOLR-9047: zkcli should allow alternative locations for log4j configuration (Gregory Chanan)

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

@@ -17,9 +17,9 @@
 package org.apache.solr.servlet;
 
 import javax.servlet.ServletInputStream;
-
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -41,7 +41,10 @@ import java.util.Random;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
+
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.input.CloseShieldInputStream;
+import org.apache.commons.io.output.CloseShieldOutputStream;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.Header;
 import org.apache.http.HeaderIterator;
@@ -533,7 +536,8 @@ public class HttpSolrCall {
       } else if (isPostOrPutRequest) {
         HttpEntityEnclosingRequestBase entityRequest =
             "POST".equals(req.getMethod()) ? new HttpPost(urlstr) : new HttpPut(urlstr);
-        HttpEntity entity = new InputStreamEntity(req.getInputStream(), req.getContentLength());
+        InputStream in = new CloseShieldInputStream(req.getInputStream()); // Prevent close of container streams
+        HttpEntity entity = new InputStreamEntity(in, req.getContentLength());
         entityRequest.setEntity(entity);
         method = entityRequest;
       } else if ("DELETE".equals(req.getMethod())) {
@@ -722,7 +726,8 @@ public class HttpSolrCall {
       }
 
       if (Method.HEAD != reqMethod) {
-        QueryResponseWriterUtil.writeQueryResponse(response.getOutputStream(), responseWriter, solrReq, solrRsp, ct);
+        OutputStream out = new CloseShieldOutputStream(response.getOutputStream()); // Prevent close of container streams, see SOLR-8933
+        QueryResponseWriterUtil.writeQueryResponse(out, responseWriter, solrReq, solrRsp, ct);
       }
       //else http HEAD request, nothing to write out, waited this long just to get ContentType
     } catch (EOFException e) {

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

@@ -17,6 +17,7 @@
 package org.apache.solr.servlet;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.output.CloseShieldOutputStream;
 import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.solr.common.params.CommonParams;
@@ -25,6 +26,7 @@ import org.apache.solr.core.SolrCore;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStreamWriter;
@@ -49,11 +51,14 @@ public final class LoadAdminUiServlet extends BaseSolrServlet {
     String admin = request.getRequestURI().substring(request.getContextPath().length());
     CoreContainer cores = (CoreContainer) request.getAttribute("org.apache.solr.CoreContainer");
     InputStream in = getServletContext().getResourceAsStream(admin);
+    Writer out = null;
     if(in != null && cores != null) {
       try {
         response.setCharacterEncoding("UTF-8");
         response.setContentType("text/html");
-        Writer out = new OutputStreamWriter(response.getOutputStream(), StandardCharsets.UTF_8);
+
+        // Protect container owned streams from being closed by us, see SOLR-8933
+        out = new OutputStreamWriter(new CloseShieldOutputStream(response.getOutputStream()), StandardCharsets.UTF_8);
 
         String html = IOUtils.toString(in, "UTF-8");
         Package pack = SolrCore.class.getPackage();
@@ -70,9 +75,9 @@ public final class LoadAdminUiServlet extends BaseSolrServlet {
         };
         
         out.write( StringUtils.replaceEach(html, search, replace) );
-        out.flush();
       } finally {
         IOUtils.closeQuietly(in);
+        IOUtils.closeQuietly(out);
       }
     } else {
       response.sendError(404);

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

@@ -0,0 +1,105 @@
+/*
+ * 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.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+
+import javax.servlet.ReadListener;
+import javax.servlet.ServletInputStream;
+
+import org.apache.solr.common.util.SuppressForbidden;
+
+/**
+ * Provides a convenient extension of the {@link ServletInputStream} class that can be subclassed by developers wishing
+ * to adapt the behavior of a Stream. One such example may be to override {@link #close()} to instead be a no-op as in
+ * SOLR-8933.
+ *
+ * This class implements the Wrapper or Decorator pattern. Methods default to calling through to the wrapped stream.
+ */
+@SuppressForbidden(reason = "delegate methods")
+public class ServletInputStreamWrapper extends ServletInputStream {
+  final ServletInputStream stream;
+  
+  public ServletInputStreamWrapper(ServletInputStream stream) throws IOException {
+    this.stream = stream;
+  }
+  
+  public int hashCode() {
+    return stream.hashCode();
+  }
+
+  public boolean equals(Object obj) {
+    return stream.equals(obj);
+  }
+
+  public int available() throws IOException {
+    return stream.available();
+  }
+
+  public void close() throws IOException {
+    stream.close();
+  }
+
+  public boolean isFinished() {
+    return stream.isFinished();
+  }
+
+  public boolean isReady() {
+    return stream.isReady();
+  }
+
+  public int read() throws IOException {
+    return stream.read();
+  }
+
+  public int read(byte[] b) throws IOException {
+    return stream.read(b);
+  }
+
+  public int read(byte[] b, int off, int len) throws IOException {
+    return stream.read(b, off, len);
+  }
+
+  public void mark(int readlimit) {
+    stream.mark(readlimit);
+  }
+
+  public boolean markSupported() {
+    return stream.markSupported();
+  }
+
+  public int readLine(byte[] b, int off, int len) throws IOException {
+    return stream.readLine(b, off, len);
+  }
+
+  public void reset() throws IOException {
+    stream.reset();
+  }
+
+  public void setReadListener(ReadListener arg0) {
+    stream.setReadListener(arg0);
+  }
+
+  public long skip(long n) throws IOException {
+    return stream.skip(n);
+  }
+
+  public String toString() {
+    return stream.toString();
+  }
+  
+}

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

@@ -0,0 +1,140 @@
+/*
+ * 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.
+ */
+package org.apache.solr.servlet;
+
+import java.io.IOException;
+
+import javax.servlet.ServletOutputStream;
+import javax.servlet.WriteListener;
+
+import org.apache.solr.common.util.SuppressForbidden;
+
+/**
+ * Provides a convenient extension of the {@link ServletOutputStream} class that can be subclassed by developers wishing
+ * to adapt the behavior of a Stream. One such example may be to override {@link #close()} to instead be a no-op as in
+ * SOLR-8933.
+ *
+ * This class implements the Wrapper or Decorator pattern. Methods default to calling through to the wrapped stream.
+ */
+@SuppressForbidden(reason = "delegate methods")
+public class ServletOutputStreamWrapper extends ServletOutputStream {
+  final ServletOutputStream stream;
+  
+  public ServletOutputStreamWrapper(ServletOutputStream stream) {
+    this.stream = stream;
+  }
+
+  public int hashCode() {
+    return stream.hashCode();
+  }
+
+  public boolean equals(Object obj) {
+    return stream.equals(obj);
+  }
+
+  public void flush() throws IOException {
+    stream.flush();
+  }
+
+  public void close() throws IOException {
+    stream.close();
+  }
+
+  public boolean isReady() {
+    return stream.isReady();
+  }
+
+  public void print(boolean arg0) throws IOException {
+    stream.print(arg0);
+  }
+
+  public void print(char c) throws IOException {
+    stream.print(c);
+  }
+
+  public void print(double d) throws IOException {
+    stream.print(d);
+  }
+
+  public void print(float f) throws IOException {
+    stream.print(f);
+  }
+
+  public void print(int i) throws IOException {
+    stream.print(i);
+  }
+
+  public void print(long l) throws IOException {
+    stream.print(l);
+  }
+
+  public void print(String arg0) throws IOException {
+    stream.print(arg0);
+  }
+
+  public void println() throws IOException {
+    stream.println();
+  }
+
+  public void println(boolean b) throws IOException {
+    stream.println(b);
+  }
+
+  public void println(char c) throws IOException {
+    stream.println(c);
+  }
+
+  public void println(double d) throws IOException {
+    stream.println(d);
+  }
+
+  public void println(float f) throws IOException {
+    stream.println(f);
+  }
+
+  public void println(int i) throws IOException {
+    stream.println(i);
+  }
+
+  public void println(long l) throws IOException {
+    stream.println(l);
+  }
+
+  public void println(String s) throws IOException {
+    stream.println(s);
+  }
+
+  public void setWriteListener(WriteListener arg0) {
+    stream.setWriteListener(arg0);
+  }
+
+  public void write(int b) throws IOException {
+    stream.write(b);
+  }
+
+  public void write(byte[] b) throws IOException {
+    stream.write(b);
+  }
+
+  public void write(byte[] b, int off, int len) throws IOException {
+    stream.write(b, off, len);
+  }
+
+  public String toString() {
+    return stream.toString();
+  }
+}

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

@@ -20,12 +20,18 @@ import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletInputStream;
+import javax.servlet.ServletOutputStream;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -36,6 +42,8 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.io.input.CloseShieldInputStream;
+import org.apache.commons.io.output.CloseShieldOutputStream;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.client.HttpClient;
 import org.apache.solr.common.SolrException;
@@ -66,6 +74,9 @@ public class SolrDispatchFilter extends BaseSolrFilter {
   protected String abortErrorMessage = null;
   protected HttpClient httpClient;
   private ArrayList<Pattern> excludePatterns;
+  
+  // Effectively immutable
+  private Boolean testMode = null;
 
   /**
    * Enum to define action that needs to be processed.
@@ -80,6 +91,19 @@ public class SolrDispatchFilter extends BaseSolrFilter {
   }
   
   public SolrDispatchFilter() {
+    // turn on test mode when running tests
+    assert testMode = true;
+    
+    if (testMode == null) {
+      testMode = false;
+    } else {
+      String tm = System.getProperty("solr.tests.doContainerStreamCloseAssert");
+      if (tm != null) {
+        testMode = Boolean.parseBoolean(tm);
+      } else {
+        testMode = true;
+      }
+    }
   }
 
   public static final String PROPERTIES_ATTRIBUTE = "solr.properties";
@@ -202,6 +226,10 @@ public class SolrDispatchFilter extends BaseSolrFilter {
       if (wrappedRequest.get() != null) {
         request = wrappedRequest.get();
       }
+
+      request = closeShield(request, retry);
+      response = closeShield(response, retry);
+      
       if (cores.getAuthenticationPlugin() != null) {
         log.debug("User principal: {}", ((HttpServletRequest) request).getUserPrincipal());
       }
@@ -298,4 +326,68 @@ public class SolrDispatchFilter extends BaseSolrFilter {
     }
     return true;
   }
+  
+  /**
+   * Wrap the request's input stream with a close shield, as if by a {@link CloseShieldInputStream}. If this is a
+   * retry, we will assume that the stream has already been wrapped and do nothing.
+   *
+   * @param request The request to wrap.
+   * @param retry If this is an original request or a retry.
+   * @return A request object with an {@link InputStream} that will ignore calls to close.
+   */
+  private ServletRequest closeShield(ServletRequest request, boolean retry) {
+    if (testMode && !retry) {
+      return new HttpServletRequestWrapper((HttpServletRequest) request) {
+        ServletInputStream stream;
+        
+        @Override
+        public ServletInputStream getInputStream() throws IOException {
+          // Lazy stream creation
+          if (stream == null) {
+            stream = new ServletInputStreamWrapper(super.getInputStream()) {
+              @Override
+              public void close() {
+                assert false : "Attempted close of request input stream.";
+              }
+            };
+          }
+          return stream;
+        }
+      };
+    } else {
+      return request;
+    }
+  }
+  
+  /**
+   * Wrap the response's output stream with a close shield, as if by a {@link CloseShieldOutputStream}. If this is a
+   * retry, we will assume that the stream has already been wrapped and do nothing.
+   *
+   * @param response The response to wrap.
+   * @param retry If this response corresponds to an original request or a retry.
+   * @return A response object with an {@link OutputStream} that will ignore calls to close.
+   */
+  private ServletResponse closeShield(ServletResponse response, boolean retry) {
+    if (testMode && !retry) {
+      return new HttpServletResponseWrapper((HttpServletResponse) response) {
+        ServletOutputStream stream;
+        
+        @Override
+        public ServletOutputStream getOutputStream() throws IOException {
+          // Lazy stream creation
+          if (stream == null) {
+            stream = new ServletOutputStreamWrapper(super.getOutputStream()) {
+              @Override
+              public void close() {
+                assert false : "Attempted close of response output stream.";
+              }
+            };
+          }
+          return stream;
+        }
+      };
+    } else {
+      return response;
+    }
+  }
 }

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

@@ -17,6 +17,7 @@
 package org.apache.solr.servlet;
 
 import javax.servlet.http.HttpServletRequest;
+
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -41,6 +42,7 @@ import java.util.Map;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
+import org.apache.commons.io.input.CloseShieldInputStream;
 import org.apache.lucene.util.IOUtils;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
@@ -484,7 +486,8 @@ public class SolrRequestParsers
 
     @Override
     public InputStream getStream() throws IOException {
-      return req.getInputStream();
+      // Protect container owned streams from being closed by us, see SOLR-8933
+      return new CloseShieldInputStream(req.getInputStream());
     }
   }
 
@@ -618,7 +621,8 @@ public class SolrRequestParsers
       final Charset charset = (cs == null) ? StandardCharsets.UTF_8 : Charset.forName(cs);
 
       try {
-        in = FastInputStream.wrap( in == null ? req.getInputStream() : in);
+        // Protect container owned streams from being closed by us, see SOLR-8933
+        in = FastInputStream.wrap( in == null ? new CloseShieldInputStream(req.getInputStream()) : in );
 
         final long bytesRead = parseFormDataContent(in, maxLength, charset, map, false);
         if (bytesRead == 0L && totalLength > 0L) {
@@ -737,7 +741,9 @@ public class SolrRequestParsers
       if (formdata.isFormData(req)) {
         String userAgent = req.getHeader("User-Agent");
         boolean isCurl = userAgent != null && userAgent.startsWith("curl/");
-        FastInputStream input = FastInputStream.wrap( req.getInputStream() );
+
+        // Protect container owned streams from being closed by us, see SOLR-8933
+        FastInputStream input = FastInputStream.wrap( new CloseShieldInputStream(req.getInputStream()) );
 
         if (isCurl) {
           SolrParams params = autodetect(req, streams, input);

solr/solrj/src/test/org/apache/solr/client/solrj/embedded/JettyWebappTest.java

@@ -63,7 +63,8 @@ public class JettyWebappTest extends SolrTestCaseJ4
     super.setUp();
     System.setProperty("solr.solr.home", SolrJettyTestBase.legacyExampleCollection1SolrHome());
     System.setProperty("tests.shardhandler.randomSeed", Long.toString(random().nextLong()));
-
+    System.setProperty("solr.tests.doContainerStreamCloseAssert", "false");
+    
     File dataDir = createTempDir().toFile();
     dataDir.mkdirs();
 
@@ -94,6 +95,7 @@ public class JettyWebappTest extends SolrTestCaseJ4
     } catch( Exception ex ) {}
     System.clearProperty("tests.shardhandler.randomSeed");
     System.clearProperty("solr.data.dir");
+    System.clearProperty("solr.tests.doContainerStreamCloseAssert");
     super.tearDown();
   }