[rfc][icedtea-web] reproducer for PR905

Danesh Dadachanji ddadacha at redhat.com
Tue Apr 3 14:48:35 PDT 2012


Hi Jiri,

On 26/03/12 12:55 PM, Jiri Vanek wrote:
> Hi all!
>
> This reproducer is representing pr905 behaviour. All the tests except
> signed jar with parametrised archive are passing, which is correct, and
> the failing test is representing the issue.
>

Great job, thanks a lot for doing this! Please see comments below.

> However there is issue which make this reproducer ... useless. All the
> passing ones are nice, and we can include them (if you agree), but the
> falling one is signed applet (and simple signed applet test too) in
> browser. That means that user have to click on "trust" button :-/
>
> And I have currently no idea how to forward to plugin something like
> -Xtrustall :-/
>
> My only idea until now is some environment variable which will plugin
> forward to netx like Xtrustall. However - I have very strong feelings
> that this can be bloody security hole.

That definitely sounds like a bad security flaw. We need to setup something that user chooses to flag at runtime themselves (each time 
possibly), similarly to explicitly passing -Xtrustall to javaws. However, I'm not sure of any way that can do this securely. =(

One option off the top of my head, _just for testing_, might be to have a custom build flag for icedtea-web. If we pass it in when 
building (something like --build-for-testing), it could patch some of the code to skip the security checks? I'm not sure how suitable 
this would be though because it is modifying the code, no matter how small the modification is. =)

> (definietly is in windows, where you can use activeX to inject
> environment variable imho)
>
> So what do you think about the reproducer. Have it sense to push it
> without signed-applet-in-browser tests?
>

Firstly, I don't think this test should be pushed until the fix to PR905 is going into HEAD. I don't really see why having it in before 
hand is useful considering the issue is happening regardless, it may also scare off devs from running tests even more. =( Would you 
prefer to have it in before hand?

Secondly, regarding the applet-in-browser tests, I think we should keep them there but disable them until they can actually pass. Let's 
try to follow what was discussed in this email[1]. See my comments on AppletTestSigned below, they are similar.

> For future - any ideas how to create automated tests for signed applets
> in browser? (without awt robot)
>

Another idea could be to store some option (securely) accessible via itweb-settings..something like a checkbox for "Trust all applets 
run in the plugin" and then set this up properly on the test machine. I am again unclear as to how we would do this in a way that 
prevents outsiders from modifying the file that stores this flag on the FS. Do you know of any way in which we could save this 
"setting"? This does go against what I said above where this should be something specified at runtime of each applet but just throwing 
it out there. =) I suppose figuring this out would help us set something similar like an environment variable too though...

Some more comments in-line.

 > +2012-03-26  Jiri Vanek<jvanek at redhat.com>

2 spaces between Last name and <email> please.

 > +
 > +	Added tests for PR905 - parameters in jnlp/html application/applet resources
 > +	* tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html:
 > +	html file for lunching signed applet
 > +	* tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.jnlp:
 > +	jnlp fiel for lunched signed applet

s/fiel/file

 > +	* tests/jnlp_tests/signed/AppletTestSigned/srcs/AppletTestSigned.java
 > +	body of signed applet
 > +	* tests/jnlp_tests/signed/AppletTestSigned/testcases/AppletTestSignedTests.java:
 > +	(AppletTestSignedTest) testing method to launch signed applet in javaws
 > +	(AppletTestSignedFirefoxTest) testing method to launch signed applet in
 > +	browser

I think this can be a separate changeset that can be pushed. Just handle the one that is meant to fail appropriately. I think the 
resolution from the email thread[1] was to disable them and make bugs for them (for now at least). Could you go through the changes 
mentioned below that affect AppletTestSigned and post a separate review please? Thanks!

[snip]

 > +	* tests/jnlp_tests/simple/ParametrizedJarUrl/testcases/ParametrizedJarUrlTests.java:
 > +	testaceses of above ParametrizedJarUrl/jnlps+htmls
 > +	Except regular launches are included also not parametrized  calls of parametrized jnlps

Can you reword this? I don't understand what you mean. =(

 > +	(parametrizedAppletTestSignedTest), (testParametrizedJarUrl2) and (testParametrizedJarUrlSigned2)

Uhh what about these 3? If there's no comment, I don't see a reason to keep then in the ChangeLog!

 > +	(parametrizedAppletTestSignedFirefoxTest) is the only one failing and so
 > +	representing PR905
 > +


[snip]
+++ b/tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.jnlp	Mon Mar 26 18:31:36 2012 +0200
...
+<?xml version="1.0" encoding="utf-8"?>
+<jnlp spec="1.0" href="AppletTestSigned.jnlp" codebase=".">
+    <information>
+        <title>AppletTest</title>
+        <vendor>NetX</vendor>
+        <homepage href="http://jnlp.sourceforge.net/netx/"/>

Could you make sure this is IcedTea for this and all future tests? It would be great to standardize this. In fact, you motivated me to 
submit a patch for changing all the past ones! =) Can you also change the homepage to IcedTea-Web's wiki page[2] too.


[snip]
+++ b/tests/jnlp_tests/simple/ParametrizedJarUrl/resources/ParametrizedJarAppletUrlSigned.jnlp	Mon Mar 26 18:31:36 2012 +0200
...
+    <resources>
+        <j2se version="1.4+"/>
+        <jar href="AppletTestSigned.jar?time=123456"/>

Could you please be consistent with the param? I see "time" in some places and "test" in others. I prefer "test" because I think "time" 
makes it look like the param has some actual use at a first glance. Note this is in both JNLP and Java files, when you execute 
javaws/the browser server.


[snip]
+++ b/tests/jnlp_tests/simple/ParametrizedJarUrl/testcases/ParametrizedJarUrlTests.java	Mon Mar 26 18:31:36 2012 +0200
...
+    @Test
+    public void parametrizedAppletJavawsTest2() throws Exception {
+        System.out.println("connecting ParametrizedJarAppletUrl2 request");
+        System.err.println("connecting ParametrizedJarAppletUrl2 request");
+        ServerAccess.ProcessResult pr = server.executeJavawsHeadless(null, "/ParametrizedJarAppletUrl2.jnlp?time=123456");

This is supposed to simulate running 'javaws ParametrizedJarAppletUrl2.jnlp?time=123456', right? Well this is strange because if I run 
this command from terminal, I get the following:

$ cd ./icedtea-web/build/tests.build/jnlp_test_server
$ javaws ParametrizedJarAppletUrl2.jnlp?time=123456
netx: Invalid jnlp file ParametrizedJarAppletUrl2.jnlp?time=123456

I hope I'm misunderstanding the purpose of the above test (and the others similar to it) but if this is the point, then something very 
strange is happening! Could you clarify what the above test is supposed to simulate?

The tests themselves look great, great to see that as much code is being reused as possible (WRT AppletTest/Simpletest1).

Thanks again for doing this!

Cheers,
Danesh

[1] http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2012-March/017900.html
[2] http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web



More information about the distro-pkg-dev mailing list