Bug 109756

Summary:	SimpleURIResolver throws assertion error when xsl:import is incorrect, loosing the filename information
Product:	[Retired] Red Hat Web Application Framework	Reporter:	Daniel Berrangé <berrange>
Component:	other	Assignee:	Vadim Nasardinov <vnasardinov>
Status:	CLOSED RAWHIDE	QA Contact:	Jon Orris <jorris>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	nightly
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2003-11-25 22:40:27 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	109665

Description Daniel Berrangé 2003-11-11 16:55:34 UTC

Description of problem:
The SimpleURIResolver class performs an assertion for existance of the
stylesheet returned by Templating.transformURL. Unfortunately, it is
perfectly legitimate for this method to return null - ie if the file
being transformed to doesn't actually exist. So, if the designer gets
an xsl:import statment wrong all they see is an assertion failure,
rather than an error message with the line number, filename of the
incorrect line of XSL. To correct this we need to instead throw a
TransformerException, enabling the XSL transfomer to fill in line
number info.

While we're at it, we should probably convert all the
UncheckedWrapperExceptions in SimpleURIResolver to
TransformerExceptions too, to provide better error handling


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. List a non-existant file in an xsl:import
2. Request the page
3.
  
Actual results:
Assertion thrown

Expected results:
Error report shown, with line & filename in logs.

Additional info:

Here is the diff of the change from my checkout:

====
//core-platform/dev/src/com/arsdigita/templating/SimpleURIResolver.java#5
-
/var/ccm-devel/dev/dan/aplaws-rickshaw/core/src/com/arsdigita/templating/SimpleURIResolver.java
====
116,117c116,123
< 
<             Assert.exists(xfrmedURL, URL.class);
---
>             
>             // Assertions are bad because java.lang.Errors
>             // aren't caught by the XSL engine so it never
>             // fills in file & line information.
>             //Assert.exists(xfrmedURL, URL.class);
>             if (xfrmedURL == null) {
>                 throw new TransformerException("URL does not exist "
+ thisURL);
>             }

Comment 1 Vadim Nasardinov 2003-11-21 21:37:30 UTC

Fixed on the trunk in change 38228.

Some useful data points.

Prior to this change, SimpleURIResolver would fail with an assertion
error, given the circumstances described above.  Since AssertionError
is an Error, it would bubble up all the way to the top uncaught, which
is good.

We now throw a TransformerException instead.  Some trasnformers
propagate it, some don't.  Specifically, Xalan seems to swallow this
exception and simply ignore the unresolved xsl:import element.  On the
other hand, Saxon (6.5.2) propagates the exception by rethrowing a
TransformerConfigurationException.  In either case, our
XSLTempplate.Log4JErrorListener logs the exception.

However, if you aren't watching the server log, you are screwed in
either case.  With Xalan, you may not notice the error, because the
exception is swallowed.

With Saxon 6.5.2, the rethrown exception does not contain the original
cause.  It catches TransformerException that we throw in the URI
resolver and rethrows TransformerConfigurationException like so:

$ cd com/icl/saxon/
$ nl -ba PreparedStyleSheet.java | head -n 141 | tail -n 7
   135          if (errorCount > 0) {
   136              throw new TransformerConfigurationException(
   137                              "Failed to compile stylesheet. " +
   138                              errorCount +
   139                              (errorCount==1 ? " error " : " errors ") +
   140                              "detected.");
   141          }


(See http://prdownloads.sourceforge.net/saxon/saxon6_5_2.zip?download)

The stack trace displayed to the user looks like so:

javax.xml.transform.TransformerConfigurationException: Failed to compile stylesheet. 1 error detected.
        at com.icl.saxon.PreparedStyleSheet.prepare(PreparedStyleSheet.java:136)
        at com.icl.saxon.TransformerFactoryImpl.newTemplates(TransformerFactoryImpl.java:127)
        at com.arsdigita.templating.XSLTemplate.(XSLTemplate.java:78)
        at com.arsdigita.templating.Templating.getTemplate(Templating.java:137)
        at com.arsdigita.templating.Templating.getTemplate(Templating.java:167)
        at com.arsdigita.bebop.page.PageTransformer.servePage(PageTransformer.java:190)
        at com.arsdigita.bebop.page.PageTransformer.servePage(PageTransformer.java:152)
        at com.arsdigita.bebop.page.PageDispatcher.dispatch(PageDispatcher.java:80)
        at com.arsdigita.dispatcher.MapDispatcher.dispatch(MapDispatcher.java:156)


So, the bottom line is, exception handling in both Saxon and Xalan
seems rather flaky.

I wonder if there are any hidden knobs in Xalan that can be adjusted
to make it stricter.  It seems to support a lot of command-line
options:

http://xml.apache.org/xalan-j/commandline.html

Comment 2 Daniel Berrangé 2003-11-24 09:45:05 UTC

Saxon's behaviour does actually kind of make sense. It tries to report
as many errors at once as possible, via the ErrorListener. Since you
can only have a single root cause for a
TransformerConfigurationException, its not really possible to pass
around all the original exceptions. In any case, if the calling code
did need to get all the original exceptions with their file name &
line numbers, it can simply register an ErrorListener impl that
collects them all into a java.util.List. This list could be examined
whenever a TransformerConfigurationException were encountered. So I
think it doesn't really matter what a particular transformer engine
includes into its exception, provided it uses the ErrorListener
interface whenever a problem occurs.

As a side note, does anyone really care about Xalan any more ? We've
use either Saxon / JD.XSLT exclusively for all our development &
production systems, occassionally toying with XSLTC/Resin.