admin@glassfish.java.net

Re: CODE REVIEW for GlassFish 9.1ur1 IT 3754

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Fri, 26 Oct 2007 20:19:05 -0700

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.