[ 
https://issues.apache.org/jira/browse/SOLR-5951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17884083#comment-17884083
 ] 

David Smiley commented on SOLR-5951:
------------------------------------

[~uschindler] , I think the problem of not having the right logging JARs should 
be a non-issue since Solr 5, where Solr insists on Jetty pre-packaged with our 
opinionated choice of logging JARs.  If someone messes with them; they are on 
their own to figure out the right ones.  I'm skeptical a person would really 
need the code change here to figure that out.  I'd like to remove 
CheckLoggingConfiguration and BaseSolrFilter and BaseSolrServlet, which are in 
servitude of it.  WDYT?  My motivation is merely a small simplification.

> SolrDispatchFilter no longer displays useful error message on statup when 
> logging jars are missing
> --------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-5951
>                 URL: https://issues.apache.org/jira/browse/SOLR-5951
>             Project: Solr
>          Issue Type: Bug
>    Affects Versions: 4.7, 4.7.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>            Priority: Major
>             Fix For: 4.7.2, 4.8, 6.0
>
>         Attachments: SOLR-5951.patch
>
>
> We no longer have logging jars in the webapp since SOLR-3706. Because of this 
> we added a extra check in SolrDispatchFilter's ctor to print a nice exception 
> if the logging jars were failing. This check was unfortunately never tests 
> and recently broke:
> The check delays initialization of the Logger instance to inside a try-catch 
> block inside the explicit ctor. If it fails with ClassNotFound, it throws 
> Exception.
> Recently we upgraded to a newer HttpClient version. Unfortunately 
> SolrDispatchFliter also has an implicit constructor a few lines before the 
> main constructor:
> {code:java}
>   protected final HttpClient httpClient = HttpClientUtil.createClient(new 
> ModifiableSolrParams()); // <-- this breaks the detection
>   
>   private static final Charset UTF8 = StandardCharsets.UTF_8;
>   public SolrDispatchFilter() {
>     try {
>       log = LoggerFactory.getLogger(SolrDispatchFilter.class);
>     } catch (NoClassDefFoundError e) {
>       throw new SolrException(
>           ErrorCode.SERVER_ERROR,
>           "Could not find necessary SLF4j logging jars. If using Jetty, the 
> SLF4j logging jars need to go in "
>           +"the jetty lib/ext directory. For other containers, the 
> corresponding directory should be used. "
>           +"For more information, see: 
> http://wiki.apache.org/solr/SolrLogging";,
>           e);
>     }
>   }
> {code}
> The first line above {{HttpClientUtil.createClient(new 
> ModifiableSolrParams());}} breaks the whole thing, because it is executed 
> before the declared constructor. The user just sees a ClassNotFoundEx at this 
> line of code, the nice error message is hidden.
> Because this is so easy to break, we should make the whole thing more safe 
> (any maybe test it). 2 options:
> # Into the webapp add a fake Servlet (not bound to anything, just loaded 
> first) that does not use any Solr classes at all, nothing only plain java
> # Alternatively add a Superclass between ServletFilter and SolrDispatchFilter 
> (pkg-private). When the servlet container loads SolrDispatchFilter, it has in 
> any case to first load the superclass. And this superclass does the check and 
> throws ServletException or whatever (no Solr Exception) with the message from 
> the current code.
> I tend to the second approach, because it does not need to modify web-inf. It 
> will also work with other Solr servlets, they must just extend this hidden 
> class. I will provide a patch for that.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to