Issue Details (XML | Word | Printable)

Key: GLASSFISH-7561
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Hong Zhang
Reporter: Alexis MP
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
glassfish

Check for existence of libraries files when deploying with --libraries

Created: 09/Apr/09 12:44 PM   Updated: 11/Jul/13 03:51 AM   Resolved: 11/Jul/13 03:51 AM
Component/s: deployment
Affects Version/s: v2.1
Fix Version/s: 4.0.1

Time Tracking:
Not Specified

Environment:

Operating System: All
Platform: All


Issuezilla Id: 7,561
Tags: ee7ri_cleanup_deferred
Participants: Alexis MP, Hong Zhang, Jeremy_Lv, kumara and Tom Mueller


 Description  « Hide

When deploying with --libraries, the deployment process does not check if the specified files actually
exist on disk.

For users (like me) not reading the documentation which states that "relative paths are relative to
instance-root/lib/applibs", the deployment is said to have completed successfully but the libraries cannot
be located at runtime (which could lead people to think this is some sort of class-loader issue).

I would suggest that the deployment should :

  • echo the absolute name of the library
  • fail when the library file(s) don't exist on disk at deploy-time


kumara added a comment - 01/Sep/09 01:09 AM

Changing version from 9.1.1 to v2.1 to reflect new name/version.


Hong Zhang added a comment - 23/Mar/11 06:40 PM

target for 3.2


Tom Mueller added a comment - 17/Oct/12 08:19 PM

Marking the fix version field as "future-release". This is based on an evaluation by John, Michael, and Tom WRT to the PRD for the Java EE 7 RI/SDK. This issues was deemed to not be a P1 for that release. If this is in error or there are other reasons why this RFE should be targeted for the Java EE 7 RI/SDK release, then change the fix version field back to an appropriate build.


Jeremy_Lv added a comment - 15/Apr/13 08:48 AM

Hong:
It seems this issue has been worked as designed. I have test the --libraries option specified a lib which doesn't exist on the disc and some of the error messages as follow:

E:\glassfish4\glassfish\bin>asadmin deploy --libraries d:\nonexist.jar e:\Test_s
ample\ServletEJB.ear
remote failure: Error occurred during deployment: Exception while loading the ap
p : java.lang.IllegalStateException: ContainerBase.addChild: start: org.apache.c
atalina.LifecycleException: java.lang.RuntimeException: java.io.FileNotFoundExce
ption: d:\nonexist.jar (指定されたファイルが見つかりません。). Please see server
.log for more details.
Command deploy failed.

So I think the list two tips has been implemented.


Hong Zhang added a comment - 16/Apr/13 02:12 AM

Jeremy: I think what the original request wanted is for the deployment infrastructure to do a check of the path before it passes control to the container or other backend code(such as web container in the case you tried). In your test, the web container gives a pretty clear error message, but in other cases it could lead to some unpredicted failures with error messages which do not provide much help for user to locate the cause of the error.


Jeremy_Lv added a comment - 16/Apr/13 02:34 AM

Hong:
You are right, I have test the situation when deploy the ejb application and the application can be deployed successfully without any clear error messages. Whether should we do some validations on deployment side?


Hong Zhang added a comment - 16/Apr/13 02:46 AM

Yes, you could add the validation for libraries in DeploymentContextImpl.getAppLibs, this is where we process the library URLs:

if (parameters.libraries() != null) {
URL[] urls =
ASClassLoaderUtil.getDeployParamLibrariesAsURLs(
parameters.libraries(), env);
for (URL url : urls) { libURIs.add(url.toURI()); }

You could check if each library URL is a valid/existent URL before you add it to the list and log the absolute path in the server.log.


Jeremy_Lv added a comment - 16/Apr/13 02:54 AM

Ok, Thanks, I will do some check about this and comments my changes here later.


Jeremy_Lv added a comment - 16/Apr/13 07:04 AM - edited

Hong:

How about change the code as follows:

DeploymentContextImpl.java
Index: DeploymentContextImpl.java
===================================================================
--- DeploymentContextImpl.java	(revision 61360)
+++ DeploymentContextImpl.java	(working copy)
@@ -55,6 +55,7 @@
 import org.glassfish.loader.util.ASClassLoaderUtil;
 
 import java.util.*;
+import java.util.jar.JarFile;
 import java.util.logging.Logger;
 import java.util.logging.Level;
 import java.io.File;
@@ -66,6 +67,8 @@
 import java.net.URLClassLoader;
 
 import org.glassfish.hk2.api.PreDestroy;
+
+import com.sun.enterprise.util.CULoggerInfo;
 import com.sun.enterprise.util.io.FileUtils;
 import org.glassfish.hk2.classmodel.reflect.Parser;
 import org.glassfish.hk2.classmodel.reflect.Types;
@@ -444,7 +447,17 @@
                 ASClassLoaderUtil.getDeployParamLibrariesAsURLs(
                     parameters.libraries(), env);
             for (URL url : urls) {
-                libURIs.add(url.toURI());
+                try {
+                    new JarFile(new File(url.getFile()));
+                    libURIs.add(url.toURI());
+                } catch (IOException ex) {
+                    if (deplLogger.isLoggable(Level.WARNING)) {
+                        deplLogger.log(Level.WARNING, 
+                                CULoggerInfo.getString(CULoggerInfo.exceptionJarOpen, url.toString()), 
+                                ex);
+                    }
+                    throw new RuntimeException(ex);
+                }
             }
         }

After this, you will find the friendly error messages were thrown out as:

E:\glassfish4\glassfish\bin>asadmin deploy --libraries d:\nonexists.jar d:\Test_
sample\Hello.jar
remote failure: Error occurred during deployment: Exception while deploying the
app [Hello] : java.io.FileNotFoundException: file:\d:\nonexists.jar (ファイル名
、ディレクトリ名、またはボリューム ラベルの構文が間違っています。). Please see s
erver.log for more details.
Command deploy failed.

Here's some server log:

[2013-04-16T14:57:01.471+0900] [glassfish 4.0] [SEVERE] [] [javax.enterprise.system.core] [tid: _ThreadID=36 _ThreadName=admin-listener(2)] [timeMillis: 1366091821471] [levelValue: 1000] [[
  Exception while deploying the app [Hello]]]

[2013-04-16T14:57:01.471+0900] [glassfish 4.0] [SEVERE] [NCLS-CORE-00026] [javax.enterprise.system.core] [tid: _ThreadID=36 _ThreadName=admin-listener(2)] [timeMillis: 1366091821471] [levelValue: 1000] [[
  Exception during lifecycle processing
java.lang.RuntimeException: java.io.FileNotFoundException: file:\d:\nonexists.jar (ファイル名、ディレクトリ名、またはボリューム ラベルの構文が間違っています。)
	at org.glassfish.deployment.common.DeploymentContextImpl.getAppLibs(DeploymentContextImpl.java:459)
	at org.glassfish.deployment.common.DeploymentContextImpl.createClassLoader(DeploymentContextImpl.java:241)
	at org.glassfish.deployment.common.DeploymentContextImpl.createDeploymentClassLoader(DeploymentContextImpl.java:226)
	at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:362)
	at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:219)
	at org.glassfish.deployment.admin.DeployCommand.execute(DeployCommand.java:491)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl$2$1.run(CommandRunnerImpl.java:527)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl$2$1.run(CommandRunnerImpl.java:523)
	at java.security.AccessController.doPrivileged(Native Method)
	at javax.security.auth.Subject.doAs(Subject.java:356)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl$2.execute(CommandRunnerImpl.java:522)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:546)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:1423)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl.access$1500(CommandRunnerImpl.java:108)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1762)
	at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1674)
	at org.glassfish.admin.rest.resources.admin.CommandResource.executeCommand(CommandResource.java:396)
	at org.glassfish.admin.rest.resources.admin.CommandResource.execCommandSimpInMultOut(CommandResource.java:234)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:601)
	at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory$1.invoke(ResourceMethodInvocationHandlerFactory.java:81)
	at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:125)
	at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$ResponseOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:152)
	at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:91)
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:346)
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:341)
	at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:101)
	at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:217)
	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:231)
	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:227)
	at org.glassfish.jersey.internal.Errors.process(Errors.java:275)
	at org.glassfish.jersey.internal.Errors.process(Errors.java:257)
	at org.glassfish.jersey.internal.Errors.process(Errors.java:227)
	at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:317)
	at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:191)
	at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:819)
	at org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpContainer.service(GrizzlyHttpContainer.java:325)
	at org.glassfish.admin.rest.adapter.JerseyContainerCommandService$3.service(JerseyContainerCommandService.java:161)
	at org.glassfish.admin.rest.adapter.RestAdapter.service(RestAdapter.java:181)
	at com.sun.enterprise.v3.services.impl.ContainerMapper.service(ContainerMapper.java:246)
	at org.glassfish.grizzly.http.server.HttpHandler.runService(HttpHandler.java:191)
	at org.glassfish.grizzly.http.server.HttpHandler.doHandle(HttpHandler.java:168)
	at org.glassfish.grizzly.http.server.HttpServerFilter.handleRead(HttpServerFilter.java:189)
	at org.glassfish.grizzly.filterchain.ExecutorResolver$9.execute(ExecutorResolver.java:119)
	at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeFilter(DefaultFilterChain.java:288)
	at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeChainPart(DefaultFilterChain.java:206)
	at org.glassfish.grizzly.filterchain.DefaultFilterChain.execute(DefaultFilterChain.java:136)
	at org.glassfish.grizzly.filterchain.DefaultFilterChain.process(DefaultFilterChain.java:114)
	at org.glassfish.grizzly.ProcessorExecutor.execute(ProcessorExecutor.java:77)
	at org.glassfish.grizzly.nio.transport.TCPNIOTransport.fireIOEvent(TCPNIOTransport.java:838)
	at org.glassfish.grizzly.strategies.AbstractIOStrategy.fireIOEvent(AbstractIOStrategy.java:113)
	at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.run0(WorkerThreadIOStrategy.java:115)
	at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.access$100(WorkerThreadIOStrategy.java:55)
	at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy$WorkerThreadRunnable.run(WorkerThreadIOStrategy.java:135)
	at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.doWork(AbstractThreadPool.java:564)
	at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.run(AbstractThreadPool.java:544)
	at java.lang.Thread.run(Thread.java:722)
Caused by: java.io.FileNotFoundException: file:\d:\nonexists.jar (ファイル名、ディレクトリ名、またはボリューム ラベルの構文が間違っています。)
	at java.util.zip.ZipFile.open(Native Method)
	at java.util.zip.ZipFile.<init>(ZipFile.java:214)
	at java.util.zip.ZipFile.<init>(ZipFile.java:144)
	at java.util.jar.JarFile.<init>(JarFile.java:152)
	at java.util.jar.JarFile.<init>(JarFile.java:116)
	at org.glassfish.deployment.common.DeploymentContextImpl.getAppLibs(DeploymentContextImpl.java:451)
	... 58 more
]]

Hong Zhang added a comment - 16/Apr/13 04:00 PM

Thanks for looking into this! Please just use File API to check the existence of the URL (with JarFile, we need to be careful to always close it afterwards so it does not cause any locking issue), and I think we should probably compose the error message ourselves when the File does not exist something like "Specified library jar xxx does not exist" so it's more obvious to user what went wrong, and please also log the full path of the library jar in the server.log.


Jeremy_Lv added a comment - 17/Apr/13 02:17 AM

I see, How about the changes as follows:

DeploymentContextImpl.java
Index: DeploymentContextImpl.java
===================================================================
--- DeploymentContextImpl.java	(revision 61360)
+++ DeploymentContextImpl.java	(working copy)
@@ -444,7 +444,14 @@
                 ASClassLoaderUtil.getDeployParamLibrariesAsURLs(
                     parameters.libraries(), env);
             for (URL url : urls) {
-                libURIs.add(url.toURI());
+                File file = new File(url.getFile());
+                if (file.exists()){
+                    libURIs.add(url.toURI());
+                } else {
+                    String errorMsg = "Specified library jar "+ file.getName() + " does not exist";
+                    deplLogger.log(Level.SEVERE, errorMsg + ": " + file.getAbsolutePath());
+                    throw new RuntimeException(errorMsg);
+                }
             }
         }

Hong Zhang added a comment - 18/Apr/13 02:35 PM

Thanks for submitting the patch. A couple comments:
1. Can we use localized string for the exception message (define a local string in LocalStrings.properties etc)? Probably better for the exception to be IllegalArgumentException as the message is for an invalid command line option value.
2. For the log message, I was thinking to log it (probably as FINE message) for all cases where the library jar option is used, but we don't have to. For the error case, the exception will automatically logged in the server.log anyways so we don't need to log it separately.


Jeremy_Lv added a comment - 18/Apr/13 03:51 PM

Hong:
I have changed the modification again as you comments..

Index: src/main/java/org/glassfish/deployment/common/DeploymentContextImpl.java
===================================================================
--- src/main/java/org/glassfish/deployment/common/DeploymentContextImpl.java	(revision 61521)
+++ src/main/java/org/glassfish/deployment/common/DeploymentContextImpl.java	(working copy)
@@ -41,6 +41,7 @@
 package org.glassfish.deployment.common;
 
 import com.sun.enterprise.config.serverbeans.ServerTags;
+
 import org.glassfish.deployment.versioning.VersioningUtils;
 import java.lang.instrument.ClassFileTransformer;
 import org.glassfish.api.ActionReport;
@@ -66,6 +67,8 @@
 import java.net.URLClassLoader;
 
 import org.glassfish.hk2.api.PreDestroy;
+
+import com.sun.enterprise.util.LocalStringManagerImpl;
 import com.sun.enterprise.util.io.FileUtils;
 import org.glassfish.hk2.classmodel.reflect.Parser;
 import org.glassfish.hk2.classmodel.reflect.Types;
@@ -89,6 +92,8 @@
     public static final Logger deplLogger =
         Logger.getLogger(DEPLOYMENT_LOGGER, SHARED_LOGMESSAGE_RESOURCE);
 
+    final private static LocalStringManagerImpl localStrings = new LocalStringManagerImpl(DeploymentContextImpl.class);
+
     private static final String INTERNAL_DIR_NAME = "__internal";
     private static final String APP_TENANTS_SUBDIR_NAME = "__app-tenants";
 
@@ -445,6 +450,14 @@
                     parameters.libraries(), env);
             for (URL url : urls) {
                 libURIs.add(url.toURI());
+                File file = new File(url.getFile());
+                if (file.exists()){
+                    libURIs.add(url.toURI());
+                    deplLogger.log(Level.FINE, "Specified library jar: "+file.getAbsolutePath());
+                } else {
+                    deplLogger.log(Level.SEVERE, localStrings.getLocalString("enterprise.deployment.nonexist.libraries", "Specified library jar {0} does not exist: {1}", file.getName(), file.getAbsolutePath()));
+                    throw new IllegalArgumentException();
+                }
             }
         }
 
Index: src/main/resources/org/glassfish/deployment/common/LocalStrings.properties
===================================================================
--- src/main/resources/org/glassfish/deployment/common/LocalStrings.properties	(revision 61521)
+++ src/main/resources/org/glassfish/deployment/common/LocalStrings.properties	(working copy)
@@ -248,3 +248,6 @@
 enterprise.deployment.backend.get_descriptor_failed=Failed to load deployment descriptor for application ${0}.
 illegal_char_in_name=Illegal character [{0}] in the name [{1}].
 enterprise.deployment.remoteDirPathUnusable=The directory deployment path {0} is not a directory or is inaccessible
+
+#DeploymentContextImpl.java
+enterprise.deployment.nonexist.libraries=Specified library jar {0} does not exist: {1}
\ No newline at end of file

Hong Zhang added a comment - 18/Apr/13 05:48 PM

Thanks for creating the local string. Can we move the logging of the file path before the "if" block so it's done for both cases? And we can remove the logging inside the else block as the exception will be logged anyways, and construct the IllegalArgumentException with the error message you created using the local strings. Thanks.


Jeremy_Lv added a comment - 19/Apr/13 02:49 AM

Ok, Here's changes as follows:

Index: main/java/org/glassfish/deployment/common/DeploymentContextImpl.java
===================================================================
--- main/java/org/glassfish/deployment/common/DeploymentContextImpl.java	(revision 61484)
+++ main/java/org/glassfish/deployment/common/DeploymentContextImpl.java	(working copy)
@@ -66,6 +66,8 @@
 import java.net.URLClassLoader;
 
 import org.glassfish.hk2.api.PreDestroy;
+
+import com.sun.enterprise.util.LocalStringManagerImpl;
 import com.sun.enterprise.util.io.FileUtils;
 import org.glassfish.hk2.classmodel.reflect.Parser;
 import org.glassfish.hk2.classmodel.reflect.Types;
@@ -89,6 +91,8 @@
     public static final Logger deplLogger =
         Logger.getLogger(DEPLOYMENT_LOGGER, SHARED_LOGMESSAGE_RESOURCE);
 
+    final private static LocalStringManagerImpl localStrings = new LocalStringManagerImpl(DeploymentContextImpl.class);
+
     private static final String INTERNAL_DIR_NAME = "__internal";
     private static final String APP_TENANTS_SUBDIR_NAME = "__app-tenants";
 
@@ -444,7 +448,14 @@
                 ASClassLoaderUtil.getDeployParamLibrariesAsURLs(
                     parameters.libraries(), env);
             for (URL url : urls) {
-                libURIs.add(url.toURI());
+                File file = new File(url.getFile());
+                deplLogger.log(Level.FINE, "Specified library jar: "+file.getAbsolutePath());
+                if (file.exists()){
+                    libURIs.add(url.toURI());
+                } else {
+                    throw new IllegalArgumentException(localStrings.getLocalString("enterprise.deployment.nonexist.libraries", "Specified library jar {0} does not exist: {1}", file.getName(), file.getAbsolutePath()));
+                }
+
             }
         }
 
Index: main/resources/org/glassfish/deployment/common/LocalStrings.properties
===================================================================
--- main/resources/org/glassfish/deployment/common/LocalStrings.properties	(revision 61484)
+++ main/resources/org/glassfish/deployment/common/LocalStrings.properties	(working copy)
@@ -248,3 +248,7 @@
 enterprise.deployment.backend.get_descriptor_failed=Failed to load deployment descriptor for application ${0}.
 illegal_char_in_name=Illegal character [{0}] in the name [{1}].
 enterprise.deployment.remoteDirPathUnusable=The directory deployment path {0} is not a directory or is inaccessible
+
+
+#DeploymentContextImpl.java
+enterprise.deployment.nonexist.libraries=Specified library jar {0} does not exist: {1}

Hong Zhang added a comment - 19/Apr/13 02:17 PM

The updated changes look good, please hold on to this set of the change and we can check in once the branch is created. Thanks.


Jeremy_Lv added a comment - 06/May/13 02:05 AM

Sending deployment/common/src/main/java/org/glassfish/deployment/common/DeployContextImpl.java

Sending deployment/common/src/main/resource/org/glassfish/deployment/common/LocalStrings.properties

Transmitting file data .
Committed revision 61834.


Jeremy_Lv added a comment - 07/May/13 01:14 AM

Revert the changes as the changes cause the ejb dev tests failed.


Jeremy_Lv added a comment - 11/Jul/13 03:51 AM

Sending deployment/common/src/main/java/org/glassfish/deployment/common/DeployContextImpl.java

Sending deployment/common/src/main/resource/org/glassfish/deployment/common/LocalStrings.properties

Transmitting file data .
Committed revision 62003.