[icedtea-web] RFC: handle exceptions during boot
Dr Andrew John Hughes
ahughes at redhat.com
Wed Mar 30 15:27:45 PDT 2011
On 17:35 Wed 30 Mar , Omair Majid wrote:
> On 03/30/2011 04:40 PM, Dr Andrew John Hughes wrote:
> > On 15:54 Wed 30 Mar , Omair Majid wrote:
> >> Hi,
> >>
> >> The attached patch makes the LaunchHandler handle exceptions that happen
> >> during the parsing of the JNLP file.
> >>
> >> Most of the changes are simply moving things from Boot to Launcher.
> >> Launcher, which has an associated LaunchHandler, is in a much better
> >> position to use the LaunchHandler to inform the user about exceptions
> >> that happen while parsing.
> >>
> >> I am not too happy about the name of the class InformationToMerge. Any
> >> ideas?
> >>
> >
> > It suggests to me it doesn't warrant being an object.
> >
> > Any particular reason you need a class at all and didn't just use an
> > array or a map, given they all have the same type?
> >
> > So with a Map<String[]>, extra.getParameters() would become extra.get("parameters")
> > and there's room for further expansion with minimal effort.
> >
>
> Good point. Fixed in the attached patch.
>
> Any other suggestions or comments?
>
I don't see anything obvious, but it's a little hard to read with the code movement.
Is most of the new code just moved without change?
If possible, it would be preferable to do the movement separately.
> Cheers,
> Omair
> diff -r 77ac95466baa netx/net/sourceforge/jnlp/Launcher.java
> --- a/netx/net/sourceforge/jnlp/Launcher.java Wed Mar 30 11:47:41 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/Launcher.java Wed Mar 30 17:34:47 2011 -0400
> @@ -27,6 +27,7 @@
> import java.net.UnknownHostException;
> import java.util.LinkedList;
> import java.util.List;
> +import java.util.Map;
> import java.util.jar.JarFile;
>
> import net.sourceforge.jnlp.cache.CacheUtil;
> @@ -76,6 +77,9 @@
> /** If the application should call System.exit on fatal errors */
> private boolean exitOnFailure = true;
>
> + private ParserSettings parserSettings = new ParserSettings();
> +
> + private Map<String, String[]> extra = null;
>
> /**
> * Create a launcher with the runtime's default update policy
> @@ -164,6 +168,18 @@
> return this.context;
> }
>
> + /** Set the parser settings to use when the Launcher initiates parsing of
> + * a JNLP file.
> + * @param settings
> + */
> + public void setParserSettings(ParserSettings settings) {
> + parserSettings = settings;
> + }
> +
> + public void setInformationToMerge(Map<String, String[]> input) {
> + this.extra = input;
> + }
> +
> /**
> * Launches a JNLP file by calling the launch method for the
> * appropriate file type. The application will be started in
> @@ -189,6 +205,8 @@
> public ApplicationInstance launch(JNLPFile file, Container cont) throws LaunchException {
> TgThread tg;
>
> + mergeExtraInformation(file, extra);
> +
> JNLPRuntime.markNetxRunning();
>
> //First checks whether offline-allowed tag is specified inside the jnlp
> @@ -246,6 +264,106 @@
>
> /**
> * Launches a JNLP file by calling the launch method for the
> + * appropriate file type.
> + *
> + * @param location the URL of the JNLP file to launch
> + * @param fromSource if true, the JNLP file will be re-read from the source
> + * location to get the pristine version
> + * @throws LaunchException if there was an exception
> + * @return the application instance
> + */
> + public ApplicationInstance launch(URL location, boolean fromSource) throws LaunchException {
> + return launch(fromUrl(location, fromSource));
> + }
> +
> + /**
> + * Merges extra information into the jnlp file
> + *
> + * @param file the JNLPFile
> + * @param extra extra information to merge into the JNLP file
> + * @throws LaunchException if an exception occurs while extracting
> + * extra information
> + */
> + private void mergeExtraInformation(JNLPFile file, Map<String, String[]> extra) throws LaunchException {
> + if (extra == null) {
> + return;
> + }
> +
> + String[] properties = extra.get("properties");
> + if (properties != null) {
> + addProperties(file, properties);
> + }
> +
> + String[] arguments = extra.get("arguments");
> + if (arguments != null && file.isApplication()) {
> + addArguments(file, arguments);
> + }
> +
> + String[] parameters = extra.get("parameters");
> + if (parameters != null && file.isApplet()) {
> + addParameters(file, parameters);
> + }
> + }
> +
> + /**
> + * Add the properties to the JNLP file.
> + * @throws LaunchException if an exception occurs while extracting
> + * extra information
> + */
> + private void addProperties(JNLPFile file, String[] props) throws LaunchException {
> + ResourcesDesc resources = file.getResources();
> +
> + for (int i = 0; i < props.length; i++) {
> + // allows empty property, not sure about validity of that.
> + int equals = props[i].indexOf("=");
> + if (equals == -1) {
> + throw launchError(new LaunchException(R("BBadProp", props[i])));
> + }
> +
> + String key = props[i].substring(0, equals);
> + String value = props[i].substring(equals + 1, props[i].length());
> +
> + resources.addResource(new PropertyDesc(key, value));
> + }
> + }
> +
> + /**
> + * Add the params to the JNLP file; only call if file is
> + * actually an applet file.
> + * @throws LaunchException if an exception occurs while extracting
> + * extra information
> + */
> + private void addParameters(JNLPFile file, String[] params) throws LaunchException {
> + AppletDesc applet = file.getApplet();
> +
> + for (int i = 0; i < params.length; i++) {
> + // allows empty param, not sure about validity of that.
> + int equals = params[i].indexOf("=");
> + if (equals == -1) {
> + throw launchError(new LaunchException(R("BBadParam", params[i])));
> + }
> +
> + String name = params[i].substring(0, equals);
> + String value = params[i].substring(equals + 1, params[i].length());
> +
> + applet.addParameter(name, value);
> + }
> + }
> +
> + /**
> + * Add the arguments to the JNLP file; only call if file is
> + * actually an application (not installer).
> + */
> + private void addArguments(JNLPFile file, String[] args) {
> + ApplicationDesc app = file.getApplication();
> +
> + for (int i = 0; i < args.length; i++) {
> + app.addArgument(args[i]);
> + }
> + }
> +
> + /**
> + * Launches a JNLP file by calling the launch method for the
> * appropriate file type in a different thread.
> *
> * @param file the JNLP file to launch
> @@ -345,6 +463,32 @@
> /**
> * Returns the JNLPFile for the URL, with error handling.
> */
> + private JNLPFile fromUrl(URL location, boolean fromSource) throws LaunchException {
> + try {
> + JNLPFile file = null;
> +
> + file = new JNLPFile(location, parserSettings.isStrict());
> +
> + if (fromSource) {
> + // Launches the jnlp file where this file originated.
> + if (file.getSourceLocation() != null) {
> + file = new JNLPFile(file.getSourceLocation(), parserSettings.isStrict());
> + }
> + }
> + return file;
> + } catch (Exception ex) {
> + if (ex instanceof LaunchException)
> + throw (LaunchException) ex; // already sent to handler when first thrown
> + else
> + // IO and Parse
> + throw launchError(new LaunchException(null, ex, R("LSFatal"), R("LCReadError"), R("LCantRead"), R("LCantReadInfo")));
> + }
> + }
> +
> + /**
> + * Returns the JNLPFile for the URL, with error handling.
> + */
> + @Deprecated
> private JNLPFile toFile(URL location) throws LaunchException {
> try {
> JNLPFile file = null;
> diff -r 77ac95466baa netx/net/sourceforge/jnlp/ParserSettings.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/ParserSettings.java Wed Mar 30 17:34:47 2011 -0400
> @@ -0,0 +1,23 @@
> +package net.sourceforge.jnlp;
> +
> +/**
> + * Contains settings to be used by the Parser while parsing JNLP files.
> + *
> + * Immutable and therefore thread-safe.
> + */
> +public class ParserSettings {
> +
> + private final boolean isStrict;
> +
> + public ParserSettings() {
> + isStrict = false;
> + }
> +
> + public ParserSettings(boolean strict) {
> + isStrict = strict;
> + }
> +
> + public boolean isStrict() {
> + return isStrict;
> + }
> +}
> diff -r 77ac95466baa netx/net/sourceforge/jnlp/resources/Messages.properties
> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties Wed Mar 30 11:47:41 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties Wed Mar 30 17:34:47 2011 -0400
> @@ -129,8 +129,6 @@
> BNeedsFile=Must specify a .jnlp file
> RNoAboutJnlp=Unable to find about.jnlp
> BFileLoc=JNLP file location
> -BArgNA=Arguments not used for this type of JNLP file (ignored).
> -BParamNA=Parameters not used for this type of JNLP file (ignored).
> BBadProp=Incorrect property format {0} (should be key=value)
> BBadParam=Incorrect parameter format {0} (should be name=value)
> BNoDir=Directory {0} does not exist.
> diff -r 77ac95466baa netx/net/sourceforge/jnlp/runtime/Boot.java
> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java Wed Mar 30 11:47:41 2011 -0400
> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java Wed Mar 30 17:34:47 2011 -0400
> @@ -19,23 +19,18 @@
> import static net.sourceforge.jnlp.runtime.Translator.R;
>
> import java.io.File;
> -import java.io.IOException;
> -import java.net.MalformedURLException;
> import java.net.URL;
> import java.security.AccessController;
> import java.security.PrivilegedAction;
> import java.util.ArrayList;
> import java.util.Arrays;
> +import java.util.HashMap;
> import java.util.List;
> +import java.util.Map;
>
> -import net.sourceforge.jnlp.AppletDesc;
> -import net.sourceforge.jnlp.ApplicationDesc;
> -import net.sourceforge.jnlp.JNLPFile;
> import net.sourceforge.jnlp.LaunchException;
> import net.sourceforge.jnlp.Launcher;
> -import net.sourceforge.jnlp.ParseException;
> -import net.sourceforge.jnlp.PropertyDesc;
> -import net.sourceforge.jnlp.ResourcesDesc;
> +import net.sourceforge.jnlp.ParserSettings;
> import net.sourceforge.jnlp.cache.CacheUtil;
> import net.sourceforge.jnlp.cache.UpdatePolicy;
> import net.sourceforge.jnlp.security.viewer.CertificateViewer;
> @@ -189,8 +184,19 @@
> return null;
> }
>
> + Map<String, String[]> extra = new HashMap<String, String[]>();
> + extra.put("arguments", getOptions("-arg"));
> + extra.put("parameters", getOptions("-param"));
> + extra.put("properties", getOptions("-property"));
> +
> + boolean strict = (null != getOption("-strict"));
> + ParserSettings settings = new ParserSettings(strict);
> +
> try {
> - new Launcher(false).launch(getFile());
> + Launcher launcher = new Launcher(false);
> + launcher.setParserSettings(settings);
> + launcher.setInformationToMerge(extra);
> + launcher.launch(getFileLocation(), true);
> } catch (LaunchException ex) {
> // default handler prints this
> } catch (Exception ex) {
> @@ -236,10 +242,10 @@
> }
>
> /**
> - * Returns the file to open; does not return if no file was
> - * specified.
> + * Returns the url of file to open; does not return if no file was
> + * specified, or if the file location was invalid.
> */
> - private static JNLPFile getFile() throws ParseException, MalformedURLException, IOException {
> + private static URL getFileLocation() {
>
> String location = getJNLPFile();
>
> @@ -274,89 +280,7 @@
> e.printStackTrace();
> }
>
> - boolean strict = (null != getOption("-strict"));
> -
> - JNLPFile file = new JNLPFile(url, strict);
> -
> - // Launches the jnlp file where this file originated.
> - if (file.getSourceLocation() != null) {
> - file = new JNLPFile(file.getSourceLocation(), strict);
> - }
> -
> - // add in extra params from command line
> - addProperties(file);
> -
> - if (file.isApplet())
> - addParameters(file);
> -
> - if (file.isApplication())
> - addArguments(file);
> -
> - if (JNLPRuntime.isDebug()) {
> - if (getOption("-arg") != null)
> - if (file.isInstaller() || file.isApplet())
> - System.out.println(R("BArgsNA"));
> -
> - if (getOption("-param") != null)
> - if (file.isApplication())
> - System.out.println(R("BParamNA"));
> - }
> -
> - return file;
> - }
> -
> - /**
> - * Add the properties to the JNLP file.
> - */
> - private static void addProperties(JNLPFile file) {
> - String props[] = getOptions("-property");
> - ResourcesDesc resources = file.getResources();
> -
> - for (int i = 0; i < props.length; i++) {
> - // allows empty property, not sure about validity of that.
> - int equals = props[i].indexOf("=");
> - if (equals == -1)
> - fatalError(R("BBadProp", props[i]));
> -
> - String key = props[i].substring(0, equals);
> - String value = props[i].substring(equals + 1, props[i].length());
> -
> - resources.addResource(new PropertyDesc(key, value));
> - }
> - }
> -
> - /**
> - * Add the params to the JNLP file; only call if file is
> - * actually an applet file.
> - */
> - private static void addParameters(JNLPFile file) {
> - String params[] = getOptions("-param");
> - AppletDesc applet = file.getApplet();
> -
> - for (int i = 0; i < params.length; i++) {
> - // allows empty param, not sure about validity of that.
> - int equals = params[i].indexOf("=");
> - if (equals == -1)
> - fatalError(R("BBadParam", params[i]));
> -
> - String name = params[i].substring(0, equals);
> - String value = params[i].substring(equals + 1, params[i].length());
> -
> - applet.addParameter(name, value);
> - }
> - }
> -
> - /**
> - * Add the arguments to the JNLP file; only call if file is
> - * actually an application (not installer).
> - */
> - private static void addArguments(JNLPFile file) {
> - String args[] = getOptions("-arg"); // FYI args also global variable
> - ApplicationDesc app = file.getApplication();
> -
> - for (int i = 0; i < args.length; i++) {
> - app.addArgument(args[i]);
> - }
> + return url;
> }
>
> /**
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list