Commons Validator
  1. Commons Validator
  2. VALIDATOR-209

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.3.0 Release
    • Fix Version/s: 1.3.1 Release
    • Component/s: Framework
    • Labels:
      None

      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.

        Issue Links

          Activity

          Henri Yandell made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Niall Pemberton made changes -
          Fix Version/s 1.3.1 [ 12311934 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Assignee Niall Pemberton [ niallp ]
          Resolution Fixed [ 1 ]
          Show
          Niall Pemberton added a comment - Fixed thanks http://svn.apache.org/viewvc?view=rev&revision=478473
          Hide
          Craig McClanahan added a comment -

          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?

          Show
          Craig McClanahan added a comment - 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 made changes -
          Attachment validator-209-ValidatorResources.patch [ 12345522 ]
          Hide
          Niall Pemberton added a comment -

          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

          Show
          Niall Pemberton added a comment - 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 made changes -
          Field Original Value New Value
          Link This issue is blocked by DIGESTER-110 [ DIGESTER-110 ]
          Hide
          Craig McClanahan added a comment -

          > 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 ...

          Show
          Craig McClanahan added a comment - > 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 ...
          Hide
          Niall Pemberton added a comment -

          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?

          Show
          Niall Pemberton added a comment - 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 created issue -

            People

            • Assignee:
              Niall Pemberton
              Reporter:
              Craig McClanahan
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development