You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2018/05/04 23:02:15 UTC
lucene-solr:master: SOLR-12290: Do not close any servlet streams and
improve our servlet stream closing prevention code for users and devs.
Repository: lucene-solr
Updated Branches:
refs/heads/master ad0ad2ec8 -> 296201055
SOLR-12290: Do not close any servlet streams and improve our servlet stream closing prevention code for users and devs.
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/29620105
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/29620105
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/29620105
Branch: refs/heads/master
Commit: 296201055f24f01e1610f2fb87aba7fa90b9dda1
Parents: ad0ad2e
Author: Mark Miller <ma...@apache.org>
Authored: Fri May 4 18:02:06 2018 -0500
Committer: Mark Miller <ma...@apache.org>
Committed: Fri May 4 18:02:06 2018 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +
.../org/apache/solr/handler/BlobHandler.java | 6 +-
.../apache/solr/handler/ReplicationHandler.java | 6 +-
.../solr/handler/loader/CSVLoaderBase.java | 75 +++++-----
.../solr/handler/loader/JavabinLoader.java | 16 +--
.../org/apache/solr/servlet/HttpSolrCall.java | 6 +-
.../apache/solr/servlet/LoadAdminUiServlet.java | 14 +-
.../solr/servlet/ServletInputStreamWrapper.java | 2 +-
.../servlet/ServletOutputStreamWrapper.java | 2 +-
.../apache/solr/servlet/SolrDispatchFilter.java | 139 ++++++++++++-------
.../apache/solr/servlet/SolrRequestParsers.java | 6 +-
11 files changed, 147 insertions(+), 128 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d4c2097..f74e2fd 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -206,6 +206,9 @@ Bug Fixes
* SOLR-12202: Fix errors in solr-exporter.cmd. (Minoru Osuka via koji)
+* SOLR-12290: Do not close any servlet streams and improve our servlet stream closing prevention code for users
+ and devs. (Mark Miller)
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
index 30301c0..a998657 100644
--- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
@@ -17,7 +17,6 @@
package org.apache.solr.handler;
import java.io.IOException;
-import java.io.InputStream;
import java.io.OutputStream;
import java.lang.invoke.MethodHandles;
import java.math.BigInteger;
@@ -109,9 +108,8 @@ public class BlobHandler extends RequestHandlerBase implements PluginInfoInitial
for (ContentStream stream : req.getContentStreams()) {
ByteBuffer payload;
- try (InputStream is = stream.getStream()) {
- payload = SimplePostTool.inputStreamToByteArray(is, maxSize);
- }
+ payload = SimplePostTool.inputStreamToByteArray(stream.getStream(), maxSize);
+
MessageDigest m = MessageDigest.getInstance("MD5");
m.update(payload.array(), payload.position(), payload.limit());
String md5 = new BigInteger(1, m.digest()).toString(16);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
index 1707c80..6afb8a0 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -56,6 +56,7 @@ import java.util.zip.Checksum;
import java.util.zip.DeflaterOutputStream;
import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.output.CloseShieldOutputStream;
import org.apache.lucene.codecs.CodecUtil;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexCommit;
@@ -1515,6 +1516,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
}
protected void createOutputStream(OutputStream out) {
+ out = new CloseShieldOutputStream(out); // DeflaterOutputStream requires a close call, but don't close the request outputstream
if (Boolean.parseBoolean(compress)) {
fos = new FastOutputStream(new DeflaterOutputStream(out));
} else {
@@ -1579,7 +1581,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
}
if (read != buf.length) {
writeNothingAndFlush();
- fos.close();
+ fos.close(); // we close because DeflaterOutputStream requires a close call, but but the request outputstream is protected
break;
}
offset += read;
@@ -1638,7 +1640,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
long bytesRead = channel.read(bb);
if (bytesRead <= 0) {
writeNothingAndFlush();
- fos.close();
+ fos.close(); // we close because DeflaterOutputStream requires a close call, but but the request outputstream is protected
break;
}
fos.writeInt((int) bytesRead);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
index b503fa3..fd8935d 100644
--- a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
@@ -28,7 +28,6 @@ import org.apache.solr.update.*;
import org.apache.solr.update.processor.UpdateRequestProcessor;
import org.apache.solr.internal.csv.CSVStrategy;
import org.apache.solr.internal.csv.CSVParser;
-import org.apache.commons.io.IOUtils;
import java.util.regex.Pattern;
import java.util.List;
@@ -315,54 +314,48 @@ abstract class CSVLoaderBase extends ContentStreamLoader {
/** load the CSV input */
@Override
- public void load(SolrQueryRequest req, SolrQueryResponse rsp, ContentStream stream, UpdateRequestProcessor processor) throws IOException {
+ public void load(SolrQueryRequest req, SolrQueryResponse rsp, ContentStream stream, UpdateRequestProcessor processor)
+ throws IOException {
errHeader = "CSVLoader: input=" + stream.getSourceInfo();
- Reader reader = null;
- try {
- reader = stream.getReader();
- if (skipLines>0) {
- if (!(reader instanceof BufferedReader)) {
- reader = new BufferedReader(reader);
- }
- BufferedReader r = (BufferedReader)reader;
- for (int i=0; i<skipLines; i++) {
- r.readLine();
- }
+ Reader reader = stream.getReader();
+ if (skipLines > 0) {
+ if (!(reader instanceof BufferedReader)) {
+ reader = new BufferedReader(reader);
}
-
- CSVParser parser = new CSVParser(reader, strategy);
-
- // parse the fieldnames from the header of the file
- if (fieldnames==null) {
- fieldnames = parser.getLine();
- if (fieldnames==null) {
- throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,"Expected fieldnames in CSV input");
- }
- prepareFields();
+ BufferedReader r = (BufferedReader) reader;
+ for (int i = 0; i < skipLines; i++) {
+ r.readLine();
}
+ }
- // read the rest of the CSV file
- for(;;) {
- int line = parser.getLineNumber(); // for error reporting in MT mode
- String[] vals = null;
- try {
- vals = parser.getLine();
- } catch (IOException e) {
- //Catch the exception and rethrow it with more line information
- input_err("can't read line: " + line, null, line, e);
- }
- if (vals==null) break;
+ CSVParser parser = new CSVParser(reader, strategy);
- if (vals.length != fieldnames.length) {
- input_err("expected "+fieldnames.length+" values but got "+vals.length, vals, line);
- }
+ // parse the fieldnames from the header of the file
+ if (fieldnames == null) {
+ fieldnames = parser.getLine();
+ if (fieldnames == null) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Expected fieldnames in CSV input");
+ }
+ prepareFields();
+ }
- addDoc(line,vals);
+ // read the rest of the CSV file
+ for (;;) {
+ int line = parser.getLineNumber(); // for error reporting in MT mode
+ String[] vals = null;
+ try {
+ vals = parser.getLine();
+ } catch (IOException e) {
+ // Catch the exception and rethrow it with more line information
+ input_err("can't read line: " + line, null, line, e);
}
- } finally{
- if (reader != null) {
- IOUtils.closeQuietly(reader);
+ if (vals == null) break;
+
+ if (vals.length != fieldnames.length) {
+ input_err("expected " + fieldnames.length + " values but got " + vals.length, vals, line);
}
+
+ addDoc(line, vals);
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
index f502a8e..aca3df4 100644
--- a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
+++ b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
@@ -48,20 +48,8 @@ import org.apache.solr.update.processor.UpdateRequestProcessor;
public class JavabinLoader extends ContentStreamLoader {
@Override
- public void load(SolrQueryRequest req, SolrQueryResponse rsp, ContentStream stream, UpdateRequestProcessor processor) throws Exception {
- InputStream is = null;
- try {
- is = stream.getStream();
- parseAndLoadDocs(req, rsp, is, processor);
- } finally {
- if(is != null) {
- is.close();
- }
- }
- }
-
- private void parseAndLoadDocs(final SolrQueryRequest req, SolrQueryResponse rsp, InputStream stream,
- final UpdateRequestProcessor processor) throws IOException {
+ public void load(SolrQueryRequest req, SolrQueryResponse rsp, ContentStream cs, UpdateRequestProcessor processor) throws Exception {
+ InputStream stream = cs.getStream();
UpdateRequest update = null;
JavaBinUpdateRequestCodec.StreamingUpdateHandler handler = new JavaBinUpdateRequestCodec.StreamingUpdateHandler() {
private AddUpdateCommand addCmd = null;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/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 50fa71c..5e4dd78 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -39,8 +39,6 @@ import java.util.Set;
import java.util.concurrent.TimeUnit;
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;
@@ -590,7 +588,7 @@ public class HttpSolrCall {
} else if (isPostOrPutRequest) {
HttpEntityEnclosingRequestBase entityRequest =
"POST".equals(req.getMethod()) ? new HttpPost(urlstr) : new HttpPut(urlstr);
- InputStream in = new CloseShieldInputStream(req.getInputStream()); // Prevent close of container streams
+ InputStream in = req.getInputStream();
HttpEntity entity = new InputStreamEntity(in, req.getContentLength());
entityRequest.setEntity(entity);
method = entityRequest;
@@ -785,7 +783,7 @@ public class HttpSolrCall {
}
if (Method.HEAD != reqMethod) {
- OutputStream out = new CloseShieldOutputStream(response.getOutputStream()); // Prevent close of container streams, see SOLR-8933
+ OutputStream out = response.getOutputStream();
QueryResponseWriterUtil.writeQueryResponse(out, responseWriter, solrReq, solrRsp, ct);
}
//else http HEAD request, nothing to write out, waited this long just to get ContentType
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
index c496ce1..1aa1137 100644
--- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
+++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
@@ -17,7 +17,6 @@
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;
@@ -41,10 +40,12 @@ import java.nio.charset.StandardCharsets;
public final class LoadAdminUiServlet extends BaseSolrServlet {
@Override
- public void doGet(HttpServletRequest request,
- HttpServletResponse response)
+ public void doGet(HttpServletRequest _request,
+ HttpServletResponse _response)
throws IOException {
-
+ HttpServletRequest request = SolrDispatchFilter.closeShield(_request, false);
+ HttpServletResponse response = SolrDispatchFilter.closeShield(_response, false);
+
response.addHeader("X-Frame-Options", "DENY"); // security: SOLR-7966 - avoid clickjacking for admin interface
// This attribute is set by the SolrDispatchFilter
@@ -57,8 +58,8 @@ public final class LoadAdminUiServlet extends BaseSolrServlet {
response.setCharacterEncoding("UTF-8");
response.setContentType("text/html");
- // Protect container owned streams from being closed by us, see SOLR-8933
- out = new OutputStreamWriter(new CloseShieldOutputStream(response.getOutputStream()), StandardCharsets.UTF_8);
+ // Don't close this! - see SOLR-8933
+ out = new OutputStreamWriter(response.getOutputStream(), StandardCharsets.UTF_8);
String html = IOUtils.toString(in, "UTF-8");
Package pack = SolrCore.class.getPackage();
@@ -77,7 +78,6 @@ public final class LoadAdminUiServlet extends BaseSolrServlet {
out.write( StringUtils.replaceEach(html, search, replace) );
} finally {
IOUtils.closeQuietly(in);
- IOUtils.closeQuietly(out);
}
} else {
response.sendError(404);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java
index d229bf7..826dc59 100644
--- a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java
+++ b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java
@@ -32,7 +32,7 @@ import org.apache.solr.common.util.SuppressForbidden;
*/
@SuppressForbidden(reason = "delegate methods")
public class ServletInputStreamWrapper extends ServletInputStream {
- final ServletInputStream stream;
+ ServletInputStream stream;
public ServletInputStreamWrapper(ServletInputStream stream) throws IOException {
this.stream = stream;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java
index d12c3bd..1164146 100644
--- a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java
+++ b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java
@@ -32,7 +32,7 @@ import org.apache.solr.common.util.SuppressForbidden;
*/
@SuppressForbidden(reason = "delegate methods")
public class ServletOutputStreamWrapper extends ServletOutputStream {
- final ServletOutputStream stream;
+ ServletOutputStream stream;
public ServletOutputStreamWrapper(ServletOutputStream stream) {
this.stream = stream;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/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 fc0c28f..8f32a7d 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -37,20 +37,20 @@ import java.util.regex.Pattern;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
+import javax.servlet.ReadListener;
import javax.servlet.ServletException;
import javax.servlet.ServletInputStream;
import javax.servlet.ServletOutputStream;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.UnavailableException;
+import javax.servlet.WriteListener;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
import org.apache.commons.io.FileCleaningTracker;
-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.lucene.util.Version;
@@ -98,8 +98,6 @@ public class SolrDispatchFilter extends BaseSolrFilter {
protected HttpClient httpClient;
private ArrayList<Pattern> excludePatterns;
- // Effectively immutable
- private Boolean testMode = null;
private boolean isV2Enabled = !"true".equals(System.getProperty("disable.v2.api", "false"));
private final String metricTag = Integer.toHexString(hashCode());
@@ -119,19 +117,6 @@ 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";
@@ -341,8 +326,8 @@ public class SolrDispatchFilter extends BaseSolrFilter {
public void doFilter(ServletRequest _request, ServletResponse _response, FilterChain chain, boolean retry) throws IOException, ServletException {
if (!(_request instanceof HttpServletRequest)) return;
- HttpServletRequest request = (HttpServletRequest)_request;
- HttpServletResponse response = (HttpServletResponse)_response;
+ HttpServletRequest request = closeShield((HttpServletRequest)_request, retry);
+ HttpServletResponse response = closeShield((HttpServletResponse)_response, retry);
try {
@@ -387,7 +372,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
}
}
- HttpSolrCall call = getHttpSolrCall(closeShield(request, retry), closeShield(response, retry), retry);
+ HttpSolrCall call = getHttpSolrCall(request, response, retry);
ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
try {
Action result = call.call();
@@ -486,31 +471,82 @@ public class SolrDispatchFilter extends BaseSolrFilter {
return true;
}
+ public static class ClosedServletInputStream extends ServletInputStream {
+
+ public static final ClosedServletInputStream CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream();
+
+ @Override
+ public int read() {
+ return -1;
+ }
+
+ @Override
+ public boolean isFinished() {
+ return false;
+ }
+
+ @Override
+ public boolean isReady() {
+ return false;
+ }
+
+ @Override
+ public void setReadListener(ReadListener arg0) {}
+ }
+
+ public static class ClosedServletOutputStream extends ServletOutputStream {
+
+ public static final ClosedServletOutputStream CLOSED_SERVLET_OUTPUT_STREAM = new ClosedServletOutputStream();
+
+ @Override
+ public void write(final int b) throws IOException {
+ throw new IOException("write(" + b + ") failed: stream is closed");
+ }
+
+ @Override
+ public void flush() throws IOException {
+ throw new IOException("flush() failed: stream is closed");
+ }
+
+ @Override
+ public boolean isReady() {
+ return false;
+ }
+
+ @Override
+ public void setWriteListener(WriteListener arg0) {
+ throw new RuntimeException("setWriteListener() failed: stream is closed");
+ }
+ }
+
/**
- * Wrap the request's input stream with a close shield, as if by a {@link CloseShieldInputStream}. If this is a
+ * Wrap the request's input stream with a close shield. If this is a
* retry, we will assume that the stream has already been wrapped and do nothing.
*
+ * Only the container should ever actually close the servlet output stream.
+ *
* @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 HttpServletRequest closeShield(HttpServletRequest request, boolean retry) {
- if (testMode && !retry) {
+ public static HttpServletRequest closeShield(HttpServletRequest request, boolean retry) {
+ if (!retry) {
return new HttpServletRequestWrapper(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;
+
+ return new ServletInputStreamWrapper(super.getInputStream()) {
+ @Override
+ public void close() {
+ // even though we skip closes, we let local tests know not to close so that a full understanding can take
+ // place
+ assert Thread.currentThread().getStackTrace()[2].getClassName().matches(
+ "org\\.apache\\.(?:solr|lucene).*") ? false : true : "Attempted close of request input stream - never do this, you will spoil connection reuse and possibly disrupt a client";
+ this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM;
+ }
+ };
+
}
};
} else {
@@ -519,31 +555,34 @@ public class SolrDispatchFilter extends BaseSolrFilter {
}
/**
- * Wrap the response's output stream with a close shield, as if by a {@link CloseShieldOutputStream}. If this is a
+ * Wrap the response's output stream with a close shield. If this is a
* retry, we will assume that the stream has already been wrapped and do nothing.
*
+ * Only the container should ever actually close the servlet request stream.
+ *
* @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 HttpServletResponse closeShield(HttpServletResponse response, boolean retry) {
- if (testMode && !retry) {
+ public static HttpServletResponse closeShield(HttpServletResponse response, boolean retry) {
+ if (!retry) {
return new HttpServletResponseWrapper(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;
+
+ return new ServletOutputStreamWrapper(super.getOutputStream()) {
+ @Override
+ public void close() {
+ // even though we skip closes, we let local tests know not to close so that a full understanding can take
+ // place
+ assert Thread.currentThread().getStackTrace()[2].getClassName().matches(
+ "org\\.apache\\.(?:solr|lucene).*") ? false : true : "Attempted close of response output stream - never do this, you will spoil connection reuse and possibly disrupt a client";
+ stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM;
+ }
+ };
}
+
};
} else {
return response;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
----------------------------------------------------------------------
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 60b6d2f..4bcb8d8 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -519,8 +519,7 @@ public class SolrRequestParsers
@Override
public InputStream getStream() throws IOException {
- // Protect container owned streams from being closed by us, see SOLR-8933
- return new CloseShieldInputStream(req.getInputStream());
+ return req.getInputStream();
}
}
@@ -767,8 +766,7 @@ public class SolrRequestParsers
String userAgent = req.getHeader("User-Agent");
boolean isCurl = userAgent != null && userAgent.startsWith("curl/");
- // Protect container owned streams from being closed by us, see SOLR-8933
- FastInputStream input = FastInputStream.wrap( new CloseShieldInputStream(req.getInputStream()) );
+ FastInputStream input = FastInputStream.wrap(req.getInputStream());
if (isCurl) {
SolrParams params = autodetect(req, streams, input);
RE: lucene-solr:master: SOLR-12290: Do not close any servlet streams and improve our servlet stream closing prevention code for users and devs.
Posted by Uwe Schindler <uw...@thetaphi.de>.
This commit breaks TestCSVLoader on Windows. It looks like Solr no longer closes any content streams that are passed separately to the servlet stream:
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestCSVLoader -Dtests.method=testLiteral -Dtests.seed=B40869AE03F63CBC -Dtests.locale=ms -Dtests.timezone=ECT -Dtests.asserts=true -Dtests.file.encoding=UTF-8
[junit4] ERROR 0.06s | TestCSVLoader.testLiteral <<<
[junit4] > Throwable #1: java.nio.file.FileSystemException: C:\Users\Uwe Schindler\Projects\lucene\trunk-lusolr1\solr\build\solr-core\test\J0\temp\solr.handler.TestCSVLoader_B40869AE03F63CBC-001\TestCSVLoader-006\solr_tmp.csv: Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet wird.
[junit4] > at __randomizedtesting.SeedInfo.seed([B40869AE03F63CBC:B2ACAF677D87833E]:0)
[junit4] > at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
[junit4] > at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
[junit4] > at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
[junit4] > at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
[junit4] > at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
[junit4] > at java.nio.file.Files.delete(Files.java:1126)
[junit4] > at org.apache.solr.handler.TestCSVLoader.tearDown(TestCSVLoader.java:64)
[junit4] > at java.lang.Thread.run(Thread.java:748)
I am not sure behind the reasoning of the whole thing, but IMHO, the better way to handle this is:
- Wrap the ServletInput/ServletOutput streams with a CloseShieldXxxxStream and only pass this around. This allows that code anywhere in solr is free to close any streams correctly, but the ServletStreams are kept open by the shield.
The reason behind everything here is a bug in Jetty. Jetty should prevent closing the stream!
I will reopen this issue. We now have a file descriptor leak also on Linux/Mac!
Uwe
-----
Uwe Schindler
Achterdiek 19, D-28357 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de
> -----Original Message-----
> From: markrmiller@apache.org <ma...@apache.org>
> Sent: Saturday, May 5, 2018 1:02 AM
> To: commits@lucene.apache.org
> Subject: lucene-solr:master: SOLR-12290: Do not close any servlet streams
> and improve our servlet stream closing prevention code for users and devs.
>
> Repository: lucene-solr
> Updated Branches:
> refs/heads/master ad0ad2ec8 -> 296201055
>
>
> SOLR-12290: Do not close any servlet streams and improve our servlet
> stream closing prevention code for users and devs.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
> Commit: http://git-wip-us.apache.org/repos/asf/lucene-
> solr/commit/29620105
> Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/29620105
> Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/29620105
>
> Branch: refs/heads/master
> Commit: 296201055f24f01e1610f2fb87aba7fa90b9dda1
> Parents: ad0ad2e
> Author: Mark Miller <ma...@apache.org>
> Authored: Fri May 4 18:02:06 2018 -0500
> Committer: Mark Miller <ma...@apache.org>
> Committed: Fri May 4 18:02:06 2018 -0500
>
> ----------------------------------------------------------------------
> solr/CHANGES.txt | 3 +
> .../org/apache/solr/handler/BlobHandler.java | 6 +-
> .../apache/solr/handler/ReplicationHandler.java | 6 +-
> .../solr/handler/loader/CSVLoaderBase.java | 75 +++++-----
> .../solr/handler/loader/JavabinLoader.java | 16 +--
> .../org/apache/solr/servlet/HttpSolrCall.java | 6 +-
> .../apache/solr/servlet/LoadAdminUiServlet.java | 14 +-
> .../solr/servlet/ServletInputStreamWrapper.java | 2 +-
> .../servlet/ServletOutputStreamWrapper.java | 2 +-
> .../apache/solr/servlet/SolrDispatchFilter.java | 139 ++++++++++++-------
> .../apache/solr/servlet/SolrRequestParsers.java | 6 +-
> 11 files changed, 147 insertions(+), 128 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/CHANGES.txt
> ----------------------------------------------------------------------
> diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
> index d4c2097..f74e2fd 100644
> --- a/solr/CHANGES.txt
> +++ b/solr/CHANGES.txt
> @@ -206,6 +206,9 @@ Bug Fixes
>
> * SOLR-12202: Fix errors in solr-exporter.cmd. (Minoru Osuka via koji)
>
> +* SOLR-12290: Do not close any servlet streams and improve our servlet
> stream closing prevention code for users
> + and devs. (Mark Miller)
> +
> Optimizations
> ----------------------
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/BlobHandl
> er.java
> ----------------------------------------------------------------------
> diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> index 30301c0..a998657 100644
> --- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> +++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
> @@ -17,7 +17,6 @@
> package org.apache.solr.handler;
>
> import java.io.IOException;
> -import java.io.InputStream;
> import java.io.OutputStream;
> import java.lang.invoke.MethodHandles;
> import java.math.BigInteger;
> @@ -109,9 +108,8 @@ public class BlobHandler extends
> RequestHandlerBase implements PluginInfoInitial
>
> for (ContentStream stream : req.getContentStreams()) {
> ByteBuffer payload;
> - try (InputStream is = stream.getStream()) {
> - payload = SimplePostTool.inputStreamToByteArray(is, maxSize);
> - }
> + payload =
> SimplePostTool.inputStreamToByteArray(stream.getStream(), maxSize);
> +
> MessageDigest m = MessageDigest.getInstance("MD5");
> m.update(payload.array(), payload.position(), payload.limit());
> String md5 = new BigInteger(1, m.digest()).toString(16);
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/Replication
> Handler.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> index 1707c80..6afb8a0 100644
> --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
> @@ -56,6 +56,7 @@ import java.util.zip.Checksum;
> import java.util.zip.DeflaterOutputStream;
>
> import org.apache.commons.io.IOUtils;
> +import org.apache.commons.io.output.CloseShieldOutputStream;
> import org.apache.lucene.codecs.CodecUtil;
> import org.apache.lucene.index.DirectoryReader;
> import org.apache.lucene.index.IndexCommit;
> @@ -1515,6 +1516,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
> }
>
> protected void createOutputStream(OutputStream out) {
> + out = new CloseShieldOutputStream(out); // DeflaterOutputStream
> requires a close call, but don't close the request outputstream
> if (Boolean.parseBoolean(compress)) {
> fos = new FastOutputStream(new DeflaterOutputStream(out));
> } else {
> @@ -1579,7 +1581,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
> }
> if (read != buf.length) {
> writeNothingAndFlush();
> - fos.close();
> + fos.close(); // we close because DeflaterOutputStream requires a
> close call, but but the request outputstream is protected
> break;
> }
> offset += read;
> @@ -1638,7 +1640,7 @@ public class ReplicationHandler extends
> RequestHandlerBase implements SolrCoreAw
> long bytesRead = channel.read(bb);
> if (bytesRead <= 0) {
> writeNothingAndFlush();
> - fos.close();
> + fos.close(); // we close because DeflaterOutputStream requires a
> close call, but but the request outputstream is protected
> break;
> }
> fos.writeInt((int) bytesRead);
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/CSV
> LoaderBase.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> index b503fa3..fd8935d 100644
> ---
> a/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> +++
> b/solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
> @@ -28,7 +28,6 @@ import org.apache.solr.update.*;
> import org.apache.solr.update.processor.UpdateRequestProcessor;
> import org.apache.solr.internal.csv.CSVStrategy;
> import org.apache.solr.internal.csv.CSVParser;
> -import org.apache.commons.io.IOUtils;
>
> import java.util.regex.Pattern;
> import java.util.List;
> @@ -315,54 +314,48 @@ abstract class CSVLoaderBase extends
> ContentStreamLoader {
>
> /** load the CSV input */
> @Override
> - public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor) throws
> IOException {
> + public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor)
> + throws IOException {
> errHeader = "CSVLoader: input=" + stream.getSourceInfo();
> - Reader reader = null;
> - try {
> - reader = stream.getReader();
> - if (skipLines>0) {
> - if (!(reader instanceof BufferedReader)) {
> - reader = new BufferedReader(reader);
> - }
> - BufferedReader r = (BufferedReader)reader;
> - for (int i=0; i<skipLines; i++) {
> - r.readLine();
> - }
> + Reader reader = stream.getReader();
> + if (skipLines > 0) {
> + if (!(reader instanceof BufferedReader)) {
> + reader = new BufferedReader(reader);
> }
> -
> - CSVParser parser = new CSVParser(reader, strategy);
> -
> - // parse the fieldnames from the header of the file
> - if (fieldnames==null) {
> - fieldnames = parser.getLine();
> - if (fieldnames==null) {
> - throw new SolrException(
> SolrException.ErrorCode.BAD_REQUEST,"Expected fieldnames in CSV input");
> - }
> - prepareFields();
> + BufferedReader r = (BufferedReader) reader;
> + for (int i = 0; i < skipLines; i++) {
> + r.readLine();
> }
> + }
>
> - // read the rest of the CSV file
> - for(;;) {
> - int line = parser.getLineNumber(); // for error reporting in MT mode
> - String[] vals = null;
> - try {
> - vals = parser.getLine();
> - } catch (IOException e) {
> - //Catch the exception and rethrow it with more line information
> - input_err("can't read line: " + line, null, line, e);
> - }
> - if (vals==null) break;
> + CSVParser parser = new CSVParser(reader, strategy);
>
> - if (vals.length != fieldnames.length) {
> - input_err("expected "+fieldnames.length+" values but got
> "+vals.length, vals, line);
> - }
> + // parse the fieldnames from the header of the file
> + if (fieldnames == null) {
> + fieldnames = parser.getLine();
> + if (fieldnames == null) {
> + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
> "Expected fieldnames in CSV input");
> + }
> + prepareFields();
> + }
>
> - addDoc(line,vals);
> + // read the rest of the CSV file
> + for (;;) {
> + int line = parser.getLineNumber(); // for error reporting in MT mode
> + String[] vals = null;
> + try {
> + vals = parser.getLine();
> + } catch (IOException e) {
> + // Catch the exception and rethrow it with more line information
> + input_err("can't read line: " + line, null, line, e);
> }
> - } finally{
> - if (reader != null) {
> - IOUtils.closeQuietly(reader);
> + if (vals == null) break;
> +
> + if (vals.length != fieldnames.length) {
> + input_err("expected " + fieldnames.length + " values but got " +
> vals.length, vals, line);
> }
> +
> + addDoc(line, vals);
> }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/handler/loader/Jav
> abinLoader.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> index f502a8e..aca3df4 100644
> --- a/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> +++
> b/solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
> @@ -48,20 +48,8 @@ import
> org.apache.solr.update.processor.UpdateRequestProcessor;
> public class JavabinLoader extends ContentStreamLoader {
>
> @Override
> - public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream stream, UpdateRequestProcessor processor) throws
> Exception {
> - InputStream is = null;
> - try {
> - is = stream.getStream();
> - parseAndLoadDocs(req, rsp, is, processor);
> - } finally {
> - if(is != null) {
> - is.close();
> - }
> - }
> - }
> -
> - private void parseAndLoadDocs(final SolrQueryRequest req,
> SolrQueryResponse rsp, InputStream stream,
> - final UpdateRequestProcessor processor) throws
> IOException {
> + public void load(SolrQueryRequest req, SolrQueryResponse rsp,
> ContentStream cs, UpdateRequestProcessor processor) throws Exception {
> + InputStream stream = cs.getStream();
> UpdateRequest update = null;
> JavaBinUpdateRequestCodec.StreamingUpdateHandler handler = new
> JavaBinUpdateRequestCodec.StreamingUpdateHandler() {
> private AddUpdateCommand addCmd = null;
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/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 50fa71c..5e4dd78 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
> @@ -39,8 +39,6 @@ import java.util.Set;
> import java.util.concurrent.TimeUnit;
>
> 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;
> @@ -590,7 +588,7 @@ public class HttpSolrCall {
> } else if (isPostOrPutRequest) {
> HttpEntityEnclosingRequestBase entityRequest =
> "POST".equals(req.getMethod()) ? new HttpPost(urlstr) : new
> HttpPut(urlstr);
> - InputStream in = new CloseShieldInputStream(req.getInputStream()); //
> Prevent close of container streams
> + InputStream in = req.getInputStream();
> HttpEntity entity = new InputStreamEntity(in, req.getContentLength());
> entityRequest.setEntity(entity);
> method = entityRequest;
> @@ -785,7 +783,7 @@ public class HttpSolrCall {
> }
>
> if (Method.HEAD != reqMethod) {
> - OutputStream out = new
> CloseShieldOutputStream(response.getOutputStream()); // Prevent close of
> container streams, see SOLR-8933
> + OutputStream out = response.getOutputStream();
> QueryResponseWriterUtil.writeQueryResponse(out, responseWriter,
> solrReq, solrRsp, ct);
> }
> //else http HEAD request, nothing to write out, waited this long just to
> get ContentType
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/LoadAdmin
> UiServlet.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> index c496ce1..1aa1137 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
> @@ -17,7 +17,6 @@
> 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;
> @@ -41,10 +40,12 @@ import java.nio.charset.StandardCharsets;
> public final class LoadAdminUiServlet extends BaseSolrServlet {
>
> @Override
> - public void doGet(HttpServletRequest request,
> - HttpServletResponse response)
> + public void doGet(HttpServletRequest _request,
> + HttpServletResponse _response)
> throws IOException {
> -
> + HttpServletRequest request = SolrDispatchFilter.closeShield(_request,
> false);
> + HttpServletResponse response =
> SolrDispatchFilter.closeShield(_response, false);
> +
> response.addHeader("X-Frame-Options", "DENY"); // security: SOLR-7966
> - avoid clickjacking for admin interface
>
> // This attribute is set by the SolrDispatchFilter
> @@ -57,8 +58,8 @@ public final class LoadAdminUiServlet extends
> BaseSolrServlet {
> response.setCharacterEncoding("UTF-8");
> response.setContentType("text/html");
>
> - // Protect container owned streams from being closed by us, see SOLR-
> 8933
> - out = new OutputStreamWriter(new
> CloseShieldOutputStream(response.getOutputStream()),
> StandardCharsets.UTF_8);
> + // Don't close this! - see SOLR-8933
> + out = new OutputStreamWriter(response.getOutputStream(),
> StandardCharsets.UTF_8);
>
> String html = IOUtils.toString(in, "UTF-8");
> Package pack = SolrCore.class.getPackage();
> @@ -77,7 +78,6 @@ public final class LoadAdminUiServlet extends
> BaseSolrServlet {
> out.write( StringUtils.replaceEach(html, search, replace) );
> } finally {
> IOUtils.closeQuietly(in);
> - IOUtils.closeQuietly(out);
> }
> } else {
> response.sendError(404);
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletInput
> StreamWrapper.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> index d229bf7..826dc59 100644
> ---
> a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> +++
> b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.ja
> va
> @@ -32,7 +32,7 @@ import
> org.apache.solr.common.util.SuppressForbidden;
> */
> @SuppressForbidden(reason = "delegate methods")
> public class ServletInputStreamWrapper extends ServletInputStream {
> - final ServletInputStream stream;
> + ServletInputStream stream;
>
> public ServletInputStreamWrapper(ServletInputStream stream) throws
> IOException {
> this.stream = stream;
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/ServletOutp
> utStreamWrapper.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.j
> ava
> b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.
> java
> index d12c3bd..1164146 100644
> ---
> a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.j
> ava
> +++
> b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.
> java
> @@ -32,7 +32,7 @@ import
> org.apache.solr.common.util.SuppressForbidden;
> */
> @SuppressForbidden(reason = "delegate methods")
> public class ServletOutputStreamWrapper extends ServletOutputStream {
> - final ServletOutputStream stream;
> + ServletOutputStream stream;
>
> public ServletOutputStreamWrapper(ServletOutputStream stream) {
> this.stream = stream;
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/SolrDispatch
> Filter.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 fc0c28f..8f32a7d 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
> @@ -37,20 +37,20 @@ import java.util.regex.Pattern;
>
> import javax.servlet.FilterChain;
> import javax.servlet.FilterConfig;
> +import javax.servlet.ReadListener;
> import javax.servlet.ServletException;
> import javax.servlet.ServletInputStream;
> import javax.servlet.ServletOutputStream;
> import javax.servlet.ServletRequest;
> import javax.servlet.ServletResponse;
> import javax.servlet.UnavailableException;
> +import javax.servlet.WriteListener;
> import javax.servlet.http.HttpServletRequest;
> import javax.servlet.http.HttpServletRequestWrapper;
> import javax.servlet.http.HttpServletResponse;
> import javax.servlet.http.HttpServletResponseWrapper;
>
> import org.apache.commons.io.FileCleaningTracker;
> -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.lucene.util.Version;
> @@ -98,8 +98,6 @@ public class SolrDispatchFilter extends BaseSolrFilter {
> protected HttpClient httpClient;
> private ArrayList<Pattern> excludePatterns;
>
> - // Effectively immutable
> - private Boolean testMode = null;
> private boolean isV2Enabled =
> !"true".equals(System.getProperty("disable.v2.api", "false"));
>
> private final String metricTag = Integer.toHexString(hashCode());
> @@ -119,19 +117,6 @@ 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";
> @@ -341,8 +326,8 @@ public class SolrDispatchFilter extends BaseSolrFilter {
>
> public void doFilter(ServletRequest _request, ServletResponse _response,
> FilterChain chain, boolean retry) throws IOException, ServletException {
> if (!(_request instanceof HttpServletRequest)) return;
> - HttpServletRequest request = (HttpServletRequest)_request;
> - HttpServletResponse response = (HttpServletResponse)_response;
> + HttpServletRequest request = closeShield((HttpServletRequest)_request,
> retry);
> + HttpServletResponse response =
> closeShield((HttpServletResponse)_response, retry);
>
> try {
>
> @@ -387,7 +372,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
> }
> }
>
> - HttpSolrCall call = getHttpSolrCall(closeShield(request, retry),
> closeShield(response, retry), retry);
> + HttpSolrCall call = getHttpSolrCall(request, response, retry);
> ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
> try {
> Action result = call.call();
> @@ -486,31 +471,82 @@ public class SolrDispatchFilter extends
> BaseSolrFilter {
> return true;
> }
>
> + public static class ClosedServletInputStream extends ServletInputStream {
> +
> + public static final ClosedServletInputStream
> CLOSED_SERVLET_INPUT_STREAM = new ClosedServletInputStream();
> +
> + @Override
> + public int read() {
> + return -1;
> + }
> +
> + @Override
> + public boolean isFinished() {
> + return false;
> + }
> +
> + @Override
> + public boolean isReady() {
> + return false;
> + }
> +
> + @Override
> + public void setReadListener(ReadListener arg0) {}
> + }
> +
> + public static class ClosedServletOutputStream extends
> ServletOutputStream {
> +
> + public static final ClosedServletOutputStream
> CLOSED_SERVLET_OUTPUT_STREAM = new ClosedServletOutputStream();
> +
> + @Override
> + public void write(final int b) throws IOException {
> + throw new IOException("write(" + b + ") failed: stream is closed");
> + }
> +
> + @Override
> + public void flush() throws IOException {
> + throw new IOException("flush() failed: stream is closed");
> + }
> +
> + @Override
> + public boolean isReady() {
> + return false;
> + }
> +
> + @Override
> + public void setWriteListener(WriteListener arg0) {
> + throw new RuntimeException("setWriteListener() failed: stream is
> closed");
> + }
> + }
> +
> /**
> - * Wrap the request's input stream with a close shield, as if by a {@link
> CloseShieldInputStream}. If this is a
> + * Wrap the request's input stream with a close shield. If this is a
> * retry, we will assume that the stream has already been wrapped and do
> nothing.
> *
> + * Only the container should ever actually close the servlet output stream.
> + *
> * @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 HttpServletRequest closeShield(HttpServletRequest request,
> boolean retry) {
> - if (testMode && !retry) {
> + public static HttpServletRequest closeShield(HttpServletRequest request,
> boolean retry) {
> + if (!retry) {
> return new HttpServletRequestWrapper(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;
> +
> + return new ServletInputStreamWrapper(super.getInputStream()) {
> + @Override
> + public void close() {
> + // even though we skip closes, we let local tests know not to close so
> that a full understanding can take
> + // place
> + assert
> Thread.currentThread().getStackTrace()[2].getClassName().matches(
> + "org\\.apache\\.(?:solr|lucene).*") ? false : true : "Attempted
> close of request input stream - never do this, you will spoil connection reuse
> and possibly disrupt a client";
> + this.stream =
> ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM;
> + }
> + };
> +
> }
> };
> } else {
> @@ -519,31 +555,34 @@ public class SolrDispatchFilter extends
> BaseSolrFilter {
> }
>
> /**
> - * Wrap the response's output stream with a close shield, as if by a {@link
> CloseShieldOutputStream}. If this is a
> + * Wrap the response's output stream with a close shield. If this is a
> * retry, we will assume that the stream has already been wrapped and do
> nothing.
> *
> + * Only the container should ever actually close the servlet request stream.
> + *
> * @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 HttpServletResponse closeShield(HttpServletResponse response,
> boolean retry) {
> - if (testMode && !retry) {
> + public static HttpServletResponse closeShield(HttpServletResponse
> response, boolean retry) {
> + if (!retry) {
> return new HttpServletResponseWrapper(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;
> +
> + return new ServletOutputStreamWrapper(super.getOutputStream()) {
> + @Override
> + public void close() {
> + // even though we skip closes, we let local tests know not to close so
> that a full understanding can take
> + // place
> + assert
> Thread.currentThread().getStackTrace()[2].getClassName().matches(
> + "org\\.apache\\.(?:solr|lucene).*") ? false : true : "Attempted
> close of response output stream - never do this, you will spoil connection
> reuse and possibly disrupt a client";
> + stream =
> ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM;
> + }
> + };
> }
> +
> };
> } else {
> return response;
>
> http://git-wip-us.apache.org/repos/asf/lucene-
> solr/blob/29620105/solr/core/src/java/org/apache/solr/servlet/SolrRequest
> Parsers.java
> ----------------------------------------------------------------------
> 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 60b6d2f..4bcb8d8 100644
> --- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> +++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
> @@ -519,8 +519,7 @@ public class SolrRequestParsers
>
> @Override
> public InputStream getStream() throws IOException {
> - // Protect container owned streams from being closed by us, see SOLR-
> 8933
> - return new CloseShieldInputStream(req.getInputStream());
> + return req.getInputStream();
> }
> }
>
> @@ -767,8 +766,7 @@ public class SolrRequestParsers
> String userAgent = req.getHeader("User-Agent");
> boolean isCurl = userAgent != null && userAgent.startsWith("curl/");
>
> - // Protect container owned streams from being closed by us, see SOLR-
> 8933
> - FastInputStream input = FastInputStream.wrap( new
> CloseShieldInputStream(req.getInputStream()) );
> + FastInputStream input = FastInputStream.wrap(req.getInputStream());
>
> if (isCurl) {
> SolrParams params = autodetect(req, streams, input);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org