admin@glassfish.java.net

Re: CODE REVIEW for GlassFish 9.1ur1 IT 3754

From: Jane Young <Jane.Young_at_Sun.COM>
Date: Mon, 29 Oct 2007 10:31:54 -0700

Hi Byron,

Thanks for the review.
(1) I will use getCanonicalPath().
(2) The problem with changing the code to:
    if(!f.isFile())
      // ERROR OUT
is that the command error out if the file is on the server side but not
on client side. The resource xml file will need to be on both server
and client machine or that "asadmin" is executed on the server machine.
I'm going to keep the logic where if the file exists on the client side,
then I'll send the canonical path, if not then send the exact operand
that is entered by the user.

Jane


Byron Nevins wrote:

> Pedantic FYI:
>
> 12:00 PM is *not* noon. In fact there is no such thing as 12:00 PM.
> PM means -- "after noon". The instant of noon itself is neither AM or
> PM. The same applies to midnight.
>
>
> ---------
>
> I have 2 issues:
>
> 1) getAbsolutePath and getAbsoluteFile both, IMHO, suck. I always use
> Canonical instead like so:
>
> try { path = f.getCanonicalPath(); }
> catch(IOException ioe) { path = f.getAbsolutePath(); }
>
> Why? Say you are in /a/b/c and the filename is given as "./foo.xml"
> getAbsolutePath returns: "/a/b/c/./foo.xml"
> getCanonicalPath returns "/a/b/c/foo.xml"
>
>
> I.e. getCanonical guarantees one and only one name for a file, while
> there are many possible absolute paths.
> I personally just always use Canonical
>
>
> ----------------------
>
> 2) I would make it
>
> * pickier (only allow xml files that are locally accessible)
> * simpler. Don't do the isAbsolute() check
>
> thus:
>
> File f = new File(getOperands().get(0));
> try
> {
> f = f.getCanonicalFile();
> }
> catch(IOException ioe)
> {
> f = f.getAbsoluteFile();
> }
>
> if(!f.isFile())
> // ERROR OUT
>
> params[0] = f.getPath();
>
> Jane Young wrote:
>
>> Hi,
>>
>> Please review by 12:00pm (noon), Monday, Oct 29, 2007.
>>
>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=3754
>>
>> The fix is to check if the xml-file is in absolute path. If not,
>> then check if the file exists on the client side (asadmin) then
>> convert the relative path to absolute path and send it to the server
>> side.
>>
>> Here's the diff:
>>
>> +++ AddResourcesCommand.java Fri Oct 26 15:41:12 2007
>> @@ -45,8 +45,11 @@
>> // jdk imports
>> import java.util.ArrayList;
>> import java.util.Iterator;
>> +import java.io.File;
>>
>> -public class AddResourcesCommand extends GenericCommand{
>> +public class AddResourcesCommand extends GenericCommand
>> +{
>> + private final static String TARGET_OPTION = "target";
>>
>> /**
>> @@ -56,14 +59,25 @@
>> public void runCommand()throws CommandException,
>> CommandValidationException{
>> if (!validateOptions())
>> throw new CommandValidationException("Validation failed");
>> +
>> + String sXMLFile = (String) getOperands().get(0);
>> + final File fXMLFile = new File(sXMLFile);
>> - String objectName = getObjectName();
>> - Object[] params = getParamsInfo();
>> - String operationName = getOperationName();
>> - String[] types = getTypesInfo(); + //if
>> file exists on client side and not absolute path then send the
>> + //absolute path to the server side.
>> + if (fXMLFile.isFile() && !fXMLFile.isAbsolute()) {
>> + sXMLFile = fXMLFile.getAbsolutePath();
>> + }
>> + Object[] params = new Object[2];
>> + params[0] = sXMLFile;
>> + params[1] = getOption(TARGET_OPTION);
>> +
>> + final String objectName = getObjectName();
>> + final String operationName = getOperationName();
>> + final String[] types = getTypesInfo();
>> - MBeanServerConnection mbsc =
>> getMBeanServerConnection(getHost(), getPort(),
>> - getUser(), getPassword());
>> + MBeanServerConnection mbsc =
>> getMBeanServerConnection(getHost(), getPort(),
>> +
>> getUser(),
>> getPassword());
>> try {
>> ArrayList returnValue =(ArrayList) mbsc.invoke(new
>> ObjectName(objectName),
>> operationName, params,
>> types);
>>
>>
>> Thanks!
>> Jane
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
>--
>Byron Nevins Work 408-276-4089, Home 650-359-1290, Cell 650-784-4123 - Sun Microsystems, Inc.
>