[RFC][icedtea-web] Plugin does not make JNLP's <information> available when using jnlp_href

[Danesh Dadachanji]

On 24/01/12 03:13 PM, Deepak Bhole wrote:
> * Danesh Dadachanji<ddadacha at redhat.com>  [2012-01-24 14:49]:
>> Updating patch, realized there were some tabs throwing off indentation.
>>
>> Any comments on this?
>>
>
> Assuming you have tested it, OK for 1.2 and HEAD.
>

Yep, just tested 1.2, tested HEAD before posting review. Here are the 
changesets:

http://icedtea.classpath.org/hg/icedtea-web/rev/21183f821dd4
http://icedtea.classpath.org/hg/release/icedtea-web-1.2/rev/21183f821dd4

Thanks for reviewing!

Danesh

> Deepak
>
>> Cheers,
>> Danesh
>>
>> On 06/01/12 02:52 PM, Danesh Dadachanji wrote:
>>> Aaand here's the patch.
>>>
>>>
>>> On 06/01/12 02:46 PM, Danesh Dadachanji wrote:
>>>> Hi,
>>>>
>>>> The JNLP file's information elements are not available for use when
>>>> using the plugin through the browser and jnlp_href. For example when
>>>> access dialogs pop up, they try to use PluginBridge.getInformation().
>>>> This is set to return a new (and so, empty) ArrayList rather than the
>>>> contents of the information tag. To test this, use this applet[1] and
>>>> click Open.
>>>>
>>>> I've setup the info instance var in the PluginBridge constructor to
>>>> either be the JNLP file's info or to be a new ArrayList if jnlp_href is
>>>> not being used.
>>>>
>>>> PluginBridge.getInformation() is no longer needed because the
>>>> superclass's method does what we want so I've removed it. There was the
>>>> following comment in the method:
>>>>
>>>> "Should we populate this list with applet attribute tags?"
>>>>
>>>> I left this as is for now because JNLPFile.getInformation's javadoc
>>>> states it only returns the<information>  tag's elements. It might cause
>>>> some confusion/inconsistency with the javadoc if we were to store the
>>>> applet's attribute here. If we were to add a
>>>> PluginBridge.getInformation, it would be identical in implementation.
>>>> Apart from this, I don't see the harm in adding the attributes to the
>>>> info - they are visible to the public anyway through ^U! The "name"
>>>> attribute could also be used if there is no<title>  so it might be
>>>> useful to do so.
>>>>
>>>> I've also added "(unverified)" to the ends of the Publisher label of
>>>> access dialogs because this can easily be spoofed.
>>>>
>>>> ChangeLog
>>>> +2012-01-06 Danesh Dadachanji<ddadacha at redhat.com>
>>>> +
>>>> + Use the JNLP file's information section for the Name and
>>>> + Publisher labels of access dialogs, if available.
>>>> + * netx/net/sourceforge/jnlp/PluginBridge.java:
>>>> + (PluginBridge): Assigned info variable to JNLP file's information
>>>> + section (if one is used), otherwise to a new, empty ArrayList.
>>>> + (getInformation): Removed method, superclass method
>>>> + should be used instead.
>>>> + * netx/net/sourceforge/jnlp/resources/Messages.properties:
>>>> + Adding SUnverified.
>>>> + * a/netx/net/sourceforge/jnlp/security/AccessWarningPane.java:
>>>> + (addComponents): Append unverified note to the publisher label.
>>>> +
>>>>
>>>> Thoughts/comments? If this is alright, I'd like to push it to HEAD.
>>>>
>>>> Regards,
>>>> Danesh
>>>>
>>>> [1]
>>>> http://docs.oracle.com/javase/tutorial/deployment/doingMoreWithRIA/examples/dist/applet_JNLP_API/AppletPage.html
>>>>
>>>>
>
>> diff --git a/netx/net/sourceforge/jnlp/PluginBridge.java b/netx/net/sourceforge/jnlp/PluginBridge.java
>> --- a/netx/net/sourceforge/jnlp/PluginBridge.java
>> +++ b/netx/net/sourceforge/jnlp/PluginBridge.java
>> @@ -60,6 +60,7 @@ public class PluginBridge extends JNLPFi
>>                   URL jnlp = new URL(codeBase.toExternalForm() + atts.get("jnlp_href"));
>>                   JNLPFile jnlpFile = new JNLPFile(jnlp, null, false, JNLPRuntime.getDefaultUpdatePolicy(), this.codeBase);
>>                   Map<String, String>  jnlpParams = jnlpFile.getApplet().getParameters();
>> +                info = jnlpFile.info;
>>
>>                   // Change the parameter name to lowercase to follow conventions.
>>                   for (Map.Entry<String, String>  entry : jnlpParams.entrySet()) {
>> @@ -76,6 +77,9 @@ public class PluginBridge extends JNLPFi
>>                   System.err.println("Unable to get JNLP file at: " + codeBase.toExternalForm()
>>                           + atts.get("jnlp_href"));
>>               }
>> +        } else {
>> +            // Should we populate this list with applet attribute tags?
>> +            info = new ArrayList<InformationDesc>();
>>           }
>>
>>           // also, see if cache_archive is specified
>> @@ -180,15 +184,6 @@ public class PluginBridge extends JNLPFi
>>           return name;
>>       }
>>
>> -    public InformationDesc getInformation(final Locale locale) {
>> -        return new InformationDesc(this, new Locale[] { locale }) {
>> -            protected List<Object>  getItems(Object key) {
>> -                // Should we populate this list with applet attribute tags?
>> -                return new ArrayList<Object>();
>> -            }
>> -        };
>> -    }
>> -
>>       public ResourcesDesc getResources(final Locale locale, final String os,
>>                                         final String arch) {
>>           return new ResourcesDesc(this, new Locale[] { locale }, new String[] { os },
>> diff --git a/netx/net/sourceforge/jnlp/resources/Messages.properties b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties
>> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> @@ -205,6 +205,7 @@ SClipboardWriteAccess=The application ha
>>   SPrinterAccess=The application has requested printer access. Do you want to allow this action?
>>   SNetworkAccess=The application has requested permission to establish connections to {0}. Do you want to allow this action?
>>   SNoAssociatedCertificate=<no associated certificate>
>> +SUnverified=(unverified)
>>   SAlwaysTrustPublisher=Always trust content from this publisher
>>   SHttpsUnverified=The website's certificate cannot be verified.
>>   SNotAllSignedSummary=Only parts of this application code are signed.
>> diff --git a/netx/net/sourceforge/jnlp/security/AccessWarningPane.java b/netx/net/sourceforge/jnlp/security/AccessWarningPane.java
>> --- a/netx/net/sourceforge/jnlp/security/AccessWarningPane.java
>> +++ b/netx/net/sourceforge/jnlp/security/AccessWarningPane.java
>> @@ -104,7 +104,9 @@ public class AccessWarningPane extends S
>>           }
>>
>>           try {
>> -            publisher = file.getInformation().getVendor() != null ? file.getInformation().getVendor() : R("SNoAssociatedCertificate");
>> +            publisher = file.getInformation().getVendor() != null ?
>> +                    file.getInformation().getVendor() + " " + R("SUnverified") :
>> +                    R("SNoAssociatedCertificate");
>>           } catch (Exception e) {
>>           }
>>
>