[rfc][icedtea-web] missing jnlpfile and securtydelegate for TemporaryPermissionsButton calls

[Andrew Azores]

Hi,

Still on PTO here and preparing to move back into my parents' place 
while I go back to finish school, so I haven't really invested any real 
time into this bug. But...

On 08/27/2014 05:25 AM, Jiri Vanek wrote:
> Hi!
>
> During testing of https probing, helpcrypto found and interesting issue
> in head.
>
> Under some circumstances calls to  TemporaryPermissionsButton have
> jnlpfile and securitydelegate null.

The SecurityDelegate can be instantiated much earlier in JNLPClassLoader 
if need be. It can even be the first thing done after calling the super 
constructor AFAIK. Since the JNLPClassLoader is also what eventually 
causes the security dialog to appear with the Temporary Permissions 
button on it, this will probably resolve the null securityDelegate 
reference part.

>
> Two things troubles me:
>   - both those values are needed only when "tmeporarypermissions" are
> clicked, so they should not affect startup of ITW
>   - how come that they are null? I blame
> http://icedtea.classpath.org/hg/icedtea-web/annotate/d87ee4e6e81a/netx/net/sourceforge/jnlp/security/dialogs/TemporaryPermissionsButton.java#78
>
> (looking into  Looking to this,
> http://icedtea.classpath.org/hg/icedtea-web/file/b4363c984e1b/netx/net/sourceforge/jnlp/security/dialogs/TemporaryPermissionsButton.java#l79
> )

For the null jnlpFile - I can't think off the top of my head what is 
causing that. It could just be again that the dialog is being shown 
before a field has been assigned a value, as it is with the 
securityDelegate. If it's simply an ordering issue like this then that 
isn't *too* concerning. If we truly just do not have a reference to any 
JNLPFile or PluginBridge for this applet then there's something 
seriously wrong.

>
>
> Atatched is the nasty workarorund and  here is snippet of log
>
> Securitymanager=net.sourceforge.jnlp.runtime.JNLPSecurityManager at 19f17b1
> Registering priority for string reference 2
> Registering priority for reference 2
> Returning value:0
> Setting value:0
> Setting value:null
> dummy string null, null, javax.swing.JButton[,0,0,0x0,inva...snip...
> plugin_in_pipe_callback return
> plugin_send_message_to_appletviewer return
>    PIPE: plugin wrote(?): plugin PluginProxyInfo reference 1 DIRECT
> plugin_send_message_to_appletviewer
> Proxy info: plugin PluginProxyInfo reference 1 DIRECT
>
>
>
> Without the workarround  itw dies:
>
> ITNP_SetWindow return
> ITNP_SetWindow: window already exists.
> ITNP_SetWindow
>      at java.lang.Thread.run(Thread.java:745)
>      at
> net.sourceforge.jnlp.security.SecurityDialogMessageHandler.run(SecurityDialogMessageHandler.java:81)
>
>      at
> net.sourceforge.jnlp.security.SecurityDialogMessageHandler.handleMessage(SecurityDialogMessageHandler.java:102)
>
>      at net.sourceforge.jnlp.security.SecurityDialog.
> (SecurityDialog.java:129)
>      at
> net.sourceforge.jnlp.security.SecurityDialog.initDialog(SecurityDialog.java:255)
>
>      at
> net.sourceforge.jnlp.security.SecurityDialog.installPanel(SecurityDialog.java:307)
>
>      at net.sourceforge.jnlp.security.dialogs.CertWarningPane.
> (CertWarningPane.java:116)
>      at
> net.sourceforge.jnlp.security.dialogs.CertWarningPane.addComponents(CertWarningPane.java:124)
>
>      at
> net.sourceforge.jnlp.security.dialogs.CertWarningPane.addButtons(CertWarningPane.java:242)
>
>      at
> net.sourceforge.jnlp.security.dialogs.TemporaryPermissionsButton.
> (TemporaryPermissionsButton.java:79)
>      at java.util.Objects.requireNonNull(Objects.java:201)
> Exception in thread "NetxSecurityThread" java.lang.NullPointerException
> plugin_in_pipe_callback return
> plugin_send_message_to_appletviewer return
>    PIPE: plugin wrote(?): plugin PluginProxyInfo reference 1 DIRECT
> plugin_send_message_to_appletviewer
> Proxy info: plugin PluginProxyInfo reference 1 DIRECT
>
>
>
> I think the AssertNotNUll are really really invasive here. But at least
> they cought probably more serious issue - to early call to the dialogue
> with tmppermissions button....
>
>
> J.

Yup. The PolicyEditor can't be sensibly launched if the jnlpFile is null 
(your nasty workaround really is nasty - assigning the new permissions 
to all applets rather than only ones on the current applet's codebase!), 
and the securityDelegate is needed if this button is to function at all. 
So there's no sensible thing to do if either of those fields come up 
null. Perhaps rather than outright failing to launch ITW we could have 
the button indicate a failure somehow and have the dialog simply not 
install the button, however. Just another option for Lukasz or whomever 
to look into. If this still hasn't been fixed by the time I come off PTO 
then I'll gladly take ownership of the bug.

Thanks,

-- 
Andrew Azores