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?


> 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

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.


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

Reply via email to