Apache/lucene
1
0
Fork 0
mirror of https://github.com/apache/lucene.git synced 2025-02-24 11:16:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

SOLR-12290: Do not close any servlet streams and improve our servlet stream closing prevention code for users and devs.

Browse Source
This commit is contained in:
Mark Miller 2018-05-04 18:02:06 -05:00
parent ad0ad2ec89
commit 296201055f
11 changed files with 152 additions and 133 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
solr/CHANGES.txt
View File

@ -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
----------------------

6
solr/core/src/java/org/apache/solr/handler/BlobHandler.java
View File

@ -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);

6
solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
View File

@ -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);

85
solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
View File

@ -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);
}
BufferedReader r = (BufferedReader) reader;
for (int i = 0; i < skipLines; i++) {
r.readLine();
}
}
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();
}
// 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;
if (vals.length != fieldnames.length) {
input_err("expected " + fieldnames.length + " values but got " + vals.length, vals, line);
}
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();
}
// 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;
if (vals.length != fieldnames.length) {
input_err("expected "+fieldnames.length+" values but got "+vals.length, vals, line);
}
addDoc(line,vals);
}
} finally{
if (reader != null) {
IOUtils.closeQuietly(reader);
}
addDoc(line, vals);
}
}

16
solr/core/src/java/org/apache/solr/handler/loader/JavabinLoader.java
View File

@ -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;

6
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
View File

@ -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

14
solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
View File

@ -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);

2
solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java
View File

@ -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;

2
solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java
View File

@ -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;

139
solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
View File

@ -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;

6
solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
View File

@ -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);