Byron,
I take that as you are okay with the changes to the template files.
Here's the diff of KeystoreManager.java with your suggested change:
Index:
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
---
servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java
25 Dec 2005 03:44:02 -0000 1.4
+++
servermgmt/src/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java
21 Sep 2006 01:42:38 -0000
@@ -45,6 +45,10 @@
import java.io.File;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+
/**
* @author kebbs
*/
@@ -427,8 +431,18 @@
protected void chmod(String args, File file) throws IOException
{
if (OS.isUNIX()) {
- Runtime.getRuntime().exec("/bin/chmod " + args + " " +
- file.getAbsolutePath());
+ //args and file should never be null.
+ //throw IOException since I don't want to change the
signature of this api.
+ if (args == null || file == null) throw new
IOException(_strMgr.getString("nullArg"));
+
+ // " ++" regular expression for 1 or more spaces
+ final String[] argsString = args.split(" ++");
+ List<String> cmdList = new ArrayList<String>();
+ cmdList.add("/bin/chmod");
+ for (String arg : argsString)
+ cmdList.add(arg);
+ cmdList.add(file.getAbsolutePath());
+ new ProcessBuilder(cmdList).start();
}
}
}
Thanks for reviewing!
Jane
Byron Nevins wrote:
> Oh -- I didn't notice that it was a UNIX template. Sorry for the
> annoying review I gave.
>
> Also, since you have the hood open anyways, java.lang.ProcessBuilder
> is a cool replacement for Runtime.getRuntime()
>
>
> Jane Young-Lau wrote:
>
>> Hi Byron,
>>
>> Thanks for the review.
>> I'll change it to use Runtime.getRuntime().exec(Sting[]).
>> The changes that I made will also work but your suggestion is better.
>>
>> The script change (startserv.Tomcat.template and
>> stopserv.Tomcat.template *NOT* startserv.Tomcat.bat.template and
>> stopserv.Tomcat.bat.template) is for Unix, not for Windows. I've
>> tested the change on Unix and Mac. Since the bug is reported on Mac
>> and the same script is used on both Unix and Mac that is why I tested
>> this on both OSs except for Windows.
>>
>> In Unix it's safe to declare:
>> ASENV_CONF_LOCATION="/export/a b/glassfish"
>>
>> . "$ASENV_CONF_LOCATION/asenv.conf"
>>
>> Thanks,
>>
>> Jane
>>
>>
>>
>>
>> Byron Nevins wrote:
>>
>>> 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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>