Fwd: Re: [rfc][icedtea-web] reproducer for PR905

[Danesh Dadachanji]

Adding CCs.

Cheers,
Danesh

On 04/04/12 10:42 AM, Jiri Vanek wrote:
> uups. Looks Like I have forgot to cc distro-pkg. Please reply to this one.
>
> -------- Original Message --------
> Subject: Re: [rfc][icedtea-web] reproducer for PR905
> Date: Wed, 04 Apr 2012 15:39:34 +0200
> From: Jiri Vanek <jvanek at redhat.com>
> To: Danesh Dadachanji <ddadacha at redhat.com>, Pavel Tisnovsky <ptisnovs at redhat.com>
>
> On 04/03/2012 11:48 PM, Danesh Dadachanji wrote:
>
> I have cc also Pavel and Omair because -XtrustAll for applets done wrong can be quite serious.
>
>> 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
> This will be probably the only way. But I dont like it:(
> I like also the way via itweb-settings, especially when it will be possible via commandline, but it
> looks wrong in same way as environment variable.
>> 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
> IMHO failing tests representing issue SHOULD be pushed. Omair have temporarily suspended his effort
> to ignore failing tests.
>> 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?
> Definitely O:)
> But there is the issue with necessary click right now:-/
> Without signed-parametrized applet they have just small reason.
> I'm for:
> to push the parts which are passing
> to push the signed applet
> to push failing test
> BUT to disable last two which are failing due to necessary click:(
>>
>> 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
> hmm... some file which can just root modify, and to read it for -XtrustAll? hmm.. this can work!
> I believe this setting should be held out of itweb-settings.. What about ~/.icedtea ? But probably
> some location where just root can write will be better. As this is intended just for testing
> reasons, I'm for direct /etc.
>> 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
> O:)
>>
>> > + * 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!
>
> Yes (definitely :) ). But have no sense unless signed applets can be run automaticaly or as I told
> above.
>>
>> [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. =(
> I can't :D
>>
>> > + (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!\
> As you command :)
>>
>> > + (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.
>
> Sure!
>>
>>
>> [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?
>
> Yes I can :) You must run it as url :) eg javaws
> http://localhost:44321/ParametrizedJarAppletUrl2.jnlp?time=123456
>
> Then everything behind ? is translated as parameter for server.
>
> If you are launching it as you are, then it is considered to be file, where question mark have no
> special meaning;)
>>
>> 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
> Suspended
>> [2] http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web
> sure!
>
>
>
> J.