groovy
  1. groovy
  2. GROOVY-2914

LoadConfiguration fails to load *.jar

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.5.6
    • Fix Version/s: 1.8-rc-1, 1.7.9, 1.9-beta-1
    • Component/s: None
    • Labels:
      None
    • Environment:
      Windows XP, Groovy 1.5.6
    • Number of attachments :
      0

      Description

      In my project I have a starter config like this:

       
      # load project resources
          load *.jar
      

      which causes groovy to fail with the following exception

      exception while configuring main class loader:
      java.lang.StringIndexOutOfBoundsException: String index out of range: -1
              at java.lang.String.substring(String.java:1768)
              at org.codehaus.groovy.tools.LoaderConfiguration.loadFilteredPath(LoaderConfiguration.java:193)
              at org.codehaus.groovy.tools.LoaderConfiguration.configure(LoaderConfiguration.java:113)
              at org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:81)
              at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:130)
      

      This can be fixed in LoaderConfiguration.java as follows:

      /**
        * load a possible filtered path. Filters are defined
        * by using the * wildcard like in any shell
        */
      private void loadFilteredPath(String filter) {
        if (filter==null) return;
          int starIndex = filter.indexOf(WILDCARD);
          if (starIndex == -1) {
            addFile(new File(filter));
            return;
          }
          boolean recursive = filter.indexOf(ALL_WILDCARD) != -1;
              
          String startDir = starIndex == 0 ? "./" : filter.substring(0, starIndex - 1);
          File root = new File(startDir);
      
          filter = RegexUtils.quote(filter);
          filter = filter.replaceAll("\\"+WILDCARD+"\\"+WILDCARD, MATCH_ALL);
          filter = filter.replaceAll("\\" + WILDCARD, MATCH_FILE_NAME);
          Pattern pattern = Pattern.compile(filter);
      
          final File[] files = root.listFiles();
          if (files != null) {
            findMatchingFiles(files, pattern, recursive);
          }
      }
      

      In the above method implementation the line

      String startDir = filter.substring(0, starIndex - 1);

      which caused the mentioned exception, was changed to

      String startDir = starIndex == 0 ? "./" : filter.substring(0, starIndex - 1);
      

      as you see.

        Activity

        Hide
        Klaus Kopruch added a comment -

        I'm sorry, I was a little bit too fast with my solution! I just tested my proposed solution and I diagnosed that it only solved the exception, but files are still not loaded from the current directory. So, I build Groovy 1.5.6 using the source distribution (it works extremely simple), inserted the following test case into LoaderConfigurationTest.groovy

          void testLoadFilesWithWildcardAtPositionZero() {
            def txt = 'load *.xml'
            
            def config = new LoaderConfiguration()
            config.requireMain = false
            config.configure(new StringBufferInputStream(txt))
            
            assert config.classPathUrls.toString().contains('build.xml')  
            assert config.classPathUrls.toString().contains('pom.xml')  
          }
        

        and had to change method LoaderConfiguration.loadFilteredPath in LoaderConfiguration.java as follows:

            /**
             * load a possible filtered path. Filters are defined
             * by using the * wildcard like in any shell
             */
            private void loadFilteredPath(String filter) {
                if (filter==null) return;
                int starIndex = filter.indexOf(WILDCARD);
                if (starIndex == -1) {
                    addFile(new File(filter));
                    return;
                } else if (starIndex == 0) {
                  starIndex = 2;        
                  filter = "./" + filter;
                }
                boolean recursive = filter.indexOf(ALL_WILDCARD) != -1;
                
                String startDir = filter.substring(0, starIndex - 1);
                File root = new File(startDir);
        
                filter = RegexUtils.quote(filter);
                filter = filter.replaceAll("\\"+WILDCARD+"\\"+WILDCARD, MATCH_ALL);
                filter = filter.replaceAll("\\" + WILDCARD, MATCH_FILE_NAME);
                Pattern pattern = Pattern.compile(filter);
        
                final File[] files = root.listFiles();
                if (files != null) {
                    findMatchingFiles(files, pattern, recursive);
                }
            }
        

        After that, it was possible to load jar files from the current directory using

            load */jar
        

        in the starter configuration file.

        Show
        Klaus Kopruch added a comment - I'm sorry, I was a little bit too fast with my solution! I just tested my proposed solution and I diagnosed that it only solved the exception, but files are still not loaded from the current directory. So, I build Groovy 1.5.6 using the source distribution (it works extremely simple), inserted the following test case into LoaderConfigurationTest.groovy void testLoadFilesWithWildcardAtPositionZero() { def txt = 'load *.xml' def config = new LoaderConfiguration() config.requireMain = false config.configure( new StringBufferInputStream(txt)) assert config.classPathUrls.toString().contains('build.xml') assert config.classPathUrls.toString().contains('pom.xml') } and had to change method LoaderConfiguration.loadFilteredPath in LoaderConfiguration.java as follows: /** * load a possible filtered path. Filters are defined * by using the * wildcard like in any shell */ private void loadFilteredPath( String filter) { if (filter== null ) return ; int starIndex = filter.indexOf(WILDCARD); if (starIndex == -1) { addFile( new File(filter)); return ; } else if (starIndex == 0) { starIndex = 2; filter = "./" + filter; } boolean recursive = filter.indexOf(ALL_WILDCARD) != -1; String startDir = filter.substring(0, starIndex - 1); File root = new File(startDir); filter = RegexUtils.quote(filter); filter = filter.replaceAll( "\\" +WILDCARD+ "\\" +WILDCARD, MATCH_ALL); filter = filter.replaceAll( "\\" + WILDCARD, MATCH_FILE_NAME); Pattern pattern = Pattern.compile(filter); final File[] files = root.listFiles(); if (files != null ) { findMatchingFiles(files, pattern, recursive); } } After that, it was possible to load jar files from the current directory using load */jar in the starter configuration file.
        Hide
        Paul King added a comment -

        I tried the patch and it seems to work fine. Any objections to me adding this? Or is there some benefit to having a scheme that encourages (forces?) the use of absolute paths?

        Show
        Paul King added a comment - I tried the patch and it seems to work fine. Any objections to me adding this? Or is there some benefit to having a scheme that encourages (forces?) the use of absolute paths?
        Hide
        Klaus Kopruch added a comment -

        If I use a project starter conf which comes together with the script(s) and the additional JARs in one directory, it's helpful to have relative paths as

        load *.jar

        .

        On the other hand, for the global starter conf in $

        {groovy.home}

        /conf relative paths do not make so much sense.

        Putting it all together, I suggest to allow relative paths in starter conf. Using it in a global starter conf or not, should be left to the decision of the person who installs Groovy on that system.

        Show
        Klaus Kopruch added a comment - If I use a project starter conf which comes together with the script(s) and the additional JARs in one directory, it's helpful to have relative paths as load *.jar . On the other hand, for the global starter conf in $ {groovy.home} /conf relative paths do not make so much sense. Putting it all together, I suggest to allow relative paths in starter conf. Using it in a global starter conf or not, should be left to the decision of the person who installs Groovy on that system.
        Hide
        blackdrag blackdrag added a comment -

        I am not sure that this patch does what it should. The code above is clearly designed to use absolute paths only and the change has a small flaw.... The change transforms ".jar" to "./.jar" and then a pattern will be created, which is more or less like "./<>.jar", where the "." are no regexp (because of quote" and <> is what I used in short for matching a file name. Anyway, what will the filename we use to test this pattern against? Is it safe to assume that new File("build.xml") will create a file, with a name like "./build.xml"? Because if this name is not what will be produced, then the pattern is not safe. Instead we would have to create canonical paths and also replace "./" with the conical path for the current directory.. or is there anywhere a specification that File#getPath http://java.sun.com/j2se/1.4.2/docs/api/java/io/File.html#getPath() will always return the relative path?

        Show
        blackdrag blackdrag added a comment - I am not sure that this patch does what it should. The code above is clearly designed to use absolute paths only and the change has a small flaw.... The change transforms " .jar" to "./ .jar" and then a pattern will be created, which is more or less like "./< >.jar", where the "." are no regexp (because of quote" and < > is what I used in short for matching a file name. Anyway, what will the filename we use to test this pattern against? Is it safe to assume that new File("build.xml") will create a file, with a name like "./build.xml"? Because if this name is not what will be produced, then the pattern is not safe. Instead we would have to create canonical paths and also replace "./" with the conical path for the current directory.. or is there anywhere a specification that File#getPath http://java.sun.com/j2se/1.4.2/docs/api/java/io/File.html#getPath( ) will always return the relative path?
        Hide
        Klaus Kopruch added a comment -

        Ok, if the original code is designed to use absolute paths only then at first it should be made clear if relative paths should be supported by the load command in the starter config.

        • If yes, we should specify and test this new feature.
        • If no, we should change loadFilteredPath() in that way that it does not crash with an exception if one types load *.jar
        Show
        Klaus Kopruch added a comment - Ok, if the original code is designed to use absolute paths only then at first it should be made clear if relative paths should be supported by the load command in the starter config. If yes, we should specify and test this new feature. If no, we should change loadFilteredPath() in that way that it does not crash with an exception if one types load *.jar
        Hide
        blackdrag blackdrag added a comment -

        since there is the system property "user.dir" this one can be used to do relative paths. For example $

        {user.dir}

        /*.jar. So what is really left here is changing javadoc

        Show
        blackdrag blackdrag added a comment - since there is the system property "user.dir" this one can be used to do relative paths. For example $ {user.dir} /*.jar. So what is really left here is changing javadoc
        Hide
        Guillaume Laforge added a comment -

        Fixed JavaDoc.

        Show
        Guillaume Laforge added a comment - Fixed JavaDoc.

          People

          • Assignee:
            Guillaume Laforge
            Reporter:
            Klaus Kopruch
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: