admin@glassfish.java.net

Re: code review for issue 694

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Wed, 20 Sep 2006 14:03:14 -0700

Note to everyone:

90% of our problems with spaces in dir-names are because of calling
Runtime.exec(String) instead of Runtime.exec(String[])

The reason that (String) doesn't work and (String[]) does work is that
the OS (both UNIX and Windows) treat a space in a OS level command as a
separator.
On the other hand, if the space is embedded inside a String in an array,
it is treated like any other character.

I think the following ought to work better (if "args" is more than 1
arg, you'll have to break it into strings too)

Runtime.getRuntime().exec(new String[] {"/bin/chmod", args,
file.getAbsolutePath()} );

=============================================================
I don't like the looks of the script changes. Have you exhaustively
tested it?

Below are the 4, and only 4, places that use ASENV_CONF_LOCATION in
startserv.bat in my installation:

set ASENV_CONF_LOCATION=c:/ee/config
if exist "%ASENV_CONF_LOCATION%\asenv.bat" goto gotCmdPath
call "%ASENV_CONF_LOCATION%\asenv.bat"
-Dcom.sun.aas.configRoot="%ASENV_CONF_LOCATION%"

----------
Say AS is installed in "c:/a b". The current template will produce:
set ASENV_CONF_LOCATION=c:/a b
if exist "c:/a b\asenv.bat" goto gotCmdPath
call "c:/a b\asenv.bat"
-Dcom.sun.aas.configRoot="c:/a b"

Your changes will produce this:

set ASENV_CONF_LOCATION="c:/a b"
if exist ""c:/a b"\asenv.bat" goto gotCmdPath
call ""c:/a b"\asenv.bat"
-Dcom.sun.aas.configRoot="c:/a b"

-- that lookjs like trouble to me!
=========================================================================

Jane Young wrote:
> Hi,
>
> Please review fix for Issue 694 and CR 6443427:
>
> Diffs:
>
> Index:
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java
> ===================================================================
> RCS file:
> /cvs/glassfish/admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java,v
> retrieving revision 1.4
> diff -u -r1.4 KeystoreManager.java
> ---
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java
> 25 Dec 2005 03:44:02 -0000 1.4
> +++
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java
> 20 Sep 2006 19:11:40 -0000
> @@ -427,8 +427,11 @@
> protected void chmod(String args, File file) throws IOException
> {
> if (OS.isUNIX()) {
> + //fix issue 694: Runtime.getRunTime().exec(String) does not
> + //work if there is a space in the directory path, so need
> to specify the
> + //directory in Runtime.getRuntime().exec(String, null,
> directory_path).
> Runtime.getRuntime().exec("/bin/chmod " + args + " " +
> - file.getAbsolutePath());
> + file.getName(), null,
> file.getParentFile());
> }
> }
> }
>
> Index: admin/templates/pe80/startserv.tomcat.template
> ===================================================================
> RCS file: /cvs/glassfish/admin/templates/pe80/startserv.tomcat.template,v
> retrieving revision 1.3
> diff -u -r1.3 startserv.tomcat.template
> --- admin/templates/pe80/startserv.tomcat.template 22 Aug 2005
> 05:24:33 -0000 1.3
> +++ admin/templates/pe80/startserv.tomcat.template 20 Sep 2006
> 19:11:40 -0000
> @@ -8,7 +8,7 @@
>
> # Resolve links - $0 may be a softlink
> PRG="$0"
> -ASENV_CONF_LOCATION=%%%CONFIG_HOME%%%
> +ASENV_CONF_LOCATION="%%%CONFIG_HOME%%%"
>
> SERVER_NAME=%%%SERVER_NAME%%%
> DOMAIN_NAME=%%%DOMAIN_NAME%%%
> @@ -30,7 +30,7 @@
> . "$ASENV_CONF_LOCATION/asenv.conf"
> fi
>
> -INSTANCE_ROOT=%%%INSTANCE_ROOT%%%
> +INSTANCE_ROOT="%%%INSTANCE_ROOT%%%"
>
> JAVA_HOME="$AS_JAVA"
> # Make sure prerequisite environment variables are set
>
> Index: admin/templates/pe80/stopserv.tomcat.template
> ===================================================================
> RCS file: /cvs/glassfish/admin/templates/pe80/stopserv.tomcat.template,v
> retrieving revision 1.3
> diff -u -r1.3 stopserv.tomcat.template
> --- admin/templates/pe80/stopserv.tomcat.template 22 Aug 2005
> 05:24:33 -0000 1.3
> +++ admin/templates/pe80/stopserv.tomcat.template 20 Sep 2006
> 19:58:45 -0000
> @@ -9,7 +9,7 @@
>
> # Resolve links - $0 may be a softlink
> PRG="$0"
> -ASENV_CONF_LOCATION=%%%CONFIG_HOME%%%
> +ASENV_CONF_LOCATION="%%%CONFIG_HOME%%%"
>
> SERVER_NAME=%%%SERVER_NAME%%%
>
> @@ -30,7 +30,7 @@
> . "$ASENV_CONF_LOCATION/asenv.conf"
> fi
>
> -INSTANCE_ROOT=%%%INSTANCE_ROOT%%%
> +INSTANCE_ROOT="%%%INSTANCE_ROOT%%%"
>
> JAVA_HOME="$AS_JAVA"
> # Make sure prerequisite environment variables are set
>
>
>
> Index: admin-ee/templates/ee80/startserv.tomcat.template
> ===================================================================
> RCS file:
> /cvs/glassfish/admin-ee/templates/ee80/startserv.tomcat.template,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 startserv.tomcat.template
> --- templates/ee80/startserv.tomcat.template 8 Aug 2006 19:48:40
> -0000 1.1.1.1
> +++ templates/ee80/startserv.tomcat.template 20 Sep 2006 19:13:12 -0000
> @@ -8,7 +8,7 @@
>
> # Resolve links - $0 may be a softlink
> PRG="$0"
> -ASENV_CONF_LOCATION=%%%CONFIG_HOME%%%
> +ASENV_CONF_LOCATION="%%%CONFIG_HOME%%%"
>
> SERVER_NAME=%%%SERVER_NAME%%%
> DOMAIN_NAME=%%%DOMAIN_NAME%%%
> @@ -30,7 +30,7 @@
> . "$ASENV_CONF_LOCATION/asenv.conf"
> fi
>
> -INSTANCE_ROOT=%%%INSTANCE_ROOT%%%
> +INSTANCE_ROOT="%%%INSTANCE_ROOT%%%"
>
> JAVA_HOME="$AS_JAVA"
> # Make sure prerequisite environment variables are set
>
>
> Index: admin-ee/templates/ee80/stopserv.tomcat.template
> ===================================================================
> RCS file:
> /cvs/glassfish/admin-ee/templates/ee80/stopserv.tomcat.template,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 stopserv.tomcat.template
> --- admin-ee/templates/ee80/stopserv.tomcat.template 8 Aug 2006
> 19:48:40 -0000 1.1.1.1
> +++ admin-ee/templates/ee80/stopserv.tomcat.template 20 Sep 2006
> 19:19:06 -0000
> @@ -8,7 +8,7 @@
>
> # Resolve links - $0 may be a softlink
> PRG="$0"
> -ASENV_CONF_LOCATION=%%%CONFIG_HOME%%%
> +ASENV_CONF_LOCATION="%%%CONFIG_HOME%%%"
>
> SERVER_NAME=%%%SERVER_NAME%%%
>
> @@ -29,7 +29,7 @@
> . "$ASENV_CONF_LOCATION/asenv.conf"
> fi
>
> -INSTANCE_ROOT=%%%INSTANCE_ROOT%%%
> +INSTANCE_ROOT="%%%INSTANCE_ROOT%%%"
>
> JAVA_HOME="$AS_JAVA"
> # Make sure prerequisite environment variables are set
>
>
> *Description:*
>
> KeystoreManager.java:
> Modified code to handle space in the directory path.
>
> startserv.tomcat.template, stopserv.tomcat.template:
> added quotes for ASENV_CONF_LOCATION and INSTANCE_ROOT.
>
> Thanks,
> Jane
>
>
>
>
>
>