piranha: jsp-file broken

The <jsp-file> element inside a servlet element in web.xml is broken. It used to work.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 34 (22 by maintainers)

Most upvoted comments

Change looks good I could think of a few enhancements still, but after 30 comments and a dozen commits this seems good to go as it is.

I executed the TCK and the scores improved:

     [exec] [javatest.batch] ********************************************************************************
     [exec] [javatest.batch] Number of tests completed:  1685 (1611 passed, 68 failed, 6 with errors)
     [exec] [javatest.batch] Number of tests remaining:  6
     [exec] [javatest.batch] ********************************************************************************
     [exec] [javatest.batch] Completed running 1685 tests.
     [exec] [javatest.batch] Number of Tests Passed      = 1611
     [exec] [javatest.batch] Number of Tests Failed      = 68
     [exec] [javatest.batch] Number of Tests with Errors = 6
     [exec] [javatest.batch] ********************************************************************************

arjantijms on Oct 13, 2021

To sum up, replacing a JSP implementation via just a *.jsp mapping, and then expect that it works for jsp-file in web.xml, is unlikely to ever work or be mandated by the spec.

But, overriding *.jsp so that foo.jsp invokes your own servlet, has to work. With the current PR I think that part is now broken.

arjantijms on Oct 11, 2021

@folker-kuhn See https://twitter.com/mnriem/status/1450219299122008065 for the tweet. Between your changes and those from @arjantijms it is now up to 97%. Really cool!

mnriem on Oct 18, 2021

@folker-kuhn @Thihup The JspManager API is supposed to be the glue that can be delivered to integrate JSP into a web application and TLD scanning is indeed part of such an integration. Now if the JspManager API has to be split in more than one piece another Manager API should be introduced to cover that. The DefaultWebApplication should not be directly tied to JSP.

mnriem on Oct 12, 2021

You should still be able to map *.jsp to your own Servlet right, as is indeed allowed?

I just checked, the TCK also contains jsp-file entries in some xml files, which are indeed broken now too. So the TCK is broken one way or the other at the moment.

arjantijms on Oct 11, 2021

@arjantijms The TCK currently has this test that change the *.jsp mapping: https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/servlet/spec/annotationservlet/webservletdd/servlet_annotationservlet_webservletdd_web.xml#L71

@mnriem I’ll take a look if it can be split

Thihup on Oct 11, 2021