[
http://issues.apache.org/jira/browse/VALIDATOR-209?page=comments#action_12452113
]
Craig McClanahan commented on VALIDATOR-209:
--------------------------------------------
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? :-)
> Additional constructor for ValidatorResources that takes URL[] instead of
> String[]
> ----------------------------------------------------------------------------------
>
> Key: VALIDATOR-209
> URL: http://issues.apache.org/jira/browse/VALIDATOR-209
> Project: Commons Validator
> Issue Type: Improvement
> Components: Framework
> Affects Versions: 1.3.0 Release
> Reporter: Craig McClanahan
> Priority: Minor
> Attachments: validator-209-ValidatorResources.patch
>
>
> 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.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
