[GLASSFISH-16307] NPE and issues when deploying applications with same name - different case Created: 02/Apr/11  Updated: 11/Sep/12  Resolved: 11/Sep/12

Status: Resolved
Project: glassfish
Component/s: deployment
Affects Version/s: 3.1
Fix Version/s: 4.0_b53

Type: Bug Priority: Major
Reporter: Dies Koper Assignee: Hong Zhang
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

WinXP


Attachments: Zip Archive GLASSFISH-16307_FIXED_SOURCE.zip    
Tags: 3_1_1-scrubbed, 3_1_2-exclude

 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.



 Comments   
Comment by Hong Zhang [ 04/Apr/11 ]

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?

Comment by Hong Zhang [ 04/Apr/11 ]

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

Comment by Tim Quinn [ 04/Apr/11 ]

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.

Comment by scatari [ 04/Nov/11 ]

Please evaluate this for possible inclusion into 3.1.2.

Comment by Hong Zhang [ 14/Dec/11 ]

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

Comment by Jeremy_Lv [ 09/Aug/12 ]

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.

Comment by Hong Zhang [ 09/Aug/12 ]

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);

Comment by Jeremy_Lv [ 10/Aug/12 ]

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.

Comment by Hong Zhang [ 10/Aug/12 ]

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.

Comment by Jeremy_Lv [ 21/Aug/12 ]

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

Comment by Hong Zhang [ 29/Aug/12 ]

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.

Comment by Jeremy_Lv [ 31/Aug/12 ]

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

Comment by Hong Zhang [ 05/Sep/12 ]

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.

Comment by Jeremy_Lv [ 06/Sep/12 ]

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

Comment by Hong Zhang [ 06/Sep/12 ]

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.

Comment by Hong Zhang [ 06/Sep/12 ]

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

Comment by Jeremy_Lv [ 10/Sep/12 ]

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.

Comment by Hong Zhang [ 10/Sep/12 ]

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

Comment by Jeremy_Lv [ 11/Sep/12 ]

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.

Generated at Sat May 23 00:39:51 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.