[rfc][icedtea-web] headless dialogues -tech preview
Andrew Azores
aazores at redhat.com
Mon Jun 15 14:59:46 UTC 2015
Hi,
On 14/06/15 02:04 PM, Jiri Vanek wrote:
> Hi!
>
> the dialog subsytem in itw is wrong.
> In last few and in a lot of future patches I'm trying to replace it.
> When I was doing the remeberable solution, I noted that headless
> uspport is completly wrong, and no worlaround may fix it.
> Th eonly correct reaction is to print dialog to stdou, and request
> user to type answer. +1 from me, her eis implementation :)
> It consist 9again) from little bit more work, soI will push it in parts.
> This patch add the headless logic, and have few advantages:
> - no aditional text hanling. The dialogs can prints themselves based
> on current ui!
> - no aditional input chekcing. With strong types, each can read
> itslef perfectly as it is already done!
>
> - it encapsualtes a lot of suplicated code - eg the if
> (!sahouldAskUser)...if (trustAll) .. If9trustNone).,... and more. So
> there is much more removed code then added :)
>
>
> So the patch itself is quite good, isnt it? :)
>
> Few drawbacks
> - stdin get broken if underlying application creates new VM.
> Currently I dont know how to workaround it. But it may be surviable.
> - it do nto work without X....
>
>
> yah the last sentence is pitfall. It is casued by EVERYTHING in itw
> expects SecurityDialogue to be existing. And new Window() is on the
> blacklist in headless mode.
> But I have some ideas how to ix it....
>
>
> J.
Very cool ideas here! I think I like the approach in general and some of
the refactoring is very nice. I do have some issues with the patch though:
> +
> OutputController.getLogger().printOutLn(dialog.getText().replace("<html>",
> "").replace("</html>", "")); //see htmlize.. but eg a href is ok to keep
I think the HTML-cleaning needs to be more rigorous than this. Sample
below, but to me this really isn't sufficient and the output isn't clean
looking.
> + OutputController.getLogger().printOutLn("Type 'exit' to terminate
> ITW one f below values:");
The print message here isn't really clear... perhaps "Type one of the
below values, or 'exit' to cancel loading the [applet|application]" ?
And should this message and others like it be in Messages.properties?
> + boolean keepGoing = true;
> + do {
> + try {
> +
> OutputController.getLogger().printOutLn(dialog.getText().replace("<html>",
> "").replace("</html>", "")); //see htmlize.. but eg a href is ok to keep
> + OutputController.getLogger().printOutLn("Type 'exit' to terminate
> ITW one f below values:");
> + OutputController.getLogger().printOutLn(dialog.helpToStdIn());
> + String s = br.readLine();
> + if (s==null){
> + break;
> + }
> + if (s.trim().toLowerCase().equals("exit")) {
> + JNLPRuntime.exit(0);
> + }
> + message.userResponse =
> dialog.readFromStdIn(s);
> + keepGoing = false;
> + //br.close(); we cant close, it closes
> underlyig stream
> + } catch (IOException eex) {
> + OutputController.getLogger().log(eex);
> + keepGoing = false;
> + //br.close(); we cant close, it closes
> underlyig stream
> + } catch (Exception ex) {
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL,
> ex.toString());
> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, ex);
> + }
> + } while (keepGoing);
Perhaps rather than looping the whole thing and reprinting the "dialog",
can you just reprint the prompt for input? And related, as it is now,
it's not very obvious when I input an invalid response - I get a stack
trace that makes perfect sense to me but I'd expect to make zero sense
to the average end user:
> Type 'exit' to terminate ITW one f below values:
> [YES, NO]
> NotARealResponse
> java.lang.IllegalArgumentException: No enum constant
> net.sourceforge.jnlp.security.dialogresults.BasicDialogValue.Primitive.NotARealResponse
> java.lang.IllegalArgumentException: No enum constant
> net.sourceforge.jnlp.security.dialogresults.BasicDialogValue.Primitive.NotARealResponse
> at java.lang.Enum.valueOf(Enum.java:238)
> at
> net.sourceforge.jnlp.security.dialogresults.BasicDialogValue$Primitive.valueOf(BasicDialogValue.java:43)
> at
> net.sourceforge.jnlp.security.dialogresults.YesNo.readValue(YesNo.java:52)
> at
> net.sourceforge.jnlp.security.dialogs.MissingPermissionsAttributePanel.readFromStdIn(MissingPermissionsAttributePanel.java:197)
> at
> net.sourceforge.jnlp.security.SecurityDialog.readFromStdIn(SecurityDialog.java:430)
> at
> net.sourceforge.jnlp.security.SecurityDialogMessageHandler.handleMessage(SecurityDialogMessageHandler.java:161)
> at
> net.sourceforge.jnlp.security.SecurityDialogMessageHandler.run(SecurityDialogMessageHandler.java:95)
> at java.lang.Thread.run(Thread.java:745)
This is also printed above the reprinted dialog, so it would be easy to
miss.
> + return "You can type YES/NO or komplex answe parsable by
> AccessWarningPaneComplexReturn.readValue.";
This really needs to be rewritten. I don't think anything after "YES/NO"
is going to make sense to end users - how are they supposed to know what
that readValue method is able to parse?
> + //this is default SecurityDialog "toString". All extending panels
> are recommanded to override this.
Maybe this would be better as a Javadoc comment?
> + //But what the hhell, it is working awesom!
And maybe this one is better removed...
> + public String getText() {
> + return traverse(this);
> + }
> + @Override
> + public DialogResult getDefaultNegativeAnswer() {
> + return YesNoSandboxLimited.no();
> + }
There are a lot of instances of weird indentation or lack of inner
whitespace in this patch, like the override annotation here being two
spaces over. Can you have your IDE apply autoformatting to each touched
file please?
> diff -r 0afcc93fb7fb
> tests/netx/unit/net/sourceforge/jnlp/security/dialogresults/NamePasswordTest.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++
> b/tests/netx/unit/net/sourceforge/jnlp/security/dialogresults/NamePasswordTest.java
> Sun Jun 14 19:52:22 2015 +0200
> @@ -0,0 +1,57 @@
> +/*
> + Copyright (C) 2009 Red Hat, Inc.
Is it? :)
Here's the sample "dialog" on stdout that I'm looking at right now:
> $ ./bin/javaws -headless
> ~/Downloads/1d_fast_fourier_transform_java_jnlp.jnlp
> This application does not specify a Codebase in its manifest. Please
> verify with the applet's vendor. Continuing. See:
> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html
> for details.
> This application does not specify a Codebase in its manifest. Please
> verify with the applet's vendor. Continuing. See:
> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html
> for details.
> Application title was not found in manifest. Check with application vendor
> Application title was not found in manifest. Check with application vendor
> Application <span color='red'> 1D Fast Fourier Transform </span> from
> <span color='red'>
> http://www.cs.brown.edu/exploratories/freeSoftware/repository/ </span>
> is missing the permissions attribute. Applications without this
> attribute should not be trusted. Do you wish to allow this application
> to run?Application <span color='red'> 1D Fast Fourier Transform
> </span> from <span color='red'>
> http://www.cs.brown.edu/exploratories/freeSoftware/repository/ </span>
> is missing the permissions attribute. Applications without this
> attribute should not be trusted. Do you wish to allow this application
> to run?
>
> <head>
>
> </head>
> <body>
> For more information you can visit:<br><a
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#permissions">JAR
>
> File Manifest Attributes</a><br>and<br><a
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html">Preventing
>
> the repurposing of Applications</a>
> </body>
>
>
> <head>
>
> </head>
> <body>
> For more information you can visit:<br><a
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#permissions">JAR
>
> File Manifest Attributes</a><br>and<br><a
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html">Preventing
>
> the repurposing of Applications</a>
> </body>
>
>
> YesYes
> NoNo
> <b>Remember this option?</b><b>Remember this option?</b>
> For appletFor applet
> For site
> <u>http://www.cs.brown.edu/exploratories/freeSoftware/repository/</u>For
> site <u>http://www.cs.brown.edu/exploratories/freeSoftware/repository/</u>
>
> Type 'exit' to terminate ITW one f below values:
> [YES, NO]
The HTML tags don't really look very good, and most things that are
printed are printed twice. I don't know what's causing that and how easy
it is to fix or work around, but I think that's a blocker for this patch.
Also, here's another bug for you:
> Type 'exit' to terminate ITW one f below values:
> [YES, NO]
> netx: Initialization Error: Could not initialize applet. (Your
> Extended applets security is at 'high' and this application is missing
> the 'permissions' attribute in manifest. And you have refused to run it.)
> netx: Initialization Error: Could not initialize applet. (Your
> Extended applets security is at 'high' and this application is missing
> the 'permissions' attribute in manifest. And you have refused to run it.)
> net.sourceforge.jnlp.LaunchException: Fatal: Initialization Error:
> Could not initialize applet. For more information click "more
> information button".
> at net.sourceforge.jnlp.Launcher.createApplet(Launcher.java:764)
> at net.sourceforge.jnlp.Launcher.launchApplet(Launcher.java:653)
> at net.sourceforge.jnlp.Launcher$TgThread.run(Launcher.java:939)
> Caused by: net.sourceforge.jnlp.LaunchException: Your Extended applets
> security is at 'high' and this application is missing the
> 'permissions' attribute in manifest. And you have refused to run it.
> at
> net.sourceforge.jnlp.runtime.ManifestAttributesChecker.checkPermissionsAttribute(ManifestAttributesChecker.java:279)
> at
> net.sourceforge.jnlp.runtime.ManifestAttributesChecker.checkAll(ManifestAttributesChecker.java:114)
> at
> net.sourceforge.jnlp.runtime.JNLPClassLoader.initializeResources(JNLPClassLoader.java:777)
> at
> net.sourceforge.jnlp.runtime.JNLPClassLoader.<init>(JNLPClassLoader.java:285)
> at
> net.sourceforge.jnlp.runtime.JNLPClassLoader.createInstance(JNLPClassLoader.java:357)
> at
> net.sourceforge.jnlp.runtime.JNLPClassLoader.getInstance(JNLPClassLoader.java:429)
> at
> net.sourceforge.jnlp.runtime.JNLPClassLoader.getInstance(JNLPClassLoader.java:403)
> at net.sourceforge.jnlp.Launcher.createApplet(Launcher.java:729)
> ... 2 more
I typed "Control-D"/EOF character for this. At this point it seems ITW
is hanging.
--
Thanks,
Andrew Azores
More information about the distro-pkg-dev
mailing list