1 3 4 5 6 7 8 Previous Next 116 Replies Latest reply on Nov 28, 2008 3:21 PM by anil.saldhana Go to original post
      • 60. Re: VFS Permissions - JBMICROCONT-149

         

        "anil.saldhana@jboss.com" wrote:
        Adrian, we have an opportunity to use the jar url format to specify the permissions instead of the vfs url format. I know this is not ideal but I have tested the following:

        grant codeBase "jar:file:${jboss.server.home.dir}/deploy/jms-ra.rar!/jms-ra.jar/-" {
        


        And if you put the rar in an ear, you can't do that which is why the vfs url is superior.

        • 61. Re: VFS Permissions - JBMICROCONT-149
          alesj

           

          "alesj" wrote:
          "adrian@jboss.org" wrote:

          This requires something in the VFS, my suggestion was to add
          VFSUtils.getRealURL(vfsFile);

          Should I hold off VFS 2.0.0.GA until these features get into VFS?

          I see Anil already added this to VFS.
          But it looks wrong. ;-)

           /**
           * Get the Real URL
           * @param vfsURL
           * @return
           * @throws Exception
           */
           public static URL getRealURL(URL vfsURL) throws Exception
           {
           if(vfsURL.getPath().endsWith("jar"))
           {
           String urlStr = "jar:" + FILE_PROTOCOL + ":" + vfsURL.getPath() + JAR_URL_SEPARATOR;
           return new URL(urlStr);
           }
          
           if(vfsURL.getProtocol().startsWith("vfsfile"))
           return new URL(FILE_PROTOCOL, vfsURL.getHost(), vfsURL.getPort(), vfsURL.getFile());
          
           if(vfsURL.getProtocol().startsWith("vfszip"))
           {
           String urlStr = vfsURL.toExternalForm();
           //nested file
           int indexJar = urlStr.indexOf(".jar");
           String beforejar = urlStr.substring("vfszip:".length(), urlStr.indexOf(".jar"));
           String afterjar = urlStr.substring(indexJar + 1 + ".jar".length());
           return new URL("jar:file:" + beforejar + ".jar " + JAR_URL_SEPARATOR + afterjar);
           }
           if(log.isTraceEnabled())
           log.trace("getRealURL did not have a match for:"+vfsURL.toExternalForm());
           return vfsURL;
           }
          


          First some simple code critique.

          If you add something to project you don't sustain,
          be fair and add javadocs as it should be done.
          Since this is probably gonna be called for every class we load,
          some more constants won't kill you.
          Dunno how much optimization javac puts on "vfszip."length() and repeated ".jar".

          Tests should be more exhaustive.
          As I see some extra space after last ".jar that's probably not supposed to be there.
          Dunno how this would work.

          But in general, jar/zip handling is completely broken.
          First, it should be the same code that handles it,
          since, although we currently use vfszip, user can still fall back to old vfsjar handling.
          And they are both the same as far as feature set goes (it's just that we have winz lock problems on vfsjar).
          But the biggest problem I see is how are you gonna handle multiple nested jars.
          Dunno what's the proper/real url for those, but I very much doubt it's how your code does it.
          I guess this would have to be an impl of VirtualFileHandlers, not just VFSUtils.

          Conclusion.
          Either this is done right, or it goes out.
          I'll leave it for now, but will check on it before I do 2.0.0.GA.

          If you need any help on writing this, open a new forum post.
          And I'll see what can be done. ;-)

          • 62. Re: VFS Permissions - JBMICROCONT-149

             

            "alesj" wrote:
            "alesj" wrote:
            "adrian@jboss.org" wrote:

            This requires something in the VFS, my suggestion was to add
            VFSUtils.getRealURL(vfsFile);

            Should I hold off VFS 2.0.0.GA until these features get into VFS?

            I see Anil already added this to VFS.
            But it looks wrong. ;-)


            That's the problem with quick and dirty. ;-)

            I said it should be
            VFSUtils.getRealURL(vfsFile) above but Anil has done URL.

            What should happen is this uses the implemented details of the VirtualFile
            to delegate the request to the vfs handler.
            Only the protocol specific handler knows how it maps its urls to a real url.

            Its called object orientated programming. :-)
            The list of if statements is the clue.
            if you have to write code like (which is also incredibly inefficient)
             if(vfsURL.getPath().endsWith("jar"))
            ...
             if(vfsURL.getProtocol().startsWith("vfsfile"))
            ...
             if(vfsURL.getProtocol().startsWith("vfszip"))
            ...
            etc.
            

            You're almost certainly doing something wrong.

            Correct would be something like:
            public URL getRealURL(VirtualFile file)
            {
             // public methods should always check parameters to give useful error messages
             if (file == null)
             throw new IllegalArgumentException("null file);
             VirtualFileHandler handler = file.getHandler();
             return handler.getRealURL(); // NEW METHOD
            }
            


            But at least its in the right project now. ;-)

            • 63. Re: VFS Permissions - JBMICROCONT-149
              alesj

               

              "adrian@jboss.org" wrote:
              I said it should be
              VFSUtils.getRealURL(vfsFile) above but Anil has done URL.

              What do you consider as real url?

              • 64. Re: VFS Permissions - JBMICROCONT-149

                 

                "alesj" wrote:
                "adrian@jboss.org" wrote:
                I said it should be
                VFSUtils.getRealURL(vfsFile) above but Anil has done URL.

                What do you consider as real url?


                The file: or jar: url
                The closest thing that doesn't need the vfs url handlers.

                • 65. Re: VFS Permissions - JBMICROCONT-149
                  alesj

                   

                  "adrian@jboss.org" wrote:
                  The closest thing that doesn't need the vfs url handlers.

                  How do you represent multiple nested jars in real url?
                  (too lazy to read the spec ;-))

                  • 66. Re: VFS Permissions - JBMICROCONT-149

                     

                    "alesj" wrote:
                    "adrian@jboss.org" wrote:
                    The closest thing that doesn't need the vfs url handlers.

                    How do you represent multiple nested jars in real url?
                    (too lazy to read the spec ;-))


                    Or the earlier posts in this thread. ;-)

                    You can't do it. That's why you've got to use the closest thing.

                    e.g.
                    vfszip:/my.ear/my.rar/my.jar
                    would have to use the jar url for my.rar

                    • 67. Re: VFS Permissions - JBMICROCONT-149
                      alesj

                       

                      "adrian@jboss.org" wrote:
                      "alesj" wrote:
                      "adrian@jboss.org" wrote:
                      The closest thing that doesn't need the vfs url handlers.

                      How do you represent multiple nested jars in real url?
                      (too lazy to read the spec ;-))

                      Or the earlier posts in this thread. ;-)

                      I should have written "lazy in general" then. :-)

                      "adrian@jboss.org" wrote:

                      You can't do it. That's why you've got to use the closest thing.
                      e.g.
                      vfszip:/my.ear/my.rar/my.jar
                      would have to use the jar url for my.rar

                      And this is probably enough, since the permission would be on some parent?
                      As this is also probably the best plain JDK security can offer?

                      • 68. Re: VFS Permissions - JBMICROCONT-149
                        anil.saldhana

                        You can roll back my changes if you like as I am not doing it any more. My 4 day tryst with VFS is not sufficient to complete this task.

                        Since you rolled back my changes, you can do the tasks yourself.

                        I only need that I need the security manager to be start able via system properties (like the classic way).

                        • 69. Re: VFS Permissions - JBMICROCONT-149
                          anil.saldhana

                           

                           */
                           public static final String FILE_PROTOCOL = "file";
                          
                          + /**
                          + * Constant representing the JAR file protocol
                          + */
                          + public static final String JAR_FILE_PROTOCOL = "jar:file:";
                          +
                          + /**
                          + * Constant representing the VFS ZIP protocol
                          + */
                          + public static final String VFSZIP_PROTOCOL = "vfszip";
                          +
                          + /**
                          + * Constant representing the VFS file protocol
                          + */
                          + public static final String VFS_FILE_PROTOCOL = "vfsfile";
                          +
                           /** Standard separator for JAR URL */
                           public static final String JAR_URL_SEPARATOR = "!/";
                          
                          @@ -157,6 +172,40 @@
                          
                           return buffer.toString();
                           }
                          +<<<<<<< .mine
                          +
                          + /**
                          + * Get the Real URL
                          + * @param vfsURL
                          + * @return
                          + * @throws Exception
                          + */
                          + public static URL getRealURL(URL vfsURL) throws Exception
                          + {
                          + if(vfsURL.getProtocol().startsWith(VFS_FILE_PROTOCOL)) //vfsfile:
                          + return new URL(FILE_PROTOCOL, vfsURL.getHost(), vfsURL.getPort(), vfsURL.getFile());
                          +
                          + if(vfsURL.getProtocol().startsWith(VFSZIP_PROTOCOL)) //vfszip:
                          + {
                          + String urlStr = vfsURL.toExternalForm();
                          +
                          + //Look for the first matching archive (sar, war, ear, jar)
                          + String tokens[] = urlStr.split(".[a-z&&[jsrew]]ar", 2);
                          +
                          + int lengthOfToken = tokens[0].length();
                          + String typeArchive = urlStr.substring(lengthOfToken,lengthOfToken + 4);
                          +
                          + String firstPath = tokens[0].substring(VFSZIP_PROTOCOL.length() + 1);
                          +
                          + return new URL( new URL(JAR_FILE_PROTOCOL + firstPath + typeArchive + JAR_URL_SEPARATOR ),
                          + tokens[1]);
                          + }
                          + if(log.isTraceEnabled())
                          + log.trace("getRealURL did not have a match for:"+vfsURL.toExternalForm());
                          + return vfsURL;
                          + }
                          +=======
                          +>>>>>>> .r80795
                          
                          


                          That was the final version I was supposed to put last night but failed and now it has been rolled back.

                          • 70. Re: VFS Permissions - JBMICROCONT-149
                            anil.saldhana

                             

                            "adrian@jboss.org" wrote:
                            "anil.saldhana@jboss.com" wrote:
                            Adrian, we have an opportunity to use the jar url format to specify the permissions instead of the vfs url format. I know this is not ideal but I have tested the following:

                            grant codeBase "jar:file:${jboss.server.home.dir}/deploy/jms-ra.rar!/jms-ra.jar/-" {
                            


                            And if you put the rar in an ear, you can't do that which is why the vfs url is superior.


                            My last commit that did not get in, was able to generate an URL instance for that.

                            • 71. Re: VFS Permissions - JBMICROCONT-149
                              alesj

                               

                              "anil.saldhana@jboss.com" wrote:

                              That was the final version I was supposed to put last night but failed and now it has been rolled back.

                              Still wrong. ;-)

                              Read about OO and doing this via VFHandler::getRealURL().
                              Not to mention that your parameter is URL.
                              What's the benefit of using VFS(Utils) then?

                              Never mind, I'll (or get Marko to) do it.
                              In the future, post on MC dev forum before you go and start hacking.
                              We might stop you before you spend 4 days of VFS tryst. :-)

                              • 72. Re: VFS Permissions - JBMICROCONT-149
                                anil.saldhana

                                 

                                "alesj" wrote:
                                "alesj" wrote:
                                "adrian@jboss.org" wrote:

                                This requires something in the VFS, my suggestion was to add
                                VFSUtils.getRealURL(vfsFile);

                                Should I hold off VFS 2.0.0.GA until these features get into VFS?

                                I see Anil already added this to VFS.
                                But it looks wrong. ;-)

                                 /**
                                 * Get the Real URL
                                 * @param vfsURL
                                 * @return
                                 * @throws Exception
                                 */
                                 public static URL getRealURL(URL vfsURL) throws Exception
                                 {
                                 if(vfsURL.getPath().endsWith("jar"))
                                 {
                                 String urlStr = "jar:" + FILE_PROTOCOL + ":" + vfsURL.getPath() + JAR_URL_SEPARATOR;
                                 return new URL(urlStr);
                                 }
                                
                                 if(vfsURL.getProtocol().startsWith("vfsfile"))
                                 return new URL(FILE_PROTOCOL, vfsURL.getHost(), vfsURL.getPort(), vfsURL.getFile());
                                
                                 if(vfsURL.getProtocol().startsWith("vfszip"))
                                 {
                                 String urlStr = vfsURL.toExternalForm();
                                 //nested file
                                 int indexJar = urlStr.indexOf(".jar");
                                 String beforejar = urlStr.substring("vfszip:".length(), urlStr.indexOf(".jar"));
                                 String afterjar = urlStr.substring(indexJar + 1 + ".jar".length());
                                 return new URL("jar:file:" + beforejar + ".jar " + JAR_URL_SEPARATOR + afterjar);
                                 }
                                 if(log.isTraceEnabled())
                                 log.trace("getRealURL did not have a match for:"+vfsURL.toExternalForm());
                                 return vfsURL;
                                 }
                                


                                First some simple code critique.

                                If you add something to project you don't sustain,
                                be fair and add javadocs as it should be done.
                                Since this is probably gonna be called for every class we load,
                                some more constants won't kill you.
                                Dunno how much optimization javac puts on "vfszip."length() and repeated ".jar".

                                Tests should be more exhaustive.
                                As I see some extra space after last ".jar that's probably not supposed to be there.
                                Dunno how this would work.

                                But in general, jar/zip handling is completely broken.
                                First, it should be the same code that handles it,
                                since, although we currently use vfszip, user can still fall back to old vfsjar handling.
                                And they are both the same as far as feature set goes (it's just that we have winz lock problems on vfsjar).
                                But the biggest problem I see is how are you gonna handle multiple nested jars.
                                Dunno what's the proper/real url for those, but I very much doubt it's how your code does it.
                                I guess this would have to be an impl of VirtualFileHandlers, not just VFSUtils.

                                Conclusion.
                                Either this is done right, or it goes out.
                                I'll leave it for now, but will check on it before I do 2.0.0.GA.

                                If you need any help on writing this, open a new forum post.
                                And I'll see what can be done. ;-)


                                Thanks for your comments. I already had generated the constants. I was unable to put it back as I forgot before I slept and you rolled back.

                                • 73. Re: VFS Permissions - JBMICROCONT-149
                                  alesj

                                   

                                  "anil.saldhana@jboss.com" wrote:

                                  I already had generated the constants. I was unable to put it back as I forgot before I slept and you rolled back.

                                  I didn't rollback b/c of lack of constants.
                                  I did it b/c it was wrong and the code actually is not useful --> needs to be in proper VFHandler.
                                  Even the method signature didn't fit the needs. ;-)

                                  As you could probably also see, I've rolled back your CL changes as well.
                                  Using System property in such way w/o discussing it before is a huge no-no.
                                  If you really needed that you could either
                                  - make it true by default (as it is now)
                                  - created few new classes in jbossas: deployer + policy combination that sets that flag to true

                                  Dunno how you do it in security project, but I would really appreciate it
                                  if in the future you discussed things before here on the forum.
                                  Saving us both time. ;-)


                                  • 74. Re: VFS Permissions - JBMICROCONT-149
                                    anil.saldhana

                                     

                                    "alesj" wrote:

                                    As you could probably also see, I've rolled back your CL changes as well.
                                    Using System property in such way w/o discussing it before is a huge no-no.
                                    If you really needed that you could either
                                    - make it true by default (as it is now)
                                    - created few new classes in jbossas: deployer + policy combination that sets that flag to true


                                    AS5 today is booting with the VFSClassloader system via the system property? Then why don't you fix that (by making things injectable) first rather than preach here about using a system property (in fact the vfs project has a lot of system properties)? Here let me show you.

                                    conf/classloader.xml
                                    
                                     <!--
                                     The classloader implementation
                                     -->
                                     <bean name="ClassLoaderSystem" class="org.jboss.classloader.spi.ClassLoaderSystem">
                                     <classloader><null/></classloader>
                                     <constructor factoryClass="org.jboss.classloader.spi.ClassLoaderSystem" factoryMethod="getInstance"/>
                                     </bean>
                                    
                                    


                                    Now let us look at the code.
                                    
                                    ClassLoaderSystem.java
                                    -----------------------------
                                     /** The class loading system builder */
                                     private static final ClassLoaderSystemBuilder builder = new ClassLoaderSystemBuilder();
                                    
                                    public static final ClassLoaderSystem getInstance()
                                     {
                                     SecurityManager sm = System.getSecurityManager();
                                     if (sm != null)
                                     sm.checkCreateClassLoader();
                                     return builder.get();
                                     }
                                    
                                    
                                    public class ClassLoaderSystemBuilder
                                    {
                                     /** The singleton */
                                     private static final ClassLoaderSystem singleton;
                                    
                                     static
                                     {
                                     singleton = AccessController.doPrivileged(new PrivilegedAction<ClassLoaderSystem>()
                                     {
                                     public ClassLoaderSystem run()
                                     {
                                     String className = System.getProperty(ClassLoaderSystem.class.getName(), DefaultClassLoaderSystem.class.getName());
                                     try
                                     {
                                     Class<?> clazz = Thread.currentThread().getContextClassLoader().loadClass(className);
                                     Object result = clazz.newInstance();
                                     return ClassLoaderSystem.class.cast(result);
                                     }
                                     catch (RuntimeException e)
                                     {
                                     throw e;
                                     }
                                     catch (Exception e)
                                     {
                                     throw new Error("Unexpected error loading ClassLoaderSystem " + className, e);
                                     }
                                     }
                                     });
                                    
                                     public static ClassLoaderSystem get()
                                     {
                                     return singleton;
                                     }
                                     }
                                    
                                    
                                    DefaultClassLoaderSystem.java
                                    -------------------------------------
                                    public class DefaultClassLoaderSystem extends ClassLoaderSystem
                                    {
                                     @Override
                                     protected ClassLoaderDomain createDomain(String name)
                                     {
                                     return new ClassLoaderDomain(name);
                                     }
                                    }
                                    
                                    and so on.
                                    
                                    


                                    There, there is no easy way to get to VFSClassloaderPolicy at the moment for configuration. If it was provided, then it would have been easier. :)


                                    1 3 4 5 6 7 8 Previous Next