[rfc][icedtea-web] allow access to jars' attributes from jnlpfile class

[Jiri Vanek]

On 11/07/2013 05:02 PM, Andrew Azores wrote:
> On 11/07/2013 10:19 AM, Jiri Vanek wrote:
>> As jnlp file is argument passed to various security dialogues, it should have access to attributes.
>> This is continuation of
>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2013-October/025070.html
>>
>> I do not have good feelings from patch,  but jnlpfile x jnlp classloader encapsulation ... or
>> "encapsulation" is what it is :(
>>
>> As the result this patch is implementing
>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/manifest.html#app_name
>>
>> Also I have found
>> http://icedtea.classpath.org/hg/icedtea-web/file/dcd51951d507/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java#l831
>> So I would like to ask if this condition is correct. IMO not. There should be :
>>  if (launchDesc != null) {
>>     mainClass = launchDesc.getMainClass();
>>  }
>>
>> Or am I missing something?
>
> I don't know, your suggestion seems to make sense but the JNLPClassLoader is just such a mess to
> deal with that I'm not sure I really can okay this change :S. If you can produce some test cases to
> show that this still works and doesn't cause problems...

I will send this as separate patch. And cc Adam who introduced those lines.

>
>> Also there is a lot of "if (mainClass == null){work}if (mainClass == null){work} " as speed up
>> there should be some "if (mainClass == null ) return"  hmm?
>
> I haven't looked deeply into it but I think this might break applets where they have a library JAR
> and a plain .class for the main-class in the codebase, depending on where you're thinking of putting
> it. Where do you mean to put this? And would it really make things much faster?

Checked again, it is not worthy.
>
>>
>>
>> J.
>
> Little nits:
> PluginBridge#getTitle :
> - Spacing around '!=' in the first null check perhaps?
> - Spelling in comment. "specification recommends main class instead of HTML parameter"
>
> ResourceDesc#getMainJar :
> - 'else' on same line as closing brace of the 'if', just to be consistent with other code
>
> Other than that, I think it looks good - so long as the other two new attributes get implemented as
> well :)

Above fixed, also littl ebut improved (some logic moved from pluginBridge to jnlpfile)

As this should go to 1.4 , please double check before aprove.


J.


2013-11-08  Jiri Vanek  <jvanek at redhat.com>

	Enabled access to manifests' attributes from JNLPFile class
	Implemented http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/manifest.html#app_name
	* netx/net/sourceforge/jnlp/JNLPFile.java: Added (manifestsAttributes) field.
	Added (ManifestsAttributes) inner class, to encapsulate access to attributes.
	(getTitle) can handle manifests too.
	* netx/net/sourceforge/jnlp/PluginBridge.java: is following app_name recommendations.
	* netx/net/sourceforge/jnlp/ResourcesDesc.java: (getMainJAR) made more granular
	* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: (init) inject itself
	to file's ManifestsAttributes. (checkForAttributeInJars) renamed field
	mainClassInThisJar to attributeInThisJar. Added getter for mainClass.
	* netx/net/sourceforge/jnlp/security/CertWarningPane.java: bracketing cleanup.
	* tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java: new test to
	check new functionalites
	* tests/netx/unit/net/sourceforge/jnlp/runtime/ResourcesDescTest.java: same
-------------- next part --------------
A non-text attachment was scrubbed...
Name: addedAccessToManifestsAttributesFromJnlpFile2.patch
Type: text/x-patch
Size: 29818 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20131108/f5624b8a/addedAccessToManifestsAttributesFromJnlpFile2-0001.patch