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

[Jiri Vanek]

On 08/28/2014 12:33 AM, Andrew Azores wrote:
> 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.
>

Andrew have made some investigations, and this is moreover conclusion
  - its a bug -  the tmp permissions dialogue is shared among javaws and applets, and in applets 
lifecycle can happen this NPE
  - however, the fix will take some time

Even after it may be some cases, when this NPE will rise. We agreed that disabling the button n this 
case is probably best solution.


The attache dpatch do so,

Two nits :
   - is really the  SecurityDelegate suddenly useless?
   - Why all apps reproducing  this issue suddenly started to work? ( I mean without this patch)


Still I think it is good thing to get rid of assers and disable button.


J.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: npe.patch
Type: text/x-patch
Size: 1966 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140903/32d7b94b/npe-0001.patch>