diff --git a/lucene/replicator/src/java/org/apache/lucene/replicator/http/ReplicationService.java b/lucene/replicator/src/java/org/apache/lucene/replicator/http/ReplicationService.java index e392445432e..39cd9942853 100644 --- a/lucene/replicator/src/java/org/apache/lucene/replicator/http/ReplicationService.java +++ b/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) { diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cdf75da7015..638acc0d846 100644 --- a/solr/CHANGES.txt +++ b/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) 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 63cfb7cc9a2..de138839d09 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/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) { 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 992dfe2f4ac..c496ce1baae 100644 --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java +++ b/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); diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java b/solr/core/src/java/org/apache/solr/servlet/ServletInputStreamWrapper.java new file mode 100644 index 00000000000..d229bf763c9 --- /dev/null +++ b/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(); + } + +} diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java b/solr/core/src/java/org/apache/solr/servlet/ServletOutputStreamWrapper.java new file mode 100644 index 00000000000..d12c3bded65 --- /dev/null +++ b/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(); + } +} 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 7a0e4ef321e..2d089350ddc 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/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 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; + } + } } 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 79c151b67ee..a91f6a26394 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java +++ b/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); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/JettyWebappTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/JettyWebappTest.java index fa22f80dcd2..ca37114db1c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/JettyWebappTest.java +++ b/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(); }