You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2018/01/09 03:40:31 UTC
lucene-solr:branch_7x: SOLR-11692: Constrain cases where
SolrDispatchFilter uses closeShield
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x eb2ffb667 -> 9e3c16cf2
SOLR-11692: Constrain cases where SolrDispatchFilter uses closeShield
(cherry picked from commit 7a375fd)
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/9e3c16cf
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9e3c16cf
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9e3c16cf
Branch: refs/heads/branch_7x
Commit: 9e3c16cf2ef0d2b9d562d8df75b4688d6ce0131a
Parents: eb2ffb6
Author: David Smiley <ds...@apache.org>
Authored: Mon Jan 8 22:39:17 2018 -0500
Committer: David Smiley <ds...@apache.org>
Committed: Mon Jan 8 22:40:01 2018 -0500
----------------------------------------------------------------------
solr/CHANGES.txt | 3 ++
.../solr/security/AuthenticationPlugin.java | 1 +
.../apache/solr/servlet/SolrDispatchFilter.java | 48 ++++++++++----------
.../apache/solr/cloud/TestConfigSetsAPI.java | 3 +-
4 files changed, 29 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9e3c16cf/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 753d6dc..3ddb45d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -109,6 +109,9 @@ Other Changes
* SOLR-11801: Support customisation of the "highlighting" query response element.
(Ramsey Haddad, Pranav Murugappan, David Smiley, Christine Poerschke)
+* SOLR-11692: SolrDispatchFilter's use of a "close shield" in tests should not be applied to
+ further servlet chain processing. (Jeff Miller, David Smiley)
+
================== 7.2.1 ==================
Bug Fixes
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9e3c16cf/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
index d8f2ef2..a9d112a 100644
--- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
@@ -48,6 +48,7 @@ public abstract class AuthenticationPlugin implements Closeable {
* the response and status code have already been sent.
* @throws Exception any exception thrown during the authentication, e.g. PrivilegedActionException
*/
+ //TODO redeclare params as HttpServletRequest & HttpServletResponse
public abstract boolean doAuthenticate(ServletRequest request, ServletResponse response,
FilterChain filterChain) throws Exception;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9e3c16cf/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 8f8bda8..714d127 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -326,8 +326,10 @@ public class SolrDispatchFilter extends BaseSolrFilter {
doFilter(request, response, chain, false);
}
- public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain, boolean retry) throws IOException, ServletException {
- if (!(request instanceof HttpServletRequest)) return;
+ 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;
try {
@@ -343,28 +345,24 @@ public class SolrDispatchFilter extends BaseSolrFilter {
}
}
- AtomicReference<ServletRequest> wrappedRequest = new AtomicReference<>();
- if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been
- // sent
+ AtomicReference<HttpServletRequest> wrappedRequest = new AtomicReference<>();
+ if (!authenticateRequest(request, response, wrappedRequest)) { // the response and status code have already been sent
return;
}
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());
+ log.debug("User principal: {}", request.getUserPrincipal());
}
// No need to even create the HttpSolrCall object if this path is excluded.
if (excludePatterns != null) {
- String requestPath = ((HttpServletRequest) request).getServletPath();
- String extraPath = ((HttpServletRequest) request).getPathInfo();
- if (extraPath != null) { // In embedded mode, servlet path is empty - include all post-context path here for
- // testing
+ String requestPath = request.getServletPath();
+ String extraPath = request.getPathInfo();
+ if (extraPath != null) {
+ // In embedded mode, servlet path is empty - include all post-context path here for testing
requestPath += extraPath;
}
for (Pattern p : excludePatterns) {
@@ -376,7 +374,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
}
}
- HttpSolrCall call = getHttpSolrCall((HttpServletRequest) request, (HttpServletResponse) response, retry);
+ HttpSolrCall call = getHttpSolrCall(closeShield(request, retry), closeShield(response, retry), retry);
ExecutorUtil.setServerThreadFlag(Boolean.TRUE);
try {
Action result = call.call();
@@ -385,7 +383,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
chain.doFilter(request, response);
break;
case RETRY:
- doFilter(request, response, chain, true);
+ doFilter(request, response, chain, true); // RECURSION
break;
case FORWARD:
request.getRequestDispatcher(call.getPath()).forward(request, response);
@@ -396,7 +394,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
ExecutorUtil.setServerThreadFlag(null);
}
} finally {
- consumeInputFully((HttpServletRequest) request);
+ consumeInputFully(request);
}
}
@@ -430,7 +428,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
}
}
- private boolean authenticateRequest(ServletRequest request, ServletResponse response, final AtomicReference<ServletRequest> wrappedRequest) throws IOException {
+ private boolean authenticateRequest(HttpServletRequest request, HttpServletResponse response, final AtomicReference<HttpServletRequest> wrappedRequest) throws IOException {
boolean requestContinues = false;
final AtomicBoolean isAuthenticated = new AtomicBoolean(false);
AuthenticationPlugin authenticationPlugin = cores.getAuthenticationPlugin();
@@ -440,9 +438,9 @@ public class SolrDispatchFilter extends BaseSolrFilter {
// /admin/info/key must be always open. see SOLR-9188
// tests work only w/ getPathInfo
//otherwise it's just enough to have getServletPath()
- if (PKIAuthenticationPlugin.PATH.equals(((HttpServletRequest) request).getServletPath()) ||
- PKIAuthenticationPlugin.PATH.equals(((HttpServletRequest) request).getPathInfo())) return true;
- String header = ((HttpServletRequest) request).getHeader(PKIAuthenticationPlugin.HEADER);
+ if (PKIAuthenticationPlugin.PATH.equals(request.getServletPath()) ||
+ PKIAuthenticationPlugin.PATH.equals(request.getPathInfo())) return true;
+ String header = request.getHeader(PKIAuthenticationPlugin.HEADER);
if (header != null && cores.getPkiAuthenticationPlugin() != null)
authenticationPlugin = cores.getPkiAuthenticationPlugin();
try {
@@ -450,7 +448,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
// upon successful authentication, this should call the chain's next filter.
requestContinues = authenticationPlugin.doAuthenticate(request, response, (req, rsp) -> {
isAuthenticated.set(true);
- wrappedRequest.set(req);
+ wrappedRequest.set((HttpServletRequest) req);
});
} catch (Exception e) {
log.info("Error authenticating", e);
@@ -478,9 +476,9 @@ public class SolrDispatchFilter extends BaseSolrFilter {
* @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) {
+ private HttpServletRequest closeShield(HttpServletRequest request, boolean retry) {
if (testMode && !retry) {
- return new HttpServletRequestWrapper((HttpServletRequest) request) {
+ return new HttpServletRequestWrapper(request) {
ServletInputStream stream;
@Override
@@ -510,9 +508,9 @@ public class SolrDispatchFilter extends BaseSolrFilter {
* @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) {
+ private HttpServletResponse closeShield(HttpServletResponse response, boolean retry) {
if (testMode && !retry) {
- return new HttpServletResponseWrapper((HttpServletResponse) response) {
+ return new HttpServletResponseWrapper(response) {
ServletOutputStream stream;
@Override
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9e3c16cf/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
index 374e27a..ddd8fd9 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -540,7 +540,8 @@ public class TestConfigSetsAPI extends SolrTestCaseJ4 {
m = (Map) ObjectBuilder.getVal(new JSONParser(
new StringReader(response)));
} catch (JSONParser.ParseException e) {
- fail(e.getMessage());
+ System.err.println("err response: " + response);
+ throw new AssertionError(e);
}
} finally {
httpPost.releaseConnection();