On 10/03/2016 21:16, jimi.hulleg...@svensktnaringsliv.se wrote: > On Thursday, March 10, 2016 11:20 AM, ma...@apache.org wrote: >> >>> 3. Why is the problem not limited to the first request for a jsp >>> page? >> >> Because EL imports may be dynamic so the EL has to be evaluated on >> execution. > > I'm not really sure I follow you now. Can you explain what you mean > with dynamic imports in this regard? I can't see any mentioning of it > in the specs > (http://download.oracle.com/otn-pub/jcp/el-3_0-fr-eval-spec/EL3.0.FR.pdf).
There is nothing stopping a JSP author obtaining a reference to the ImportHandler and conditionally adding classes to import. The configuration of the ImportHandler could change on every call to the page. > Can you give a concrete example, where an EL expression element > would need to be evaluated as a potential class, again and again for > each request? See above. > Because the way I see it, "foo" in ${foo.bar} in some jsp page only > needs to be evaluated once. If "foo" corresponded to a class, > matching one of the imports in the jsp, then this lookup will not > change unless the jsp code changes. And the same is true if the > lookup failes with a ClassNotFoundException. What am I missing? See above. >>> 4. Why isn't the ClassNotFoundException logged by the >>> ImportHandler? >> >> Because it is expected and logging it provides no value to the >> user. > > Well, I happen to disagree. :) In our case, if we could see all these > stacktraces in the log, by enabling DEBUG/TRACE log level on the > ImportHandler class for example, then we would quickly see just how > big this problem is (ie how often this happens), on what jsp pages it > happens, and what EL expressions cause it. We'll have to agree to disagree on that one. If you are concerned about a performance issue then you need to know where to look to enable debug logging. A profiler will tell you where to look and at that point you don't need the debug logging. > On my local machine, I was able to "catch" the ClassNotFoundException > using a debug breakpoint in my IDE, and using the breakpoint hit > count I could see that this exception is thrown and handled (ie > ignored) by the ImportHandler 2700 times for a single request to the > start page. > > >> The JavaEE specs are very big on backwards compatibility. >> Therefore, the chances of changing the spec syntax to fix this are >> zero. > > OK. Fair enough. I agree with you that backwards compatibility is > important. But there are ways to fix this while still keeping the > backwards compatibility. For example by making it possible to turn > off this feature (globally, per webapp, or per jsp), where the > default is "on". Wouldn't you agree that such a change would be 100% > backwards compatible? The code in question is in the JSP API JAR and there is no configuration API available. The only option would be a system property which would make it a global configuration option. > And at the same time it would more or less > solve this problem completely. Because people who experience the > mentioned problems could turn of this setting globally, and then only > enable it for those specific jsp pages where it is needed (and these > pages would then be cleaned up, so that no EL-references exist > without specific scope unless they are known to never be null. Tada, > problem solved! =) Um, no. See above. This would have to be a global option. It would work for some users/pages but not for others. > Actually... this wouldn't even need to be in the specifications... I > can't see any harm in a EL implementation introducing such settings > on its own, before the specifications can "catch up". That way you > could basically introduce this fix in trunk right now, and have it > tested and out in a stable release in no time =) We have added such spec breaking options in the past as you'll see if you look at the configuration docs for system properties. Generally, I'm not a fan of configuration via system property. It is usually too blunt a tool and doesn't provide the per webapp control that most users need. > On Thursday, March 10, 2016 1:24 PM, ma...@apache.org wrote: >> >> The issue is in ScopedAttributeELResolver. >> >> ScopedAttributeELResolver checks the >> page/request/session/application context first and only if it >> doesn't find the attribute there does it try loading the class. > > When debugging this in my IDE, I could see this in action. I also > noticed that the ImportHandler that performs the class lookup is > fetched from the ELContext object. So if I could wrap that object, I > could simply return null in the method getImportHandler(), thus > disabling this functionallity and therefore solving the performance > problem for us. But I couldn't find any way to wrap the ELContext, > for some reason. Can it be done, using standard code or config? No. > Or can this "import logic" be disabled some other way? You can do most things via reflection, including this. Personally, reflection would be my option of last resort. > There really should be a way for users to disable this, if the > functionallity is not used and it just is causing problems. And, like > I said above, that could be done without breaking the specs. And an > alternative way to the way above, could be like I mentioned before, > to add a configuration option that forces the class name to begin > with a capital letter. That way ${Boolean.TRUE}, ${Integer.MAX_VALUE} > and ${MyClass.myStaticField} would still work, while ${foo.bar} etc > would simply be ignored. As long as the configuration option would > default to false (ie lower case first letter is allowed, as per the > specification) it wouldn't break the specification unless the user > deliberately told it to (which is fine, right?). Yes, that is fine. Breaking the spec by default is fine too as long as there is a configuration option to make Tomcat spec compatible. There are a few things we don't do by default because most users don't use the feature and enabling it harms performance. > It would be really nice to get your input on these suggestions. And > if you don't like them, could you explain why? Generally, the suggestions are reasonable. The control can't be as fine-grained as you suggest but that isn't necessarily a show-stopper. The main disadvantage that these options have is that a better solution is available. Follow the bug report from my previous message in this thread for details. > If your opinion is "We > need to stick 100% to the specification, and never ever give even the > expert user any way to override this, ever", then I would say that > such a view causes more harm than good. :) I don't think you'll ever find any container implementer that takes that view. My first preference is to fix the root cause of the problem rather than adding configuration to work-around it. If the root cause can't be fixed then configuration is an option with a strong preference for per webapp rather than global. In this case, I believe the root cause has been fixed. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org