[RFC][icedtea-web]: Fix for PR1040, PR1041, PR1042 w/ reproducers
Adam Domurad
adomurad at redhat.com
Thu Jun 28 06:56:44 PDT 2012
> Doh, I didn't know it was possible to call a constructor from another
> constructor.
NP, I had to check the icedtea-web source for an example of this for the
syntax, and didn't realize it was possible until recently either.
Assuming your reproducers are functioning as expected, this patch is
good now.
OK for HEAD, but hold off until your reproducers are in HEAD (next on my
todo list). Also please see one last comment inline, you can change this
without posting a new patch.
> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> @@ -183,6 +183,9 @@
> * */
> private boolean foundMainJar= false;
>
> + /** Name of the application's main class */
> + private String mainClass = null;
> +
> /**
> * Variable to track how many times this loader is in use
> */
> @@ -194,6 +197,16 @@
> * @param file the JNLP file
> */
> protected JNLPClassLoader(JNLPFile file, UpdatePolicy policy)
> throws LaunchException {
> + this(file,policy,null);
> + }
> +
> + /**
> + * Create a new JNLPClassLoader from the specified file.
> + *
> + * @param file the JNLP file
> + * @param name of the application's main class
> + */
> + protected JNLPClassLoader(JNLPFile file, UpdatePolicy policy,
> String mainName) throws LaunchException {
> super(new URL[0], JNLPClassLoader.class.getClassLoader());
>
> if (JNLPRuntime.isDebug())
> @@ -203,6 +216,9 @@
> this.updatePolicy = policy;
> this.resources = file.getResources();
>
> + if (mainName != null)
The null guard check here is not necessary, as it does not add anything
to the behaviour. Removing this 'if' without submitting a new patch is
fine.
> + this.mainClass = mainName;
> +
> // initialize extensions
> initializeExtensions();
>
> @@ -309,6 +325,17 @@
> * @param policy the update policy to use when downloading
> resources
> */
> public static JNLPClassLoader getInstance(JNLPFile file,
> UpdatePolicy policy) throws LaunchException {
> + return getInstance(file, policy, null);
> + }
> +
> + /**
> + * Returns a JNLP classloader for the specified JNLP file.
> + *
> + * @param file the file to load classes for
> + * @param policy the update policy to use when downloading
> resources
> + * @param mainName Overrides the main class name of the
> application
> + */
> + public static JNLPClassLoader getInstance(JNLPFile file,
> UpdatePolicy policy, String mainName) throws LaunchException {
> JNLPClassLoader baseLoader = null;
> JNLPClassLoader loader = null;
> String uniqueKey = file.getUniqueKey();
> @@ -325,7 +352,7 @@
> (file.isApplication() &&
> !
> baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation()))) {
>
> - loader = new JNLPClassLoader(file, policy);
> + loader = new JNLPClassLoader(file, policy, mainName);
>
> // New loader init may have caused extentions to
> create a
> // loader for this unique key. Check.
> @@ -343,14 +370,14 @@
> // loader is now current + ext. But we also need to
> think of
> // the baseLoader
> if (baseLoader != null && baseLoader != loader) {
> - loader.merge(baseLoader);
> + loader.merge(baseLoader);
> }
>
> } else {
> // if key is same and locations match, this is the
> loader we want
> if (!file.isApplication()) {
> // If this is an applet, we do need to consider
> its loader
> - loader = new JNLPClassLoader(file, policy);
> + loader = new JNLPClassLoader(file, policy,
> mainName);
>
> if (baseLoader != null)
> baseLoader.merge(loader);
> @@ -380,13 +407,17 @@
> * @param location the file's location
> * @param version the file's version
> * @param policy the update policy to use when downloading
> resources
> + * @param mainName Overrides the main class name of the
> application
> */
> - public static JNLPClassLoader getInstance(URL location, String
> uniqueKey, Version version, UpdatePolicy policy)
> + public static JNLPClassLoader getInstance(URL location, String
> uniqueKey, Version version, UpdatePolicy policy, String mainName)
> throws IOException, ParseException, LaunchException {
> JNLPClassLoader loader = urlToLoader.get(uniqueKey);
>
> - if (loader == null || !
> location.equals(loader.getJNLPFile().getFileLocation()))
> - loader = getInstance(new JNLPFile(location, uniqueKey,
> version, false, policy), policy);
> + if (loader == null || !
> location.equals(loader.getJNLPFile().getFileLocation())) {
> + JNLPFile jnlpFile = new JNLPFile(location, uniqueKey,
> version, false, policy);
> +
> + loader = getInstance(jnlpFile, policy, mainName);
> + }
>
> return loader;
> }
> @@ -405,7 +436,20 @@
> for (int i = 0; i < ext.length; i++) {
> try {
> String uniqueKey = this.getJNLPFile().getUniqueKey();
> - JNLPClassLoader loader =
> getInstance(ext[i].getLocation(), uniqueKey, ext[i].getVersion(),
> updatePolicy);
> +
> + if (mainClass == null) {
> + Object obj = file.getLaunchInfo();
> +
> + if (obj instanceof ApplicationDesc) {
> + ApplicationDesc ad = (ApplicationDesc)
> file.getLaunchInfo();
> + mainClass = ad.getMainClass();
> + } else if (obj instanceof AppletDesc) {
> + AppletDesc ad = (AppletDesc)
> file.getLaunchInfo();
> + mainClass = ad.getMainClass();
> + }
> + }
> +
> + JNLPClassLoader loader =
> getInstance(ext[i].getLocation(), uniqueKey, ext[i].getVersion(),
> updatePolicy, mainClass);
> loaderList.add(loader);
> } catch (Exception ex) {
> ex.printStackTrace();
> @@ -444,8 +488,25 @@
> */
> void initializeResources() throws LaunchException {
> JARDesc jars[] = resources.getJARs();
> - if (jars == null || jars.length == 0)
> +
> + if (jars == null || jars.length == 0) {
> +
> + boolean allSigned = true;
> + for (int i = 1; i < loaders.length; i++) {
> + if (!loaders[i].getSigning()) {
> + allSigned = false;
> + break;
> + }
> + }
> +
> + if(allSigned)
> + signing = true;
> +
> + //Check if main jar is found within extensions
> + foundMainJar = foundMainJar || hasMainInExtensions();
> +
> return;
> + }
> /*
> if (jars == null || jars.length == 0) {
> throw new LaunchException(null, null, R("LSFatal"),
> @@ -507,6 +568,10 @@
> while (!foundMainJar && available != null &&
> available.size() != 0)
> addNextResource();
>
> + // If the jar with main class was not found, check
> extension
> + // jnlp's resources
> + foundMainJar = foundMainJar || hasMainInExtensions();
> +
> // If jar with main class was not found and there are
> no more
> // available jars, throw a LaunchException
> if (file.getLaunchInfo() instanceof AppletDesc ||
> @@ -593,17 +658,18 @@
> */
> private void checkForMain(List<JARDesc> jars) throws
> LaunchException {
>
> - Object obj = file.getLaunchInfo();
> - String mainClass;
> + if (mainClass == null) {
> + Object obj = file.getLaunchInfo();
>
> - if (obj instanceof ApplicationDesc) {
> - ApplicationDesc ad = (ApplicationDesc)
> file.getLaunchInfo();
> - mainClass = ad.getMainClass();
> - } else if (obj instanceof AppletDesc) {
> - AppletDesc ad = (AppletDesc) file.getLaunchInfo();
> - mainClass = ad.getMainClass();
> - } else
> - return;
> + if (obj instanceof ApplicationDesc) {
> + ApplicationDesc ad = (ApplicationDesc)
> file.getLaunchInfo();
> + mainClass = ad.getMainClass();
> + } else if (obj instanceof AppletDesc) {
> + AppletDesc ad = (AppletDesc) file.getLaunchInfo();
> + mainClass = ad.getMainClass();
> + } else
> + return;
> + }
>
> // The main class may be specified in the manifest
>
> @@ -698,6 +764,26 @@
> }
>
> /**
> + * Returns true if this loader has the main jar
> + */
> + public boolean hasMainJar() {
> + return this.foundMainJar;
> + }
> +
> + /**
> + * Returns true if extension loaders have the main jar
> + */
> + private boolean hasMainInExtensions() {
> + boolean foundMain = false;
> +
> + for (int i = 1; i < loaders.length && !foundMain; i++) {
> + foundMain = loaders[i].hasMainJar();
> + }
> +
> + return foundMain;
> + }
> +
> + /**
> * Is called by checkForMain() to check if the jar file is signed
> and if it
> * contains a signed JNLP file.
> *
More information about the distro-pkg-dev
mailing list