glassfish
  1. glassfish
  2. GLASSFISH-16307

NPE and issues when deploying applications with same name - different case

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1
    • Fix Version/s: 4.0_b53
    • Component/s: deployment
    • Labels:
      None
    • Environment:

      WinXP

      Description

      I deploy my app 'web25.war' using the CUI. Later I deploy another app 'WEB25.war'. No errors in console or server log, but I noticed domains\domain1\applications only has a directory WEB25, no web25.
      I undeploy 'web25', again seemingly successful. But when I then try to disable WEB25 I get an error:

      D:\GFv3.1\glassfish-3.1\glassfish3\glassfish>bin\asadmin disable
      Enter the value for the name operand> WEB25
      remote failure:
      Command disable failed.

      In server.log an NPE:

      [#|2011-04-03T11:41:31.796+1000|SEVERE|glassfish3.1|javax.enterprise.system.tools.admin.org.glassfish.deployment.admin|_ThreadID=56;_ThreadName=Thread-1;|Error during disabling:
      java.lang.NullPointerException
      at com.sun.enterprise.deploy.shared.FileArchive.getDirectories(FileArchive.java:175)
      at org.glassfish.deployment.common.DeploymentUtils.isEARFromIntrospecting(DeploymentUtils.java:332)
      at org.glassfish.deployment.common.DeploymentUtils.isEAR(DeploymentUtils.java:320)
      at org.glassfish.javaee.full.deployment.EarHandler.handles(EarHandler.java:146)
      [...]
      [#|2011-04-03T11:42:22.218+1000|WARNING|glassfish3.1|javax.enterprise.system.tools.admin.org.glassfish.deployment.admin|_ThreadID=82;_ThreadName=Thread-1;|Cannot find application bits at D:\tests\GFv3.1\glassfish-3.1\glassfish3\glassfish\domains\domain1\applications\WEB25|#]

      If deployment of apps only differing in case is not supported it would be better not to allow it.

        Activity

        Hide
        Hong Zhang added a comment -

        I could not reproduce the issue yet (I was trying with the trunk build). I tried both CLI and GUI, things worked as expected in either case:
        1. deploy web25.war and WEB25.war
        both deployments successful, and the applications repository have both directories
        2. undeploy web25
        undeployment successful
        3. disable WEB25
        disable successful

        Maybe it's something specific to your test cases? Could you attach the test cases you tried and also if possible trying with trunk build to see if it makes any difference?

        Show
        Hong Zhang added a comment - I could not reproduce the issue yet (I was trying with the trunk build). I tried both CLI and GUI, things worked as expected in either case: 1. deploy web25.war and WEB25.war both deployments successful, and the applications repository have both directories 2. undeploy web25 undeployment successful 3. disable WEB25 disable successful Maybe it's something specific to your test cases? Could you attach the test cases you tried and also if possible trying with trunk build to see if it makes any difference?
        Hide
        Hong Zhang added a comment -

        I just realized you were trying on windows. So this could be a windows specific issue.

        Show
        Hong Zhang added a comment - I just realized you were trying on windows. So this could be a windows specific issue.
        Hide
        Tim Quinn added a comment -

        As Hong suspected this is happening because you are on Windows XP which treats two file paths as the same even if they have different case.

        GlassFish itself treats the two applications separately because they have different applicaitons names (web25 and WEB25). But the file system treats the web25 and WEB25 directories in $

        {domainDir}/applications as the same.

        After you undeploy one application the ${domainDir}

        /applications/WEB25 directory is gone, but that is where GlassFish expects to find the files for web25 which, as you noted, causes problems.

        We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app.

        Show
        Tim Quinn added a comment - As Hong suspected this is happening because you are on Windows XP which treats two file paths as the same even if they have different case. GlassFish itself treats the two applications separately because they have different applicaitons names (web25 and WEB25). But the file system treats the web25 and WEB25 directories in $ {domainDir}/applications as the same. After you undeploy one application the ${domainDir} /applications/WEB25 directory is gone, but that is where GlassFish expects to find the files for web25 which, as you noted, causes problems. We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app.
        Hide
        scatari added a comment -

        Please evaluate this for possible inclusion into 3.1.2.

        Show
        scatari added a comment - Please evaluate this for possible inclusion into 3.1.2.
        Hide
        Hong Zhang added a comment -

        Could not get to it in this release, defer it to next release as it's a corner case.

        Show
        Hong Zhang added a comment - Could not get to it in this release, defer it to next release as it's a corner case.
        Hide
        Jeremy_Lv added a comment -

        Dear Hong,Tim:

        We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app.

        I'll investigate this issue, Does the fix for GlassFish is just to reject the deployment of a second app that would share the same directory as a previously-deployed app? I'll offer the patch in few days.

        Show
        Jeremy_Lv added a comment - Dear Hong,Tim: We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app. I'll investigate this issue, Does the fix for GlassFish is just to reject the deployment of a second app that would share the same directory as a previously-deployed app? I'll offer the patch in few days.
        Hide
        Hong Zhang added a comment -

        Yeah, I guess that could be the fix for this corner case. And the error message in addition to say the directory name is already being used, could also suggest the user to use a different application name to avoid conflict. This check could be made in ApplicationLifecycle.getContext method after this:

        File expansionDir = new File(domain.getApplicationRoot(),
        repositoryBitName);

        Show
        Hong Zhang added a comment - Yeah, I guess that could be the fix for this corner case. And the error message in addition to say the directory name is already being used, could also suggest the user to use a different application name to avoid conflict. This check could be made in ApplicationLifecycle.getContext method after this: File expansionDir = new File(domain.getApplicationRoot(), repositoryBitName);
        Hide
        Jeremy_Lv added a comment -

        Dear Hong:
        Thanks for your sugguestions, Anyway, I think we should prevent the first directory to be deleted by follow code path on the environment of windows:
        DeployCommand#excute

        if (reposDir.exists()) {
            /*
            * Delete the repository directory as an archive to allow
            * any special processing (such as stale file handling)
            * to run.
            */
            final FileArchive arch = DeploymentUtils.openAsFileArchive(reposDir, archiveFactory);
            arch.delete();
        }
        

        I'll continue to investigate this issue and upload my patch after my vocation finished.

        Show
        Jeremy_Lv added a comment - Dear Hong: Thanks for your sugguestions, Anyway, I think we should prevent the first directory to be deleted by follow code path on the environment of windows: DeployCommand#excute if (reposDir.exists()) { /* * Delete the repository directory as an archive to allow * any special processing (such as stale file handling) * to run. */ final FileArchive arch = DeploymentUtils.openAsFileArchive(reposDir, archiveFactory); arch.delete(); } I'll continue to investigate this issue and upload my patch after my vocation finished.
        Hide
        Hong Zhang added a comment -

        You are right, the code you pointed above does happen before ApplicationLifecycle.getContext call. So we should do the check here and reject the deployment as needed. One thing we need to be careful here is there are valid cases where we need to clean up any left over directories from last deployment, so probably the check here would be if the reposDir is same, but the app name is different, then we fail deployment.

        Show
        Hong Zhang added a comment - You are right, the code you pointed above does happen before ApplicationLifecycle.getContext call. So we should do the check here and reject the deployment as needed. One thing we need to be careful here is there are valid cases where we need to clean up any left over directories from last deployment, so probably the check here would be if the reposDir is same, but the app name is different, then we fail deployment.
        Hide
        Jeremy_Lv added a comment -

        Dear Hong:
        I have revised this issue and uploaded my revised source files.
        please check it and give me some advices.

        Show
        Jeremy_Lv added a comment - Dear Hong: I have revised this issue and uploaded my revised source files. please check it and give me some advices.
        Hide
        Hong Zhang added a comment -

        We should avoid introducing platform dependent checks in the code. The check should be if the respository directory is the same as a deployed application (for each application element in domain.xml, there is a location URI for where the repository is, you could use new File(new URI(application.getLocation()) to get the repository dir to compare), and the application name is not the same, fail the deployment.

        Show
        Hong Zhang added a comment - We should avoid introducing platform dependent checks in the code. The check should be if the respository directory is the same as a deployed application (for each application element in domain.xml, there is a location URI for where the repository is, you could use new File(new URI(application.getLocation()) to get the repository dir to compare), and the application name is not the same, fail the deployment.
        Hide
        Jeremy_Lv added a comment -

        Dear Hong:
        I have updated my attachments, please review my code and give me some advices.

        Show
        Jeremy_Lv added a comment - Dear Hong: I have updated my attachments, please review my code and give me some advices.
        Hide
        Hong Zhang added a comment -

        The check should include both name and repository dir, if the name does not match, but repository match, that's this reported use case and we want to fail deployment. If the name and repository both match, that's the normal case where we do want to do the clean up for any left over directories.

        Show
        Hong Zhang added a comment - The check should include both name and repository dir, if the name does not match, but repository match, that's this reported use case and we want to fail deployment. If the name and repository both match, that's the normal case where we do want to do the clean up for any left over directories.
        Hide
        Jeremy_Lv added a comment -

        I see. I have fixed it again and uploaded to the JIRA.
        Please check it again.
        Thanks

        Show
        Jeremy_Lv added a comment - I see. I have fixed it again and uploaded to the JIRA. Please check it again. Thanks
        Hide
        Hong Zhang added a comment -

        A couple comments on the new diffs:

        1. For the if block, I think we should compare the path of the repository directories instead of the File object itself (e.g. you can use File.getAbsolutePath() to compare). I don't think we should use an else if block here, you should use the else block for everything else, and we will delete the left over directories. We only want to limit the change to this particular corner use case, and everything else should stay same.

        2. For the error message, please say something like Application <the name of the new app> is trying to use the same repository directory as application <the one that's already deployed>, please choose a different application name to deploy.

        Show
        Hong Zhang added a comment - A couple comments on the new diffs: 1. For the if block, I think we should compare the path of the repository directories instead of the File object itself (e.g. you can use File.getAbsolutePath() to compare). I don't think we should use an else if block here, you should use the else block for everything else, and we will delete the left over directories. We only want to limit the change to this particular corner use case, and everything else should stay same. 2. For the error message, please say something like Application <the name of the new app> is trying to use the same repository directory as application <the one that's already deployed>, please choose a different application name to deploy.
        Hide
        Hong Zhang added a comment -

        And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle).

        Show
        Hong Zhang added a comment - And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle).
        Hide
        Jeremy_Lv added a comment -

        Hong:
        I have fixed the source again under your suggestion.please check it.

        BTW:

        And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle).

        I have run these two manual tests and it seems fine.

        Show
        Jeremy_Lv added a comment - Hong: I have fixed the source again under your suggestion.please check it. BTW: And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle). I have run these two manual tests and it seems fine.
        Hide
        Hong Zhang added a comment - - edited

        The latest changes look pretty good, please go ahead and check in (after running the usual set of the tests).

        Show
        Hong Zhang added a comment - - edited The latest changes look pretty good, please go ahead and check in (after running the usual set of the tests).
        Hide
        Jeremy_Lv added a comment -

        all QL tests and dev tests passed according to the latest version of GF.
        Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\DeployCommand.java
        Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\LocalStrings.properties
        Transmitting file data ..
        Committed revision 55893.

        Show
        Jeremy_Lv added a comment - all QL tests and dev tests passed according to the latest version of GF. Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\DeployCommand.java Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\LocalStrings.properties Transmitting file data .. Committed revision 55893.

          People

          • Assignee:
            Hong Zhang
            Reporter:
            Dies Koper
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: