Additional constructor for ValidatorResources that takes URL[] instead of String[]

• Key: VALIDATOR-209
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Niall Pemberton
• Reporter: Craig McClanahan
• Component/s: Framework
• Affects Version/s: 1.3.0 Release
• Fix Version/s: 1.3.1 Release

Description

Currently, the constructor for ValidatorResources takes an InputStream, and array of InputStream, a single String, or an array of Strings (in the latter two cases, the strings are assumed to be URIs of either webapp resources or classpath resources to be parsed). In a web application environment, a framework or application using Commons Validator will typically use either ServletContext.getResource() or Class.getResource() to find URLs of the set of resources to be configured.

However, the CommonsValidator constructor cannot take URLs correctly. Therefore, the caller will need to convert these URLs to external (String) form in order to pass them in. However, these Strings will ultimately need to be turned back into URLs anyway (inside the Digester instance being used), in order for relative references to work.

Thus, the current implementation assumes that there is a lossless conversion from a URL returned by ServletContext.getResource() or Class.getResource(), to a String, and then back to a URL. That assumption is not necessarily guaranteed for the servlet context resources (although it is generally the case in practice for most containers). It is legal for the container to embed information (such as a custom URLStreamHandler implementation) inside the URLs it returns for webapp resources.

It would be better defensive coding for ValidatorResources to accept an array of URLs of the resources to be loaded (and pass them directly in to Digester unchanged), in addition to the other constructors that are currently supported.

Comments

[Niall Pemberton]

I just had a look at the Digester javadoc but I don't see a parse() method that takes a URL - just a String uri. I guess I must be missing something, can you point me in the right direction?

[Craig McClanahan]

> I just had a look at the Digester javadoc but I don't see a parse() method
> that takes a URL - just a String uri. I guess I must be missing something,
> can you point me in the right direction?

Grumble ... you're right, of course. I thought this had been taken care of earlier in Digester.

I'll file an issue against Digester and make this one dependent on it. I suspect it won't be able to get in to Digester 1.8, but there's always 1.9 ...

OK, probably the easiest solution (rather than co-ordinating component releases) is to create an org.xml.sax.InputSource as Digester does and call the parse() method with that.

Patch attached

[Craig McClanahan]

The patch looks OK to me, but with one comment ... it makes me uncomfortable to have code that opens a stream without explicitly closing it. Even though it apparently works in the case of a resource embedded in a JAR file (such as the registered DTDs inside Digester), I'd feel better if we did the close ourselves ... so the constructor that takes a URL might be coded like this:

Digester digester = initDigester();
digester.push(this);
InputStream stream = null;
try { stream = url.openStream(); InputSource source = new InputSource(url.toExternalForm()); source.setByteStream(stream); digester.parse(source); } finally {
if (stream != null) {
try { stream.close() } catch (IOException e) { }
}

instead. I'm going to make the same recommendation to Simon on the uses of this pattern inside Digester, too (although that could get a little painful in some cases). Can you tell I've been getting bit by locked Commons JAR files on Windows a lot lately?

[Niall Pemberton]

Fixed thanks

http://svn.apache.org/viewvc?view=rev&revision=478473