Bug 59654 - Jsp spec violation in tld identifying?
Summary: Jsp spec violation in tld identifying?
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Jasper (show other bugs)
Version: trunk
Hardware: All All
: P2 major (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-02 02:26 UTC by Huxing Zhang
Modified: 2016-09-02 03:12 UTC (History)
1 user (show)



Attachments
patch against tomcat 7 trunk (1.75 KB, patch)
2016-06-02 02:33 UTC, Huxing Zhang
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Huxing Zhang 2016-06-02 02:26:01 UTC
What the spec says:
JSP.7.3.1 Identifying Tag Library Descriptors
...
TLD files should not be placed in /WEB-INF/classes or /WEB-INF/lib, and must not be placed inside /WEB-INF/tags or a subdirectory of it, unless named implicit.tld and intended to configure an implicit tag library with its JSP version and tlib-version.


Out test case:
We have simple web app with the following structure:

test
├── WEB-INF
│   ├── classes
│   └── tags
│       └── test.tld
└── taglib.jsp

test.tld:

<?xml version="1.0" encoding="UTf-8" ?>

<taglib version="2.0">
  <description></description>
  <display-name></display-name>
  <tlib-version>1.0</tlib-version>
  <short-name>f</short-name>
  <function>
    <name>get</name>
    <function-class>java.lang.System</function-class>
    <function-signature>java.lang.String getProperty(java.lang.String)</function-signature>
  </function>
</taglib>

taglib.jsp:

<%@page contentType="text/html;charset=UTF-8" pageEncoding="UTF-8"%>
<%@ taglib uri="/WEB-INF/tags/test.tld" prefix="f"%>
<html>
<body>
	<h3>${f:get("java.home")}</h3>
</body>
</html>

Tomcat 7.0.69 behavior:
After deploying the test web app and visit http://localhost:8080/test/taglib.jsp, the jsp page gets compiled and java home has been correctly displayed.

Tomcat 8 & 9 trunk behavior:
When visiting http://localhost:8080/test/taglib.jsp, server responded with HTTP 500:

HTTP Status 500 - Unable to find taglib "f" for URI: /WEB-INF/tags/test.tld

org.apache.jasper.JasperException: Unable to find taglib "f" for URI: /WEB-INF/tags/test.tld
	org.apache.jasper.compiler.DefaultErrorHandler.jspError(DefaultErrorHandler.java:55)
	org.apache.jasper.compiler.ErrorDispatcher.dispatch(ErrorDispatcher.java:277)
	org.apache.jasper.compiler.ErrorDispatcher.jspError(ErrorDispatcher.java:75)
	org.apache.jasper.compiler.TagLibraryInfoImpl.<init>(TagLibraryInfoImpl.java:183)
	org.apache.jasper.compiler.Parser.parseTaglibDirective(Parser.java:421)
	org.apache.jasper.compiler.Parser.parseDirective(Parser.java:479)
	org.apache.jasper.compiler.Parser.parseElements(Parser.java:1435)
	org.apache.jasper.compiler.Parser.parse(Parser.java:139)
	org.apache.jasper.compiler.ParserController.doParse(ParserController.java:227)
	org.apache.jasper.compiler.ParserController.parse(ParserController.java:100)
	org.apache.jasper.compiler.Compiler.generateJava(Compiler.java:199)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:356)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:336)
	org.apache.jasper.compiler.Compiler.compile(Compiler.java:323)
	org.apache.jasper.JspCompilationContext.compile(JspCompilationContext.java:585)
	org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:363)
	org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:396)
	org.apache.jasper.servlet.JspServlet.service(JspServlet.java:340)
	javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
	org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)


Conclusion:
Clearly the tomcat 7 behavior is against spec, which I think should be fixed.


Analysis:
After some digging, I found that the tld file is skipped during tld scan phase. However, when parsing tag lib directive, the org.apache.jasper.compiler.TagLibraryInfoImpl#generateTLDLocation didn't check the tld file location that is prohibited by spec.
Since the implementation is quite different between tomcat 7 % tomcat 8+. I found it hard to backport the tomcat 8 implementation to tomcat 7. Therefore, the fix should be specific to tomcat 7(tomcat 6 is not investigated.)


Proposed Fix:
Please refer to the attachment.
I've added a dedicated message to indicate such an behaivor is violating spec.

p.s. 
Should we also add the dedicated message to tomcat 8+ ? Because the message "Unable to find taglib "f" for URI: /WEB-INF/tags/test.tld"  is somewhat confusing. The file does exist, it is the spec that requires tomcat not to load it.
Comment 1 Huxing Zhang 2016-06-02 02:33:19 UTC
Created attachment 33908 [details]
patch against tomcat 7 trunk
Comment 2 Mark Thomas 2016-06-06 13:28:19 UTC
The improved message part of the fix has been applied to:
- 9.0.x for 9.0.0.M7 onwards
- 8.5.x for 8.5.3 onwards
- 8.0.x for 8.0.36 onwards
Comment 3 Mark Thomas 2016-06-06 13:42:51 UTC
Patch applied to 7.0.x for 7.0.70 onwards and 6.0p.x for 6.0.46 onwards.
Comment 4 Evan Greensmith 2016-08-31 03:31:07 UTC
I think this should just be a warning in older releases of Tomcat. Our application does place taglibs here. The application was built against servlet spec 2.5 (as declared in the web.xml) and the accompanying JSP spec 2.1, where there was no such requirement.

We can update this for future versions of the product, but clients with older versions won't be able to pick up any Tomcat security fixes past 7.0.69.
Comment 5 Evan Greensmith 2016-08-31 03:34:11 UTC
(In reply to Mark Thomas from comment #3)
> Patch applied to 7.0.x for 7.0.70 onwards and 6.0p.x for 6.0.46 onwards.

I think applying this to 6.0.x is a bug. The "which version?" page http://tomcat.apache.org/whichversion.html states 6.0.x uses jsp spec 2.1, where this is not a requirement.
Comment 6 Mark Thomas 2016-08-31 07:40:41 UTC
The requirement does apply to Tomcat 7.0.x and 6.0.x. It is present in JSP 2.1 and JSP 2.2. It is in the same section (JSP.7.3.1) as quoted in the original report.
Comment 7 Evan Greensmith 2016-09-02 03:12:01 UTC
(In reply to Mark Thomas from comment #6)
> The requirement does apply to Tomcat 7.0.x and 6.0.x. It is present in JSP
> 2.1 and JSP 2.2. It is in the same section (JSP.7.3.1) as quoted in the
> original report.

You're right. I was mistaken.

It is a frustrating change to back-port so far. Admittedly we're complicit in being non-compliant to the spec; though the same can be said for tomcat, websphere and weblogic, which all did allow this. It is not a change that will fix any application. C'est la vie, I guess.