[rfc][icedtea-web] External main-class fix
Jiri Vanek
jvanek at redhat.com
Wed Jan 29 08:27:48 PST 2014
On 01/14/2014 08:30 PM, Andrew Azores wrote:
> On 01/08/2014 04:57 PM, Andrew Azores wrote:
>> Hi,
>>
>> Jiri found a flaw in the fix for PR1513, which allows applets to run when their main-class is not
>> in a JAR, but is still available to load from the codebase. The previous fix simply didn't throw a
>> LaunchException when the main-class could not be found in a JAR, and instead displayed a warning
>> about not all of the code being signed. However, this warning could still appear even if no
>> classes or JARs could be loaded at all! This patch causes the ClassLoader to not be so optimistic
>> about finding an external main-class - it actually checks for it first, and if it still can't be
>> found on the codebase after searching all JARs, then a LaunchException is thrown. If it can be
>> found, then the applet launch proceeds as normal at this step.
>>
>> Additionally, checkNotAllSignedWithUser can only be called once now in initializeResources.
>> Previously, it could be called both due to an external main-class as well as mixed signing states
>> of JARs in the applet - so if you had one signed JAR, one unsigned JAR, and an external
>> main-class, you would be prompted twice about the mixed signing, as well as about trusting the
>> signer of the signed JAR. Craziness. Now, the prompt about mixed signing should only appear once.
>>
>> ChangeLog:
>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: (initializeResources) perform search for
>> main-class when suspected to be external. Only show mixed code signing prompt once during
>> initialization. (checkNotAllSignedWithUser) refactor to remove local variable (promptUser)
>>
>> Thanks,
>>
>
> Updated patch, Jiri pointed out over IRC that the previous patch only worked for plugin applets and
> excluded JNLP. This makes it work for JNLP as well, and also cleans up how codebase loading is set
> up on a classloader. It's now done during initialization, rather than being handled after the fact
> by a static factory-ish method.
>
> Thanks,
>
> --
> Andrew A
>
>
ok. I fight with this (a lot),and looks (probably) ok. However. I will believe it did not broke
something afer one full round of tests.
Feel free to push.
Thanx for fix!
And sorry for long review delay :((
J.
>
>
> diff --git a/netx/net/sourceforge/jnlp/Launcher.java b/netx/net/sourceforge/jnlp/Launcher.java
> --- a/netx/net/sourceforge/jnlp/Launcher.java
> +++ b/netx/net/sourceforge/jnlp/Launcher.java
> @@ -694,13 +694,7 @@
> protected AppletInstance createApplet(JNLPFile file, boolean enableCodeBase, Container cont) throws LaunchException {
> AppletInstance appletInstance = null;
> try {
> - JNLPClassLoader loader = JNLPClassLoader.getInstance(file, updatePolicy);
> -
> - if (enableCodeBase) {
> - loader.enableCodeBase();
> - } else if (file.getResources().getJARs().length == 0) {
> - throw new ClassNotFoundException("Can't do a codebase look up and there are no jars. Failing sooner rather than later");
> - }
> + JNLPClassLoader loader = JNLPClassLoader.getInstance(file, updatePolicy, enableCodeBase);
>
> ThreadGroup group = Thread.currentThread().getThreadGroup();
>
> @@ -741,13 +735,7 @@
> */
> protected Applet createAppletObject(JNLPFile file, boolean enableCodeBase, Container cont) throws LaunchException {
> try {
> - JNLPClassLoader loader = JNLPClassLoader.getInstance(file, updatePolicy);
> -
> - if (enableCodeBase) {
> - loader.enableCodeBase();
> - } else if (file.getResources().getJARs().length == 0) {
> - throw new ClassNotFoundException("Can't do a codebase look up and there are no jars. Failing sooner rather than later");
> - }
> + JNLPClassLoader loader = JNLPClassLoader.getInstance(file, updatePolicy, enableCodeBase);
>
> String appletName = file.getApplet().getMainClass();
> Class<?> appletClass = loader.loadClass(appletName);
> 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
> @@ -227,7 +227,7 @@
> * @param file the JNLP file
> */
> protected JNLPClassLoader(JNLPFile file, UpdatePolicy policy) throws LaunchException {
> - this(file,policy,null);
> + this(file, policy, null, false);
> }
>
> /**
> @@ -237,7 +237,7 @@
> * @param policy the UpdatePolicy for this class loader
> * @param mainName name of the application's main class
> */
> - protected JNLPClassLoader(JNLPFile file, UpdatePolicy policy, String mainName) throws LaunchException {
> + protected JNLPClassLoader(JNLPFile file, UpdatePolicy policy, String mainName, boolean enableCodeBase) throws LaunchException {
> super(new URL[0], JNLPClassLoader.class.getClassLoader());
>
> OutputController.getLogger().log("New classloader: " + file.getFileLocation());
> @@ -263,6 +263,13 @@
>
> jcv = new JarCertVerifier(verifier);
>
> + if (enableCodeBase) {
> + addToCodeBaseLoader(file.getCodeBase());
> + } else if (file.getResources().getJARs().length == 0) {
> + throw new LaunchException(
> + new ClassNotFoundException("Can't do a codebase lookup and there are no jars. Failing sooner rather than later"));
> + }
> +
> // initialize extensions
> initializeExtensions();
>
> @@ -383,11 +390,12 @@
> * @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
> + * @param enableCodeBase if codebase lookups are allowed
> */
> - private static JNLPClassLoader createInstance(JNLPFile file, UpdatePolicy policy, String mainName) throws LaunchException {
> + private static JNLPClassLoader createInstance(JNLPFile file, UpdatePolicy policy, String mainName, boolean enableCodeBase) throws LaunchException {
> String uniqueKey = file.getUniqueKey();
> JNLPClassLoader baseLoader = uniqueKeyToLoader.get(uniqueKey);
> - JNLPClassLoader loader = new JNLPClassLoader(file, policy, mainName);
> + JNLPClassLoader loader = new JNLPClassLoader(file, policy, mainName, enableCodeBase);
>
> // If security level is 'high' or greater, we must check if the user allows unsigned applets
> // when the JNLPClassLoader is created. We do so here, because doing so in the constructor
> @@ -420,7 +428,7 @@
>
> /**
> * 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
> */
> @@ -433,9 +441,32 @@
> *
> * @param file the file to load classes for
> * @param policy the update policy to use when downloading resources
> + * @param enableCodeBase if codebase lookups are allowed
> + */
> + public static JNLPClassLoader getInstance(JNLPFile file, UpdatePolicy policy, boolean enableCodeBase) throws LaunchException {
> + return getInstance(file, policy, null, enableCodeBase);
> + }
> +
> + /**
> + * 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 {
> + return getInstance(file, policy, mainName, false);
> + }
> +
> + /**
> + * 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
> + * @param enableCodeBase if lookups are allowed
> + */
> + public static JNLPClassLoader getInstance(JNLPFile file, UpdatePolicy policy, String mainName, boolean enableCodeBase) throws LaunchException {
> JNLPClassLoader baseLoader = null;
> JNLPClassLoader loader = null;
> String uniqueKey = file.getUniqueKey();
> @@ -449,12 +480,12 @@
> (file.isApplication() &&
> !baseLoader.getJNLPFile().getFileLocation().equals(file.getFileLocation()))) {
>
> - loader = createInstance(file, policy, mainName);
> + loader = createInstance(file, policy, mainName, enableCodeBase);
> } 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, mainName);
> + loader = new JNLPClassLoader(file, policy, mainName, enableCodeBase);
>
> if (baseLoader != null)
> baseLoader.merge(loader);
> @@ -700,11 +731,28 @@
> // jnlp's resources
> foundMainJar = foundMainJar || hasMainInExtensions();
>
> - boolean externalMainClass = (file.getLaunchInfo() != null && !foundMainJar
> + boolean externalAppletMainClass = (file.getLaunchInfo() != null && !foundMainJar
> && (available == null || available.size() == 0));
>
> - if (!jcv.allJarsSigned() || externalMainClass) {
> - checkNotAllSignedWithUser(file);
> + // We do this check here simply to ensure that if there are no JARs at all,
> + // and also no main-class in the codebase (ie the applet doesn't really exist), we
> + // fail ASAP rather than continuing (and showing the NotAllSigned dialog for no applet)
> + if (externalAppletMainClass) {
> + if (codeBaseLoader != null) {
> + try {
> + codeBaseLoader.findClass(mainClass);
> + } catch (ClassNotFoundException extCnfe) {
> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, extCnfe);
> + throw new LaunchException(file, extCnfe, R("LSFatal"), R("LCInit"), R("LCantDetermineMainClass"), R("LCantDetermineMainClassInfo"));
> + }
> + } else {
> + throw new LaunchException(file, null, R("LSFatal"), R("LCInit"), R("LCantDetermineMainClass"), R("LCantDetermineMainClassInfo"));
> + }
> + }
> +
> + // If externalAppletMainClass is true and a LaunchException was not thrown above,
> + // then the main-class can be loaded from the applet codebase, but is obviously not signed
> + if (!jcv.allJarsSigned() || externalAppletMainClass) {
> signing = SigningState.PARTIAL;
> }
>
> @@ -784,8 +832,11 @@
> }
>
> if (containsSignedJar && containsUnsignedJar) {
> + signing = SigningState.PARTIAL;
> + }
> +
> + if (signing == SigningState.PARTIAL && JNLPRuntime.isVerifying()) {
> checkNotAllSignedWithUser(file);
> - signing = SigningState.PARTIAL;
> }
>
> activateJars(initialJars);
> @@ -1090,25 +1141,14 @@
> * @throws LaunchException if the user does not approve the prompt
> */
> private void checkNotAllSignedWithUser(JNLPFile file) throws LaunchException {
> - boolean promptUser = true;
> -
> if (JNLPRuntime.isTrustAll()) {
> - promptUser = false;
> + return;
> }
>
> - if (promptUser && !SecurityDialogs.showNotAllSignedWarningDialog(file)) {
> + if (!SecurityDialogs.showNotAllSignedWarningDialog(file)) {
> throw new LaunchException(file, null, R("LSFatal"), R("LCClient"), R("LSignedAppJarUsingUnsignedJar"), R("LSignedAppJarUsingUnsignedJarInfo"));
> }
> }
> - /**
> - * Add applet's codebase URL. This allows compatibility with
> - * applets that load resources from their codebase instead of
> - * through JARs, but can slow down resource loading. Resources
> - * loaded from the codebase are not cached.
> - */
> - public void enableCodeBase() {
> - addToCodeBaseLoader(file.getCodeBase());
> - }
>
> /**
> * Sets the JNLP app this group is for; can only be called once.
>
More information about the distro-pkg-dev
mailing list