[RFC][icedtea-web:netx]: handle error code returning functions
Omair Majid
omajid at redhat.com
Fri Mar 11 11:44:33 PST 2011
On 03/11/2011 01:46 PM, Denis Lila wrote:
>> > Not sure if that's entirely correct. A user may accidentally create a
>> > file with that name. exists() will return true even though it's not a
>> > directory. For robustness's sake, I think we should always explicitly
>> > check for a directory.
> Right, I see.
>
>> > For ease of reading, may I recommend overloading this? If you create
>> > another method createParentDir(File f) which delegates to this by
>> > calling createParentDir(f, null), it might make other parts of the
>> > code
>> > easier to read.
>> >
>> > Also, please add JavaDocs.
> Done.
>
> Is it ok now?
>
Looks good now.
Cheers,
Omair
>
> diff -r 04459b8baed4 ChangeLog
> --- a/ChangeLog Fri Mar 11 09:48:55 2011 +0100
> +++ b/ChangeLog Fri Mar 11 12:50:11 2011 -0500
> @@ -1,3 +1,26 @@
> +2011-03-11 Denis Lila<dlila at redhat.com>
> +
> + * netx/net/sourceforge/jnlp/Launcher.java
> + (markNetxRunning): Throw exception if directories can't be created.
> + * netx/net/sourceforge/jnlp/cache/CacheDirectory.java
> + (cleanParent): Print error message if file can't be deleted.
> + * netx/net/sourceforge/jnlp/cache/CacheUtil.java
> + (getCacheFile): Throw exception if directories can't be created.
> + * netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java
> + (save): Throw exception if directories can't be created.
> + * netx/net/sourceforge/jnlp/controlpanel/CachePane.java
> + (createButtonPanel): Print error message if file can't be deleted.
> + * netx/net/sourceforge/jnlp/resources/Messages.properties
> + Added messages.
> + * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
> + (initializeStreams): Throw exception if directories can't be created.
> + * netx/net/sourceforge/jnlp/services/XPersistenceService.java
> + (create, get): Throw exception if directories can't be created.
> + (delete): Print error message if file can't be deleted.
> + * netx/net/sourceforge/jnlp/util/FileUtils.java
> + (createRestrictedFile): Throw exception if file permissions can't be
> + changed.
> +
> 2011-03-10 Mark Wielaard<mark at klomp.org>
>
> * tests/netx/pac/pac-funcs-test.js (testIsResolvable):
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/Launcher.java
> --- a/netx/net/sourceforge/jnlp/Launcher.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/Launcher.java Fri Mar 11 12:50:11 2011 -0500
> @@ -709,7 +709,7 @@
> File netxRunningFile = new File(JNLPRuntime.getConfiguration()
> .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE));
> if (!netxRunningFile.exists()) {
> - netxRunningFile.getParentFile().mkdirs();
> + FileUtils.createParentDir(netxRunningFile);
> FileUtils.createRestrictedFile(netxRunningFile, true);
> FileOutputStream fos = new FileOutputStream(netxRunningFile);
> try {
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/cache/CacheDirectory.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheDirectory.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/cache/CacheDirectory.java Fri Mar 11 12:50:11 2011 -0500
> @@ -39,6 +39,8 @@
> import java.io.File;
> import java.util.ArrayList;
>
> +import net.sourceforge.jnlp.util.FileUtils;
> +
> public class CacheDirectory {
> /**
> * Get the structure of directory for keeping track of the protocol and
> @@ -103,7 +105,7 @@
> if (parent.getParent() == null)
> return; // Don't delete the root.
> if (parent.getChildren().size() == 0) {
> - parent.getFile().delete();
> + FileUtils.deleteWithErrMesg(parent.getFile());
> parent.getParent().removeChild(parent);
> cleanParent(parent);
> }
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/cache/CacheUtil.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java Fri Mar 11 12:50:11 2011 -0500
> @@ -291,7 +291,7 @@
> String cacheDir = JNLPRuntime.getConfiguration()
> .getProperty(DeploymentConfiguration.KEY_USER_CACHE_DIR);
> File localFile = urlToPath(source, cacheDir);
> - localFile.getParentFile().mkdirs();
> + FileUtils.createParentDir(localFile);
>
> return localFile;
> } catch (Exception ex) {
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java
> --- a/netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java Fri Mar 11 12:50:11 2011 -0500
> @@ -38,6 +38,7 @@
> import javax.naming.ConfigurationException;
>
> import net.sourceforge.jnlp.runtime.JNLPRuntime;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> /**
> * Manages the various properties and configuration related to deployment.
> @@ -518,7 +519,7 @@
> }
> }
>
> - userPropertiesFile.getParentFile().mkdirs();
> + FileUtils.createParentDir(userPropertiesFile);
> OutputStream out = new BufferedOutputStream(new FileOutputStream(userPropertiesFile));
> try {
> toSave.store(out, DEPLOYMENT_COMMENT);
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/controlpanel/CachePane.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/CachePane.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/controlpanel/CachePane.java Fri Mar 11 12:50:11 2011 -0500
> @@ -43,6 +43,7 @@
> import net.sourceforge.jnlp.cache.DirectoryNode;
> import net.sourceforge.jnlp.config.DeploymentConfiguration;
> import net.sourceforge.jnlp.runtime.Translator;
> +import net.sourceforge.jnlp.util.FileUtils;
>
> public class CachePane extends JPanel {
>
> @@ -132,7 +133,7 @@
> DirectoryNode fileNode = ((DirectoryNode) cacheTable.getModel().getValueAt(modelRow, 0));
> if (fileNode.getFile().delete()) {
> fileNode.getParent().removeChild(fileNode);
> - fileNode.getInfoFile().delete();
> + FileUtils.deleteWithErrMesg(fileNode.getInfoFile());
> ((DefaultTableModel) cacheTable.getModel()).removeRow(modelRow);
> cacheTable.getSelectionModel().setSelectionInterval(row, row);
> CacheDirectory.cleanParent(fileNode);
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/resources/Messages.properties
> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties Fri Mar 11 12:50:11 2011 -0500
> @@ -135,6 +135,13 @@
> RExitTaken=Exit class already set and caller is not exit class.
> RCantReplaceSM=Changing the SecurityManager is not allowed.
> RCantCreateFile=Cant create file {0}
> +RCantDeleteFile=Cant delete file {0}
> +RRemoveRPermFailed=Removing read permission on file {0} failed
> +RRemoveWPermFailed=Removing write permissions on file {0} failed
> +RRemoveXPermFailed=Removing execute permissions on file {0} failed
> +RGetRPermFailed=Acquiring read permissions on file {0} failed
> +RGetWPermFailed=Acquiring write permissions on file {0} failed
> +RGetXPermFailed=Acquiring execute permissions on file {0} failed
> RCantCreateDir=Cant create directory {0}
> RCantRename=Cant rename {0} to {0}
> RDenyStopped=Stopped applications have no permissions.
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java Fri Mar 11 12:50:11 2011 -0500
> @@ -277,10 +277,10 @@
>
> try {
> File errFile = new File(logDir, JNLPRuntime.STDERR_FILE);
> - errFile.getParentFile().mkdirs();
> + FileUtils.createParentDir(errFile);
> FileUtils.createRestrictedFile(errFile, true);
> File outFile = new File(logDir, JNLPRuntime.STDOUT_FILE);
> - outFile.getParentFile().mkdirs();
> + FileUtils.createParentDir(outFile);
> FileUtils.createRestrictedFile(outFile, true);
>
> if (redirectStreams) {
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/services/XPersistenceService.java
> --- a/netx/net/sourceforge/jnlp/services/XPersistenceService.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/services/XPersistenceService.java Fri Mar 11 12:50:11 2011 -0500
> @@ -93,7 +93,8 @@
> checkLocation(location);
>
> File file = toCacheFile(location);
> - file.getParentFile().mkdirs();
> + FileUtils.createParentDir(file, "Persistence store for "
> + + location.toString());
>
> if (file.exists())
> throw new IOException("File already exists.");
> @@ -110,7 +111,7 @@
> public void delete(URL location) throws MalformedURLException, IOException {
> checkLocation(location);
>
> - toCacheFile(location).delete();
> + FileUtils.deleteWithErrMesg(toCacheFile(location), " tocache");
> }
>
> /**
> @@ -124,7 +125,8 @@
> if (!file.exists())
> throw new FileNotFoundException("Persistence store for "
> + location.toString() + " is not found.");
> - file.getParentFile().mkdirs();
> + FileUtils.createParentDir(file, "Persistence store for "
> + + location.toString());
>
> return (FileContents) ServiceUtil.createPrivilegedProxy(FileContents.class, new XFileContents(file));
> }
> diff -r 04459b8baed4 netx/net/sourceforge/jnlp/util/FileUtils.java
> --- a/netx/net/sourceforge/jnlp/util/FileUtils.java Fri Mar 11 09:48:55 2011 +0100
> +++ b/netx/net/sourceforge/jnlp/util/FileUtils.java Fri Mar 11 12:50:11 2011 -0500
> @@ -96,6 +96,59 @@
> }
>
> /**
> + * Tries to create the ancestor directories of file f. Throws
> + * an IOException if it can't be created (but not if it was
> + * already there).
> + * @param f
> + * @param eMsg - the message to use for the exception. null
> + * if the file name is to be used.
> + * @throws IOException if the directory can't be created and doesn't exist.
> + */
> + public static void createParentDir(File f, String eMsg) throws IOException {
> + File parent = f.getParentFile();
> + if (!parent.isDirectory()&& !parent.mkdirs()) {
> + throw new IOException(R("RCantCreateDir",
> + eMsg == null ? parent : eMsg));
> + }
> + }
> +
> + /**
> + * Tries to create the ancestor directories of file f. Throws
> + * an IOException if it can't be created (but not if it was
> + * already there).
> + * @param f
> + * @throws IOException if the directory can't be created and doesn't exist.
> + */
> + public static void createParentDir(File f) throws IOException {
> + createParentDir(f, null);
> + }
> +
> + /**
> + * Tries to delete file f. If the file exists but couldn't be deleted,
> + * print an error message to stderr with the file name, or eMsg if eMsg
> + * is not null.
> + * @param f the file to be deleted
> + * @param eMsg the message to print on failure (or null to print the
> + * the file name).
> + */
> + public static void deleteWithErrMesg(File f, String eMsg) {
> + if (f.exists()) {
> + if (!f.delete()) {
> + System.err.println(R("RCantDeleteFile", eMsg == null ? f : eMsg));
> + }
> + }
> + }
> +
> + /**
> + * Tries to delete file f. If the file exists but couldn't be deleted,
> + * print an error message to stderr with the file name.
> + * @param f the file to be deleted
> + */
> + public static void deleteWithErrMesg(File f) {
> + deleteWithErrMesg(f, null);
> + }
> +
> + /**
> * Creates a new file or directory with minimum permissions. The file is not
> * readable or writable by anyone other than the owner. If writeableByOnwer
> * is false, even the owner can not write to it. If isDir is true, then the
> @@ -120,21 +173,29 @@
> }
>
> // remove all permissions
> - tempFile.setExecutable(false, false);
> - tempFile.setReadable(false, false);
> - tempFile.setWritable(false, false);
> + if (!tempFile.setExecutable(false, false)) {
> + throw new IOException(R("RRemoveXPermFailed", tempFile));
> + }
> + if (!tempFile.setReadable(false, false)) {
> + throw new IOException(R("RRemoveRPermFailed", tempFile));
> + }
> + if (!tempFile.setWritable(false, false)) {
> + throw new IOException(R("RRemoveWPermFailed", tempFile));
> + }
>
> // allow owner to read
> - tempFile.setReadable(true, true);
> + if (!tempFile.setReadable(true, true)) {
> + throw new IOException(R("RGetRPermFailed", tempFile));
> + }
>
> // allow owner to write
> - if (writableByOwner) {
> - tempFile.setWritable(true, true);
> + if (writableByOwner&& !tempFile.setWritable(true, true)) {
> + throw new IOException(R("RGetWPermFailed", tempFile));
> }
>
> // allow owner to enter directories
> - if (isDir) {
> - tempFile.setExecutable(true, true);
> + if (isDir&& !tempFile.setExecutable(true, true)) {
> + throw new IOException(R("RGetXPermFailed", tempFile));
> }
>
> // rename this file. Unless the file is moved/renamed, any program that
More information about the distro-pkg-dev
mailing list